086.md

Fierce Pecan Swallow

Medium

Does not check if Oracle for tokenOut exists when creating new orders in the Bracket contract

Summary

_createOrder() function in Bracket contract checks (using require statement) whether registered oracle exists for the tokenIn and tokenOut before creating the new order. But this require statement always return true even if Oracle of tokenOut does not exist because there is a typo error for the second check

 require(
            address(MASTER.oracles(tokenIn)) != address(0x0) &&
                address(MASTER.oracles(tokenIn)) != address(0x0),
            "Oracle !exist"
        );

Suggestion solution: Update tokenIn to tokenOut in the second condition check

require(
            address(MASTER.oracles(tokenIn)) != address(0x0) &&
                **address(MASTER.oracles(tokenOut)) != address(0x0),**
            "Oracle !exist"
        );

Root Cause

The incorrect code is below

    //verify both oracles exist, as we need both to calc the exchange rate
        require(
            address(MASTER.oracles(tokenIn)) != address(0x0) &&
                address(MASTER.oracles(tokenIn)) != address(0x0),
            "Oracle !exist"
        );

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

Attack Path

Attack POC might not be possible because getExchangeRate() will revert. Hence marking it as medium vulnerability. Recommended to fix this raised issue to avoid unexpected behaviours in future.

Impact

No major impact as getExchangeRate() is returning revert but better to resolve this main issue as might lead to unexpected consequences in future or someone might misuse this to their advantage and find the attacking path.

PoC

Attack POC might not be possible because of getExchangeRate() is validated on zero pricing

        // const tokens = [await s.WETH.getAddress(), await s.USDC.getAddress(), await s.UNI.getAddress(), await s.ARB.getAddress()]
        // const oracles = [await s.wethOracle.getAddress(), await s.usdcOracle.getAddress(), await s.uniOracle.getAddress(), await s.arbOracle.getAddress()]


        const tokens = [await s.WETH.getAddress(), await s.UNI.getAddress(), await s.ARB.getAddress()]
        const oracles = [await s.wethOracle.getAddress(), await s.uniOracle.getAddress(), await s.arbOracle.getAddress()]
describe("Execute Bracket check Order Create with registered oracle token", () => {


    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("AshishLach", 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 swapInData = await generateUniTxData(
            s.USDC,
            await s.WETH.getAddress(),
            s.usdcAmount,
            s.router02,
            s.UniPool,
            await s.Bracket.getAddress(),
            await s.Master.getMinAmountReceived(s.usdcAmount, await s.USDC.getAddress(), await s.WETH.getAddress(), swapInBips)
        )

        const swapParams: SwapParams = {
            swapTokenIn: await s.USDC.getAddress(),
            swapAmountIn: s.usdcAmount,
            swapTarget: s.router02,
            swapBips: swapInBips,
            txData: swapInData
        }

        const swapParamsTuple = [
            swapParams.swapTokenIn,           // Address (IERC20)
            BigInt(swapParams.swapAmountIn),   // uint256
            swapParams.swapTarget,             // Address
            swapParams.swapBips,               // uint32
            swapParams.txData                  // bytes
        ];

        const swapPayload = ethers.AbiCoder.defaultAbiCoder().encode(
            ["tuple(address,uint256,address,uint32,bytes)"], // Struct as a tuple
            [swapParamsTuple]                                // Data as a single tuple
        );

        await s.Bracket.connect(s.Bob).createOrder(
            swapPayload,
            currentPrice + strikeDelta,
            currentPrice - stopDelta,
            s.wethAmount,
            await s.WETH.getAddress(),
            await s.USDC.getAddress(),
            await s.Bob.getAddress(),
            5,//5 bips fee
            strikeBips,
            stopBips,
            false,//no permit
            "0x"
        )

        console.log("finised succesfully");

        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.length).to.eq(1, "1 pending order")

        //verify our input token was received
        const balance = await s.WETH.balanceOf(await s.Bracket.getAddress())
        expect(balance).to.be.closeTo(s.wethAmount, 200000000000000000n, "WETH received")

    })
})

Mitigation

Update tokenIn to tokenOut in the second condition check

  require(
            address(MASTER.oracles(tokenIn)) != address(0x0) &&
                address(MASTER.oracles(tokenOut)) != address(0x0),
            "Oracle !exist"
        );