Lock duration can be manipulated

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L257-L258
https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L265-L267

Vulnerability details

Impact

The lock duration can be set to a time before the existing unlock time

Proof of Concept

The lock duration check in setLockDuration only checks if the duration is below the current time + _duration, it however sets unlock time to lastLockTime + _duration

This can allow an user to set an unlock time before the existing unlock time.

~ Alice sets unlock time to time 2000.
Last time is now 1000, and _duration is 1000,

Time is now - 1500
Alice calls setLockDuration again with _duration as 501
The check 1500 + 501 > 2000 passes

However the unlock time would be mistakenly set as 1000 + 501 ~ 1501
allowing Alice to break the invariant and prepone the unlock time

Tools Used

Manual analysis

Recommended Mitigation Steps

use correct check that is
lastLockTime + uint32(_duration) < lockedTokens[msg.sender][tokenContract].unlockTime
instead of
uint32(block.timestamp) + uint32(_duration) < lockedTokens[msg.sender][tokenContract].unlockTime

Assessed type

Other