Skip to content

Latest commit

 

History

History
247 lines (195 loc) · 10.8 KB

091.md

File metadata and controls

247 lines (195 loc) · 10.8 KB

Uneven Satin Lizard

High

All bracket orders cannot be executed due to wrong Bracket::performData::target and Bracket::performData::txData, breaking core contract functionality

Summary

Bracket::performUpkeep executes bracket orders that are in range by calling the Bracket::performData::target address with Bracket::performData::txData function call to perform the swap. However, Bracket::performData::target and Bracket::performData::txData are wrongly set to address(this) (Bracket contract) and "0x" respectively in Bracket.sol#L60-61. This causes all bracket orders such as takeProfit (take profit) and stopPrice (stop loss) orders to fail and revert eventhough they are in range, breaking core contract functionality.

Root Cause

Bracket.sol#L60-61 is set to the wrong target address and txData function call.

Internal pre-conditions

  1. Max pending orders is not exceeded, i.e. owner calls AutomationMaster::setMaxPendingOrders to set AutomationMaster::maxPendingOrders to a non-zero value
  2. User's order exceeds min order size (simulated in this PoC by owner calling AutomationMaster::setMinOrderSize to set AutomationMaster::minOrderSize to 0 so all orders pass)
  3. User has some tokenIn and has approved Bracket contract for amountIn to create the bracket order

External pre-conditions

  1. Orders are inRange. Using longs as example in PoC, price of tokenIn and tokenOut provided by oracles must be such that exchangeRate >= takeProfit or exchangeRate <= stopPrice

Attack Path

  1. User calls Bracket::createOrder to create a bracket order
  2. exchangeRate increases above takeProfit (for takeProfit test in PoC) or decreases below stopPrice (for stopPrice test in PoC), i.e. orders are inRange
  3. Keepers calls AutomationMaster::checkUpkeep (which are passed to Bracket::checkUpkeep) to check if upKeep is needed
    3.1. Since orders are inRange, upKeep is needed (upKeepNeeded = true)
    3.2. performData is set with a MasterUpkeepData struct with target = address(this) (Bracket contract) and txData = "0x", along with other arguments \
  4. Keepers calls AutomationMaster::performUpkeep with the performData returned by checkUpkeep. This executes a series of function calls as follows:
    4.1. AutomationMaster::performUpkeep calls Bracket::performUpkeep with performData arguments
    4.2. Bracket::performUpkeep calls Bracket::execute with performData.target and performData.txData (and other arguments)
    4.3. Bracket::execute approves performData.target for performData.amountIn of performData.tokenIn
    4.4. Bracket::execute calls performData.target with performData.txData function call to perform the swap. Since the performData.target is the Bracket contract, Bracket::execute will call the Bracket contract with performData.txData.
    4.5. Since the Bracket contract does not have function selector matching perfomData.txData nor a fallback function, the call returns success = false.
    4.6. Bracket::execute reverts with TransactionFailed("0x")

Impact


Impact: High. None of the bracket orders (take profit or stop loss) that are in range can be executed, breaking core contract functionality.
Likelihood: High. The bug is always present as all bracket orders are executed through Bracket::performData::target with function call Bracket::performData::txData.
Severity: High

PoC

The below PoC is written in foundry. To set up the environment, run the following code in the oku-custom-order-types directory (ref).

  1. Install foundry

    curl -L https://foundry.paradigm.xyz | bash

  2. Install the @nomicfoundation/hardhat-foundry plugin

    npm install --save-dev @nomicfoundation/hardhat-foundry

  3. Import the plugin into hardhat.config.cts

    import "@nomicfoundation/hardhat-foundry";

  4. Initialize foundry configuration file (foundry.toml) and install forge-std using the below

    npx hardhat init-foundry

  5. Install libraries and dependencies required for testing

    forge install openzeppelin/openzeppelin-contracts --no-commit


The PoC below tests both the takeProfit and stopPrice function of a bracket order. Place the following PoC into test/Bracket.t.sol and run the following

forge test --mt testBracketOrderFails --via-ir

// SPDX-License-Identifer: MIT

pragma solidity ^0.8.20;

import {Test} from "forge-std/Test.sol";
import {AutomationMaster, IAutomation} from "contracts/automatedTrigger/AutomationMaster.sol";
import {Bracket, IBracket} from "contracts/automatedTrigger/Bracket.sol";
import {StopLimit, IStopLimit} from "contracts/automatedTrigger/StopLimit.sol";
import {ERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import {IERC20} from "contracts/interfaces/openzeppelin/IERC20.sol";
import {IPythRelay} from "contracts/oracle/IPythRelay.sol";
import {IPermit2} from "contracts/interfaces/uniswapV3/IPermit2.sol";

contract BracketTest is Test {
    address owner = makeAddr("owner");
    address keeper = makeAddr("chainlink");
    // uniswapV3 permit not used in this PoC, just dummy address
    address permit2 = makeAddr("permit2");
    address user = makeAddr("user");

    AutomationMaster MASTER;
    Bracket bracket;
    StopLimit stopLimit;
    MockWETHOracle wethOracle;
    MockUSDxOracle usdxOracle;
    ERC20Mock weth;
    ERC20Mock usdx;

    uint256 constant WETH_INITIAL_BALANCE = 1e18;

    function setUp() public {
        // Deploy contracts
        vm.startPrank(owner);
        MASTER = new AutomationMaster();
        bracket = new Bracket(MASTER, IPermit2(permit2));
        stopLimit = new StopLimit(MASTER, bracket, IPermit2(permit2));
        vm.stopPrank();

        // Deploy mock tokens
        weth = new ERC20Mock("WETH", "WETH");
        usdx = new ERC20Mock("USDx", "USDx");

        // Deploy and initialize mock oracles
        wethOracle = new MockWETHOracle();
        usdxOracle = new MockUSDxOracle();

        // Register mock oracles
        IERC20[] memory _tokens = new IERC20[](2);
        _tokens[0] = IERC20(address(weth));
        _tokens[1] = IERC20(address(usdx));
        IPythRelay[] memory _oracles = new IPythRelay[](2);
        _oracles[0] = IPythRelay(address(wethOracle));
        _oracles[1] = IPythRelay(address(usdxOracle));
        vm.prank(owner);
        MASTER.registerOracle(_tokens, _oracles);

        // Register subKeepers
        vm.prank(owner);
        MASTER.registerSubKeepers(IStopLimit(stopLimit), IBracket(bracket));

        // Assume WETH price
        wethOracle.setPrice(3000e8); // 3000 WETH/USD

        // Condition 1: Max pending orders is not exceeded
        // simulated by setting maxPendingOrders = type(uint16).max so all orders pass
        vm.prank(owner);
        MASTER.setMaxPendingOrders(type(uint16).max);

        // Condition 2: Min order size is obeyed
        // simulated by setting minOrderSize = 0 so all orders pass
        vm.prank(owner);
        MASTER.setMinOrderSize(0);

        // Condition 3: User has some tokenIn (WETH) and has approved bracket contract for amountIn
        weth.mint(user, WETH_INITIAL_BALANCE);
        vm.prank(user);
        weth.approve(address(bracket), WETH_INITIAL_BALANCE);

    }

    function testBracketOrderFails() public {
        // Step 1: User makes bracket order
        bytes memory swapPayload = "";
        uint256 takeProfit = 4000e8; // TP = 4000 WETH/USD
        uint256 stopPrice = 2000e8; // SL = 2000 WETH/USD
        uint256 amountIn = WETH_INITIAL_BALANCE;
        IERC20 tokenIn = IERC20(address(weth));
        IERC20 tokenOut = IERC20(address(usdx));
        address recipient = user;
        uint16 feeBips = 0;
        uint16 takeProfitSlippage = 100; // 100 BIPS
        uint16 stopSlippage = 100; // 100 BIPS
        bool permit = false;
        bytes memory permitPayload = "";
        
        vm.prank(user);
        bracket.createOrder(swapPayload, takeProfit, stopPrice, amountIn, tokenIn, tokenOut, recipient, feeBips, takeProfitSlippage,
        stopSlippage, permit, permitPayload);

        //////////////////
        // takeProfit test
        //////////////////

        // Step 2: Order is InRange
        // simulated WETH price increase
        wethOracle.setPrice(4000e8); // 4000 WETH/USD

        // Step 3: Keeper calls checkUpkeep
        vm.prank(keeper);
        (bool upkeepNeededTP, bytes memory performDataTP) = MASTER.checkUpkeep("");
        IAutomation.MasterUpkeepData memory dataTP = abi.decode(performDataTP, (IAutomation.MasterUpkeepData));
        assertTrue(upkeepNeededTP);
        assertEq(dataTP.target, address(bracket));
        assertEq(dataTP.txData, "0x");

        // Step 4: UpKeep needed, Keeper calls performUpkeep
        vm.prank(keeper);
        vm.expectRevert();
        MASTER.performUpkeep(performDataTP);

        //////////////////
        // stopPrice test
        //////////////////

        // Step 2: Order is InRange (stopPrice test)
        // simulated WETH price decrease
        wethOracle.setPrice(2000e8); // 2000 WETH/USD

        // Step 3: Keeper calls checkUpkeep
        vm.prank(keeper);
        (bool upkeepNeededSL, bytes memory performDataSL) = MASTER.checkUpkeep("");
        IAutomation.MasterUpkeepData memory dataSL = abi.decode(performDataSL, (IAutomation.MasterUpkeepData));
        assertTrue(upkeepNeededSL);
        assertEq(dataSL.target, address(bracket));
        assertEq(dataSL.txData, "0x");

        // Step 4: UpKeep needed, Keeper calls performUpkeep
        vm.prank(keeper);
        vm.expectRevert();
        MASTER.performUpkeep(performDataSL);
    }
}

contract MockWETHOracle {
    uint256 WETHUSDprice;
    
    function setPrice(uint256 price) external {
        WETHUSDprice = price;
    }

    function currentValue() external view returns(uint256) {
        return WETHUSDprice;
    }
}

contract MockUSDxOracle {
    uint256 USDXUSDprice = 1e8;

    function currentValue() external view returns(uint256) {
        return USDXUSDprice;
    }
}

contract ERC20Mock is ERC20 {
    constructor(
        string memory name,
        string memory symbol
    )
        payable
        ERC20(name, symbol)
    {}

    function mint(address account, uint256 amount) public {
        _mint(account, amount);
    }
}

Mitigation

In Bracket.sol#L60-61, set target to a DEx and set txData to function selector and arguments of a DEx swap.