036.md

Glamorous Tweed Albatross

High

Risk of Underflow and Minimum Order Violation During Position Decrease

Summary

The modifyOrder function in the Bracket.sol contract lacks adequate checks to prevent newAmountIn from being reduced to zero or below the acceptable minimum order size. This oversight introduces potential for contract underflow errors and invalid orders, disrupting normal operations. https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L216

Root Cause

Lack of Zero Check: The code does not explicitly prevent newAmountIn from being reduced to zero.

Insufficient Validation: While the MASTER.checkMinOrderSize function checks for minimum requirements, it does so after the newAmountIn calculation, leaving a window for invalid operations.

Internal pre-conditions

1. Initial Amount: The order.amountIn is set during order creation.

2. Delta Reduction: amountInDelta is provided during position modification, which reduces order.amountIn.

External pre-conditions

1- User Action: A user calls modifyOrder with a large amountInDelta close to or equal to order.amountIn.

2- Market Changes: External market conditions may indirectly incentivize reducing positions to near-zero values.

Attack Path

Step 1: A user creates an order with amountIn set to a valid value (e.g., 100 tokens).

createOrder(
    swapPayload,
    takeProfit,
    stopPrice,
    100,  // amountIn
    tokenIn,
    tokenOut,
    recipient,
    feeBips,
    takeProfitSlippage,
    stopSlippage
);

Step 2: The user later modifies the order, setting amountInDelta close to order.amountIn, such as 99 tokens.

modifyOrder(
    orderId,
    _takeProfit,
    _stopPrice,
    99,  // amountInDelta
    tokenOut,
    recipient,
    takeProfitSlippage,
    stopSlippage,
    false,
    false,
    permitPayload
);

Step 3: The newAmountIn becomes 1.

newAmountIn = order.amountIn - amountInDelta; // 100 - 99 = 1

Step 4: The contract proceeds to MASTER.checkMinOrderSize, where:

• If the minimum size is greater than 1 token, the operation fails.
• If unchecked, subsequent operations relying on newAmountIn may underflow.

Impact

1- Order Invalidity:

• Orders with newAmountIn reduced to zero or below the minimum size cannot execute, creating inconsistencies.

2- Contract Reversion:

• Subtraction leading to underflow may cause the contract to revert, halting execution.

3- User Frustration:

• Users may encounter unexpected errors when attempting to reduce positions.

PoC

1- Setup: Deploy the Bracket.sol contract on a testnet. Create Order:

createOrder(
    swapPayload,
    takeProfit,
    stopPrice,
    100,  // amountIn
    tokenIn,
    tokenOut,
    recipient,
    feeBips,
    takeProfitSlippage,
    stopSlippage
);

2- Modify Order:

modifyOrder(
    orderId,
    _takeProfit,
    _stopPrice,
    99,  // amountInDelta
    tokenOut,
    recipient,
    takeProfitSlippage,
    stopSlippage,
    false,
    false,
    permitPayload
);

3- Observe Behavior: Without the proposed fix, the contract may revert due to MASTER.checkMinOrderSize or allow invalid operations if the minimum size is not enforced.

Mitigation

Validate Non-Zero Amounts: Prevent newAmountIn from being reduced to zero:

require(newAmountIn > 0, "Amount cannot be zero");

Ensure Minimum Compliance: Validate against MASTER.checkMinOrderSize only after confirming newAmountIn is valid:

MASTER.checkMinOrderSize(order.tokenIn, newAmountIn);

Corrected Code:

if (amountInDelta != 0) {
    if (increasePosition) {
        newAmountIn += amountInDelta;
        // Token handling logic...
    } else {
        require(amountInDelta < order.amountIn, "Invalid delta");
        newAmountIn -= amountInDelta;
+       require(newAmountIn > 0, "Amount cannot be zero");
        MASTER.checkMinOrderSize(order.tokenIn, newAmountIn);
        // Refund position logic...
    }
}