Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L373-L380

Vulnerability details

Impact

This can lead to instances where are funds getting stuck and some players will not be able to withdraw sometimes.

Per the audit documentation rebasing tokens are in the scope of this audit.

Proof of Concept

When players lock their tokens with lock(...) function the player’s lockedToken.quantity is updated to include the amount tokens that the player entered.

However, if the the tokenContract is a rebasing token the players lockedToken.quantity will be over inflated when the token supply reduces.

File: LockManager.sol
373:         // Transfer erc tokens
374:         if (_tokenContract != address(0)) { // normal ERC not ETH
375:             IERC20 token = IERC20(_tokenContract);
376:             token.transferFrom(_tokenOwner, address(this), _quantity);
377:         }
...
379:         lockedToken.remainder = remainder;
380:         lockedToken.quantity += _quantity;

Alice locks 20_000e18 token A which is a rebase token
Alice lockedToken.quantity = 20_000e18
20_000e18 is transferred from Alice wallet to LockManager but rebase applies and balance of LockManager decreases
Alice tries to unlock(...) but her tokens are stuck because there is not enough funds for her to withdraw and she can only withdraw exactly lockedToken.quantity.

Tools Used

Manual review

Recommended Mitigation Steps

Modify the _lock(...) such that the balance LockManager is checked before and after the transfer is made from the player.
unlock should be done according to the percentage of the player share in the LockManager balance

Assessed type

Token-Transfer

code-423n4 / 2024-05-munchables-findings

LockManager does not handle Rebasing tokens properly #434