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

11 stars 6 forks source link

`Pools.deposit()` and `Pools.withdraw()` can have problem with rebasing token #887

Closed c4-bot-10 closed 7 months ago

c4-bot-10 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/pools/Pools.sol#L205-#L215 https://github.com/code-423n4/2024-01-salty/blob/main/src/pools/Pools.sol#L219-#L230

Vulnerability details

Vulnerability details

In Pools contract, function deposit() and withdraw() is used to deposit and withdraw token. It stored number of token deposit/withdraw at the moment token is transfered:

   _userDeposits[msg.sender][token] += amount;

   _userDeposits[msg.sender][token] -= amount;

But the problem is it is not true when interacting with rebasing token, when the balance can be changed. In the worst case, when there are two user that deposit rebasing token in the contract, and total rebasing token is down, one of them will lost token when he withdraw/swap later than other one.

Impact

Total balance of each user is not tracked correctly when token is rebasing token

Tools Used

Manual review

Recommended Mitigation Steps

With current design, it is recommended that rebasing token should not be used

Assessed type

ERC20

Picodes commented 7 months ago

Out of scope per https://github.com/code-423n4/2024-01-salty/blob/main/bot-report.md (L-17)