Skip to content

Latest commit

 

History

History
136 lines (98 loc) · 3.9 KB

029.md

File metadata and controls

136 lines (98 loc) · 3.9 KB

Flaky Merlot Parrot

High

Unauthorized Overwriting of Orders in _createOrder Allows Malicious Order Takeover

Summary

The _createOrder function in the Bracket.sol contract permits overwriting an existing order without verifying ownership of the existingOrderId. This enables attackers to hijack or modify orders belonging to other users, potentially causing financial loss or disruption of order processing.

https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L444

Root Cause

The _createOrder function does not verify whether the caller owns the order identified by existingOrderId when it is not zero. Consequently, any user can overwrite an order if they know its orderId.

https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L476-L477

Internal pre-conditions

Unrestricted Access: The _createOrder function is invoked by public functions such as createOrder or fillStopLimitOrder.

No Ownership Verification: There is no check to ensure that the msg.sender corresponds to the recipient of the existing order.

External pre-conditions

Knowledge of orderId: An attacker must know or guess a valid existingOrderId.

Attack Path

Step 1: The attacker observes an order's existingOrderId (e.g., through emitted events like OrderCreated or other means).

Step 2: The attacker calls a public function such as createOrder or fillStopLimitOrder with:

  • The known existingOrderId.
  • Modified parameters for their benefit (e.g., favorable slippage, token swaps, or price limits).

Step 3: The _createOrder function overwrites the original order without verifying ownership.

Impact

Order Hijacking: The attacker gains control over the order, potentially altering its parameters or executing it under malicious terms.

Financial Loss: The original user may lose funds or tokens due to unintended order modifications or cancellations.

Loss of Trust: Users may lose confidence in the platform due to unauthorized actions affecting their orders.

PoC

Scenario Setup:

1- User A creates an order:

createOrder(
    swapPayload,
    2000,   // takeProfit
    1800,   // stopPrice
    100,    // amountIn
    tokenIn,
    tokenOut,
    recipientA,
    10,     // feeBips
    50,     // takeProfitSlippage
    50,     // stopSlippage
    permit,
    permitPayload
);

The order ID emitted in OrderCreated is 1.

2- Attack Execution:

Attacker observes the OrderCreated event and notes existingOrderId = 1. The attacker calls createOrder or fillStopLimitOrder with:

createOrder(
    swapPayload,
    2100,   // modified takeProfit
    1700,   // modified stopPrice
    200,    // new amountIn
    tokenIn,
    tokenOut,
    recipientB,  // Attacker's address
    20,     // Increased feeBips
    100,    // Increased takeProfitSlippage
    100,    // Increased stopSlippage
    permit,
    permitPayload
);

3- Outcome: The attacker successfully overwrites User A's order with their malicious parameters, taking control of the order and its associated tokens.

Mitigation

Add a check to ensure that if existingOrderId is provided, the caller is the owner of the order. Corrected Code:

function _createOrder(
    // Parameters...
    uint96 existingOrderId,
    address recipient,
    // More parameters...
) internal {
    // ...

    if (existingOrderId == 0) {
        existingOrderId = MASTER.generateOrderId(msg.sender);
    } else {
+       // Verify that the caller owns the existing order
+       require(orders[existingOrderId].recipient == recipient, "Not order owner");
    }

    // Construct order
    orders[existingOrderId] = Order({
        orderId: existingOrderId,
        // Other parameters...
    });

    // Store pending order
    pendingOrderIds.push(existingOrderId);

    emit OrderCreated(existingOrderId);
}