Lines of code

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

Vulnerability details

Impact

Loss of funds for users over time, because the situation gets worse after every lock

Proof of Concept

The accounting in the LockManager contract will be messed up, since the actual funds received by the contract, are less than the _quantity variable that tracks them. It gets worse as time passes, because at one point the funds in the contract will be significantly less than the less than the sum of the quantity variables that are supposed to track them. Fee-on-transfer issues are in the toke behaviour scope, so this should be considered a valid issue! Also had conversation with the sponsor and was told that they will add BLAST token in the future.

Tools Used

Manual review

Recommended Mitigation Steps

Get the token balances before and after the transfers, then subtract them to get the actual value that got transferred to the contract, like this:

 if (_tokenContract != address(0)) {
            IERC20 token = IERC20(_tokenContract);
+            uint256 balanceBefore = token.balanceOf(address(this));
            token.transferFrom(_tokenOwner, address(this), _quantity);
+            uint256 balanceAfter = token.balanceOf(address(this));
+            _quantity = balanceAfter - balanceBefore;
        }

Assessed type

ERC20

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

Fee-on-tranfer tokens will mess up the accounting of the `LockManager` contract #511