Skip to content

Latest commit

 

History

History
83 lines (52 loc) · 3.05 KB

038.md

File metadata and controls

83 lines (52 loc) · 3.05 KB

Glamorous Tweed Albatross

High

Index Validation and Order Mismatch in performUpkeep

Summary

Improper validation of the relationship between pendingOrderIdx and orderId may lead to incorrect order execution or loss of funds for users. This vulnerability arises from the assumption that pendingOrderIds[data.pendingOrderIdx] always corresponds to data.orderId, which can break in the presence of array mutations or race conditions.

Root Cause

In performUpkeep, the code retrieves an order using the index from pendingOrderIds, then verifies that the orderId matches. This approach is fragile:

https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L92-L97 If pendingOrderIds is modified between retrieval and validation, the assumption breaks.

Internal pre-conditions

1- pendingOrderIds must contain at least one element. 2- The pendingOrderIdx must be valid and within bounds.

External pre-conditions

1- Multiple transactions interact with pendingOrderIds concurrently. 2- Modifications (e.g., additions or removals) to pendingOrderIds happen asynchronously, leading to misalignment.

Attack Path

Concurrent Execution:

  • A malicious user triggers an operation that modifies pendingOrderIds (e.g., removal or addition of an order).
  • At the same time, performUpkeep is called to process an order.

Order Mismatch:

  • The pendingOrderIdx now points to a different orderId due to array mutation.
  • The code does not validate the integrity of the orderId directly, leading to incorrect execution.

Impact:

  • An order is executed for the wrong user, potentially transferring funds or altering order status unintentionally.

Impact

1- Incorrect Order Execution: Orders may execute for unintended users, leading to financial and operational discrepancies. 2- Loss of Funds: Users could lose funds if incorrect orders are filled or refunded. 3- Integrity Issues: Undermines trust in the platform’s ability to handle concurrent transactions.

PoC

Setup: 1- Deploy the Bracket contract. 2- Add several orderIds to pendingOrderIds. 3- Trigger simultaneous calls to performUpkeep and another function that modifies pendingOrderIds.

Trigger: While performUpkeep is executing, modify pendingOrderIds by removing or adding an order.

Expected Result: Observe a mismatch between the expected orderId and the actual orderId at the given index.

Mitigation

Proposed Fix

Direct Validation and Retrieval: Ensure that data.orderId is directly validated against the array to avoid reliance on fragile index-based assumptions.

Order memory order = orders[data.orderId];

require(
    pendingOrderIds[data.pendingOrderIdx] == data.orderId,
    "Order index mismatch"
);

// Ensure atomicity during order processing
pendingOrderIds = ArrayMutation.removeFromArray(data.pendingOrderIdx, pendingOrderIds);

Additional Safeguards:

  • Lock pendingOrderIds during critical operations to prevent race conditions.
  • Emit events for debugging and monitoring concurrent access.