The function unlock transfers ERC20 tokens without checking the return value from transfer function.
If the LockManager is ever used with a token contract that does return false on error instead of reverting, the unlock function would decrease the locked token quantity without actually having transferred the tokens in case of an error during transfer.
Proof of Concept
While the latest ERC20 tokens should revert on failure, not checking the return value could potentially allow silent failures in transfers as the original ERC20 spec allowed returning false in case of an error.
Lines of code
https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L422-L423
Vulnerability details
Impact
The function
unlock
transfers ERC20 tokens without checking the return value fromtransfer
function.If the LockManager is ever used with a token contract that does return false on error instead of reverting, the
unlock
function would decrease the locked token quantity without actually having transferred the tokens in case of an error duringtransfer
.Proof of Concept
While the latest ERC20 tokens should revert on failure, not checking the return value could potentially allow silent failures in transfers as the original ERC20 spec allowed returning false in case of an error.
See LockManager.sol:422-423.
Tools Used
Unreleased AI audit tool.
Recommended Mitigation Steps
Check the
transfer
function's return value or use OpenZeppelin's SafeERC20.Assessed type
ERC20