configuredTokenContracts can be blocked from updating unlockTime in setLockDuration

[c4-bot-10]

Lines of code

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

Vulnerability details

Impact

Some configuredTokenContracts will not be able to update unlockTime when setLockDuration gets called.

Proof of Concept

setLockDuration is updating unlockTime for configuredTokenContracts array according to _duration which seems fine, until a revert is done at L260:

    if (
            uint32(block.timestamp) + uint32(_duration) <
            lockedTokens[msg.sender][tokenContract].unlockTime
        ) {
            revert LockDurationReducedError();
        }

This condition will revert if unlockTime greater than _duration + block.timestamp, which is a check must be done to make sure that _duration doesn't cross unlockTime time.

This revert will block other lockedTokens from able to update unlockTime since they don't violate this check above.

Example:

• we have Tokens A,B,C of lockedTokens.
• A does violate the check:

  uint32(block.timestamp) + uint32(_duration) <
          lockedTokens[msg.sender][tokenContract].unlockTime

• B doesn't and C doesn't.
• the user can't update unlockTime for tokens B and C since a revert is done for token A.
• put in mind that users would like to invest in different tokens with different unlock period of time for each token.

You could assume that by changing _duration the issue is solved, but actually is not, because users could decide to have different unlockTime for each token, and they should be able to pass their _duration differently with each token, but in our case they will be blocked.

Tools Used

Manual Review

Recommended Mitigation Steps

Enable the user to pass array of configuredTokenContracts instead of looping on all of it.

Assessed type

DoS