Skip to content

Latest commit

 

History

History
97 lines (59 loc) · 3.06 KB

037.md

File metadata and controls

97 lines (59 loc) · 3.06 KB

Glamorous Tweed Albatross

High

Empty pendingOrderIds Array Causes Logical Errors

Summary

The ArrayMutation.removeFromArray function is used to remove an element from the pendingOrderIds array. However, the contract using this library (e.g., in performUpkeep) does not handle cases where pendingOrderIds becomes empty after the last order is removed, leading to potential logical errors and contract instability. https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/libraries/ArrayMutation.sol#L5 https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L85

Root Cause

In Bracket.sol: performUpkeep, there is no validation to ensure that pendingOrderIds is non-empty before accessing its elements. The system assumes that the array will always contain pending orders, even after all orders are processed and removed.

Internal pre-conditions

1- pendingOrderIds contains at least one order initially. 2- ArrayMutation.removeFromArray() is called to remove orders during processing. 3- All orders are removed, leaving the array empty.

External pre-conditions

1- The platform actively processes and removes orders until the pendingOrderIds array becomes empty.

2- Users or systems rely on performUpkeep for order execution or upkeep tasks.

Attack Path

1- A user or external system triggers performUpkeep. 2- The last pending order is processed and removed from the pendingOrderIds array. 3- Another performUpkeep call occurs, attempting to access the now-empty pendingOrderIds. 4- The function reverts because there are no safeguards to handle an empty array.

Impact

Denial of Service: Users and external systems are unable to process orders or call upkeep-related functions. Operational Disruption: The platform’s functionality halts for order processing until the issue is addressed. Increased Gas Costs: Repeated reverts result in wasted gas fees for users

PoC

1- Setup:

  • Deploy the Bracket contract.
  • Populate the pendingOrderIds array with a few elements.

2- Trigger:

  • Process orders using performUpkeep until all orders are removed.

3- Result:

  • Call performUpkeep again, and observe a revert due to an attempt to access an empty array.

Mitigation

Validation in performUpkeep: Add a check to ensure the pendingOrderIds array is non-empty before proceeding.

function performUpkeep(
    bytes calldata performData
) external override nonReentrant {

 require(pendingOrderIds.length > 0, "No pending orders");

 MasterUpkeepData memory data = abi.decode(
     performData,
     (MasterUpkeepData)
 );
 Order memory order = orders[pendingOrderIds[data.pendingOrderIdx]];
 // Existing logic...
}

Emit Events for Monitoring: Emit a specific event when no orders are available for processing, aiding debugging and analytics.

event NoPendingOrders();

Trigger this event when the array is empty:

if (pendingOrderIds.length == 0) {
    emit NoPendingOrders();
    return;
}