Lines of code

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

Vulnerability details

Vulnerability Detail

unlock(); function does not have a check if the LockManager has enough token balance, thus since we are using .transfer() instead of safeTransfer and not checking the return status, the transfer will fail silently while the changes to lockedToken.quantity for the user will remain, thus the user will end up receiving those

One example which could lead to less tokens for the LockManager contract, which is related to: On lock() if the token is a "Fee on Tranfer" token then funds will be lost as the _quantity set in the lockedToken.quantity also includes the fees, but the LockManager has not received them.

Impact

This will lead to user funds being locked forever as unlock function does not check for the contract’s balance and the .transfer will fail silently.

Recommendation

Always use SafeERC20 and safeTransfer, instead of .transfer();

Check account balance before and after transfers for Fee on Transfer tokens in the lock(); function.

Assessed type

Token-Transfer

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