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

5 stars 3 forks source link

Residual approvals will cause `_depositLiquidityAndIncreaseShare` to revert for some tokens (e.g USDT) #926

Closed c4-bot-3 closed 5 months ago

c4-bot-3 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L96-L97 https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L110-L114

Vulnerability details

Vulnerability details

Some tokens, the most known being USDT, will revert if an approval M is set while allowance is still >0 : https://github.com/d-xo/weird-erc20?tab=readme-ov-file#approval-race-protections The thing is, _depositLiquidityAndIncreaseShare approve maxAmount, whilesending maxAmount - addedAmount afterward, where addedAmount is the recalculated amount based on the pool reserves ratio.

If addedAmount >0, then a residual allowance will exist between the Liquidity contract and Pool contract, which will prevent any deposit to the USDT pool reserves by any user.

This is because user do not directly transfer the tokens to the pool, but first transfer them to the Liquidity, which itself approve the amount to Pool and transfer the amount.

Impact

Any liquidity deposit to a USDT pool (or any other tokens with same behavior) will revert as soon as there is a residual allowance (which will forcibly happen as explained above)

Tools Used

Manual review

Recommended Mitigation Steps

Set back allowance to zero once transfer are executed.

Assessed type

ERC20

c4-judge commented 5 months ago

Picodes marked the issue as duplicate of #940

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Out of scope