Players cannot lock their new funds by maintaining the time commitment.

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

Players cannot lock their new funds by maintaining the time commitment.

Proof of Concept

According to the doc provided https://munchables.gitbook.io/munchables/explorers-guide/glossary/lockdrop , user should be able add new fund to the lock by maintaining their time commitment.

At the end of the _lock function , lockedToken is getting updated with new remainders , quantiy and the new unlockTime.

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

        lockedToken.remainder = remainder;
        lockedToken.quantity += _quantity;
        lockedToken.lastLockTime = uint32(block.timestamp);
        lockedToken.unlockTime =
            uint32(block.timestamp) +
            uint32(_lockDuration);

But its visible that unlockTime will always get updated to the new timestamp which is greater than the current timestamp. There is no check done whether the user wants to change the unlock time by checking whether they have changed their locktime in the playerSettings[_lockRecipient].lockDuration.

Tools Used

Manual review

Recommended Mitigation Steps

Update the unlock time if the user has updated their settings.

Assessed type

Context

[CloudEllie]

Reopening for sponsor review based on Validator input

[0xinsanity]

My mistake on the documentation. This behavior is intentional.

[alex-ppg]

The Warden outlines a potential mistake in the documentation that states a user should be able to add more funds to a lock while maintaining their time commitment. This would be contradictory to the lockdrop's normal operation, as it would permit a user to lock 1 wei for a significantly large duration, lock an abnormally high amount close to the lock duration end, and acquire all the associated benefits.

As the behavior that the documentation outlines would be a vulnerability, and the actual implementation does not follow it for this purpose, I cannot consider the discrepancy to be a valid HM vulnerability.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid