Lines of code

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

Vulnerability details

Impact

Users can set their own lockDuration to any value by calling the setLockDuration function. The only requirement is that they must not have any locks which are about to expire outside the new window in the future.

for (uint256 i; i < configuredTokensLength; i++) {
    //...
    if (lockedTokens[msg.sender][tokenContract].quantity > 0) {
        //...
        if (uint32(block.timestamp) + uint32(_duration) < lockedTokens[msg.sender][tokenContract].unlockTime) {
            revert LockDurationReducedError(); //@audit rejects setting if it effectively lowers any lock duration
        }
    }
}

So if a lock is about to expire in 10 days, a user cannot set the new _duration to less than 10 days. This is done to make sure that the locks are not shortened in any way.

The issue is that other users can open locks on behalf of this user via the lockOnBehalf function. So they can open new locks, which will prevent the victim from shortening their duration.

Since any user can stop other users from reducing their lock duration, this can be considered a high severity issue.

Proof of Concept

Say Alice is a player, and has her current lock duration set to 10 days. She wants to reduce it to 5 days.

Alice has no locks active. So she should be able to call the setLockDuration function with a new _duration of 5 days.

However, Bob calls the lockOnBehalf function with Alice's address and locks some tokens for 10 days. Now, when Alice tries to call the setLockDuration function with a new _duration of 5 days, the transaction will revert. This is because the lockedTokens[msg.sender][tokenContract].unlockTime is set to be 10 days away, but block.timestamp + _duration is only 5 days away.

So Alice is unable to reduce her lock duration.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider disabling the lockOnBehalf function. Or, add a check so that only a set of addresses approved by the user can use lockOnBehalf for the target user.

Assessed type

Access Control

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

Lowering `lockDuration` can be blocked by any user #9