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

11 stars 6 forks source link

Insufficient reserve check when removing liquidity #602

Closed c4-bot-2 closed 9 months ago

c4-bot-2 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L187

Vulnerability details

Impact

The lack of not checking whether or not reserve1 is also below DUST amount leads to the reserve ratios between reserve0 and reserve1 not being relatively constant even in the case of a potential maximum withdrawal.

Proof of Concept

    // Make sure that removing liquidity doesn't drive either of the reserves below DUST.
    // This is to ensure that ratios remain relatively constant even after a maximum withdrawal.
@>  require((reserves.reserve0 >= PoolUtils.DUST) && (reserves.reserve0 >= PoolUtils.DUST), "Insufficient reserves after liquidity removal");

    // Switch reclaimed amounts back to the order that was specified in the call arguments so they make sense to the caller
    if (flipped)

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L187

Tools Used

Manual Review, VS Codium

Recommended Mitigation Steps

add reserve1 to the require check

-   require((reserves.reserve0 >= PoolUtils.DUST) && (reserves.reserve0 >= PoolUtils.DUST), "Insufficient reserves after liquidity removal");

+   require((reserves.reserve0 >= PoolUtils.DUST) && (reserves.reserve1 >= PoolUtils.DUST), "Insufficient reserves after liquidity removal");

Assessed type

Error

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #647

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory