performUpKeep can be used to extract collateral value of liquidating and AAA profit of DAO

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/Upkeep.sol#L107 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/Liquidizer.sol#L139-L141 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/Upkeep.sol#L178

Vulnerability details

In performUpKeep step1 and step 5, it will swap tokenA into tokenB.If the amount of tokenA is extremely large,this swap will ca use a substantial slippage, and equivalent tokens won't be obtained after the swap.this slippage value(collateral value of liquidating and AAA profit of DAO)manifest in the pools at incorrect prices, and can be extracted by attackers or users through swap.

Impact

Because some of collateral value is extrated from pools, it will cause usdsThatShouldBeBurned in liquidizer much great than USDS

It will cause liquidity provider get less profit from pools by AAA.

Proof of Concept

In performUpkeep step1,it will call liquidizer's performUpkeep function to swap WETH to USDS：

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

function step1() public onlySameContract
    {
    collateralAndLiquidity.liquidizer().performUpkeep();
    }

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/Liquidizer.sol#L139-L141

function performUpkeep() external
    {
    require( msg.sender == address(exchangeConfig.upkeep()), "Liquidizer.performUpkeep is only callable from the Upkeep contract" );

    uint256 maximumInternalSwapPercentTimes1000 = poolsConfig.maximumInternalSwapPercentTimes1000();

    // Swap tokens that have previously been sent to this contract for USDS
    PoolUtils._placeInternalSwap(pools, wbtc, usds, wbtc.balanceOf(address(this)), maximumInternalSwapPercentTimes1000 );
    PoolUtils._placeInternalSwap(pools, weth, usds, weth.balanceOf(address(this)), maximumInternalSwapPercentTimes1000 );
    PoolUtils._placeInternalSwap(pools, dai, usds, dai.balanceOf(address(this)), maximumInternalSwapPercentTimes1000 );

    // Any SALT balance seen here should just be burned so as to not put negative price pressure on SALT by swapping it to USDS
    uint256 saltBalance = salt.balanceOf(address(this));
    if ( saltBalance > 0 )
        {
        salt.safeTransfer(address(salt), saltBalance);
        salt.burnTokensInContract();
        }

    _possiblyBurnUSDS();
    }
}

When a large amount of collateral can be liquidated(it's not bad debt, and in 105%~110% collateral ratio）, WBTC/WETH collateral can be liquidated directly by anyone, resulting in a large amount of WETH and WBTC being held in the liquidizer.

Attackers can liquidate the collateral assets and call performUpKeep to exchange WETH and WBTC for USDS in situations with very high slippage, causing some of the liquidated collateral value to be converted into incorrect prices in the WETH/USDS pool. Even if AAA occurs after the swap and some of the value is arbitraged back into the DAO, the remaining collateral value can still be obtained by the attacker through swap.

In the extreme scenario where there are many collateral assets in the contract that need to be liquidated, the AAA path after swapping WETH to USDS is WETH->WBTC->USDS->WETH， and there is minimal arbitrage opportunity due to WETH/WBTC pool in AAA path has little liquidity(token has been liquidated), and the collateral value converted into WETH/USDS can be mostly withdrawn by the attacker.

This situation will occurs in performUpKeep step5,it will swap 90.25% of WETH(send 5% in step3 and 4.75% in step4 to DAO) to SALT:

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

function step5() public onlySameContract
    {
    uint256 wethBalance = weth.balanceOf( address(this) );
    if ( wethBalance == 0 )
        return;

    // Convert remaining WETH to SALT and send it to SaltRewards
    uint256 amountSALT = pools.depositSwapWithdraw( weth, salt, wethBalance, 0, block.timestamp );
    salt.safeTransfer(address(saltRewards), amountSALT);
    }

When pools deposit a significant amount of WETH into the DAO through AAA arbitrage, this amount will be withdrawn into the Upkeep contract at step 2 and, at step 5, 90.75% of WETH will swap into SALT, resulting in a substantial slippage. The arbitrage profits not captured by AAA can also be swapped out by attackers in the same method, capturing the profits that should have been distributed to LPs.

Tools Used

vscode、foundary

Recommended Mitigation Steps

Whent swap in performUpkeep,add swap amount limit to prevent this situation occurs.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #344

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as unsatisfactory: Insufficient proof

[Picodes]

Doesn't discuss the AA functionalities