If the player manages to call setLockDuration() before a token is configurated, he can set 1 second of lockup time and unlock almost immediately everytime he locks during a non lockdrop period. These's also another scenario which he can set 1 second if he doesn't have any locked tokens.
Proof of Concept
Player is monitoring events for Munchable contracts and after initial deployment he manages to call setLockDuration() with _duration = 1 before any call is made to configureToken().
Everytime the player locks any token via lock() during a non lockdrop period, he will be able to unlock almost immediately, e.g. after 2 seconds, since _lockDuration will evaluate to 1 and lockToken.unlockTime will evaluate to block.timestamp + 1. The check for lock.unlockTime will pass in unlock.
This is possible because setLockDuration() won't interate the configurateTokens loop, and the lockup time only reverts when _lockDuration is smaller than configurated minimum during lockup periods in lock().
There's also another scenario which would be for the player to call setLockDuration() with _duration = 1 without having any lock tokens (meaning revert check won't be executed). With this approach, his lockToken.unlockTime will also be very small when evaluated in lock().
Impact
This would allow players to harvest the rewards almost immediately without having to wait for lockup times, and call unlock() right after calling lock().
Tools Used
Manual review.
Recommented Mitigation Steps
One approach could be to check if _duration is not smaller than a default min value in setLockDuration().
Another approach could be to update _lockDuration in lock() to be the value of lockdrop.minLockDuration when _lockDuration is smaller then lockdrop.minLockDuration, because currently it updates if _lockDurationis zero.
Lines of code
https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L249-L269 https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L347 https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L382-L384
Vulnerability details
If the player manages to call
setLockDuration()before a token is configurated, he can set 1 second of lockup time and unlock almost immediately everytime he locks during a non lockdrop period. These's also another scenario which he can set 1 second if he doesn't have any locked tokens.Proof of Concept
setLockDuration()with_duration = 1before any call is made toconfigureToken().lock()during a non lockdrop period, he will be able to unlock almost immediately, e.g. after 2 seconds, since_lockDurationwill evaluate to 1 andlockToken.unlockTimewill evaluate toblock.timestamp + 1. The check forlock.unlockTimewill pass in unlock.This is possible because
setLockDuration()won't interate the configurateTokens loop, and the lockup time only reverts when_lockDurationis smaller than configurated minimum during lockup periods inlock().There's also another scenario which would be for the player to call
setLockDuration()with_duration = 1without having any lock tokens (meaning revert check won't be executed). With this approach, hislockToken.unlockTimewill also be very small when evaluated inlock().Impact
This would allow players to harvest the rewards almost immediately without having to wait for lockup times, and call
unlock()right after callinglock().Tools Used
Manual review.
Recommented Mitigation Steps
One approach could be to check if
_durationis not smaller than a default min value insetLockDuration().Another approach could be to update
_lockDurationinlock()to be the value oflockdrop.minLockDurationwhen_lockDurationis smaller thenlockdrop.minLockDuration, because currently it updates if_lockDurationis zero.Assessed type
Invalid Validation