search

code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

`Liquidizer.performUpkeep` will revert if either SALT/USDS or DAI/USDS has less than `DUST` liquidity #649

Closed c4-bot-2 closed 8 months ago

c4-bot-2 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L369-L372

Vulnerability details

Summary

Liquidizer.performUpkeep can call pools.removeLiquidity with liquidityToWithdraw set to 0, which will lead to a revert.

Vulnerability Details

Liquidizer.performUpkeep calls _possiblyBurnUSDS, which will call dao.withdrawPOL in case usdsBalance < usdsThatShouldBeBurned.

withdrawPOL calculates uint256 liquidityToWithdraw = (liquidityHeld * percentToLiquidate) / 100;

liquidityToWithdraw is 0 when liquidityHeld is less than 100;

Then in withdrawPOL, there is a call to collateralAndLiquidity.withdrawLiquidityAndClaim => _withdrawLiquidityAndClaim => pools.removeLiquidity. removeLiquidity has a requirement liquidityToRemove > 0.

When either SALT/USDS or DAI/USDS has liquidityHeld less than 100, the whole Liquidizer.performUpkeep will revert.

It means that if one of the pools has enough liquidity, it won't be used to burn USDS in case another one does not. This can happen because, by default, the USDS/DAI pool receives less liquidity (5%) in Upkeep.step3 than SALT/USDS (20%, Upkeep.step4).

The USDS supply is not decreased => USDS price decreases => USDS depeg => CoreSaltyFeed returns incorrect prices => PriceAggregator returns incorrect prices or reverts.

PriceAggregator reverts if the two closest prices have a >3% difference (by default). And it calculates an average price if the difference is <3%. A 30-minute TWAP and Chainlink spot can easily differ. And CoreSaltyFeed returns an incorrect price => it can be the closest price to TWAP or Chainlink => we have an incorrect price if the closest difference is <3% or a revert if it's > 3% => most functions in CollateralAndLiquidity revert or use an incorrect price => unexpected liquidations.

When Upkeep.performUpkeep is called often enough, and arbitrage profits are not significant, especially likely at the start of the protocol, dao.formPOL can also revert (it has require(maxAmountA > PoolUtils.DUST and require(maxAmountB > PoolUtils.DUST). So the dao keeps having low liquidity in these pools.

Impact

Liquidizer.performUpkeep reverts:

Assessed type

Invalid Validation

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #344

c4-judge commented 8 months ago

Picodes marked the issue as not a duplicate

Picodes commented 8 months ago

Downgrading to Low because if the liquidity is below dust it won't make a large difference, and that it should happen under a proper configuration of the protocol

c4-judge commented 8 months ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 8 months ago

Picodes marked the issue as grade-c