Skip to content

Latest commit

 

History

History
107 lines (81 loc) · 3.76 KB

094.md

File metadata and controls

107 lines (81 loc) · 3.76 KB

Little Admiral Squid

Medium

Incorrect Oracle Validation in _createOrder() Function Leads to Order Creation be Prevented with Unidentified Revert Message

Summary

The _createOrder() function in the contract Bracket.sol fails to validate the oracle existence for tokenOut. Instead, it redundantly checks the oracle existence for tokenIn twice. This incorrect implementation can lead to scenarios where tokenOut does not have a valid oracle, causing subsequent calls to the _getExchangeRate function in the AutomationMaster.sol contract to fail with unidentified revert reason.

Root Cause

The issue arises from the following lines in the _createOrder() function:

    function _createOrder(
        uint256 takeProfit,
        uint256 stopPrice,
        uint256 amountIn,
        uint96 existingOrderId,
        IERC20 tokenIn,
        IERC20 tokenOut,
        address recipient,
        uint16 feeBips,
        uint16 takeProfitSlippage,
        uint16 stopSlippage
    ) internal {
        //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"
        );
    ...

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

This is problematic as in the rest of the function it calls the _getExchangeRate(). In the _getExchangeRate() function of the AutomationMaster contract, the calculation of the exchange rate divides priceIn by priceOut.

    function _getExchangeRate(
        IERC20 tokenIn,
        IERC20 tokenOut
    ) internal view returns (uint256 exchangeRate) {
        // Retrieve USD prices from oracles, scaled to 1e8
        uint256 priceIn = oracles[tokenIn].currentValue();
        uint256 priceOut = oracles[tokenOut].currentValue();

        // Return the exchange rate in 1e8 terms
        return (priceIn * 1e8) / priceOut;
    }

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

If the oracle for tokenOut is missing, priceOut becomes zero, resulting in a division by zero and a transaction revert.

Internal pre-conditions

  1. The oracle address of a token (tokenOut) doesn't exist yet and needs to be address zero

External pre-conditions

No response

Attack Path

  1. Users want to create an order or fill stop limit for an order
  2. Their considered token out is A which its oracle is not initialized yet
  3. They call createOrder() (or fillStopLimitOrder())
  4. This call will revert and the revert reason is not caught properly

Impact

  1. The order creation will be reverted due to division by zero error and the revert reason couldn't be identified as there is not an error catching for this issue
  2. Users cannot create orders if tokenOut lacks a valid existent oracle, as the system reverts during the order creation.

PoC

No response

Mitigation

Consider correcting the oracle existence check:

    function _createOrder(
        uint256 takeProfit,
        uint256 stopPrice,
        uint256 amountIn,
        uint96 existingOrderId,
        IERC20 tokenIn,
        IERC20 tokenOut,
        address recipient,
        uint16 feeBips,
        uint16 takeProfitSlippage,
        uint16 stopSlippage
    ) internal {
        //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),
+                address(MASTER.oracles(tokenOut)) != address(0x0),
            "Oracle !exist"
        );