No minimum lock duration could cause loss of harvestedSchnibbles

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Description

The user can set a lock duration on how long he wants to lock his tokens to the contract. The function only checks if the duration input is lesser than the maxlockduration (line 246) and the duration can be set already (see line 265-267).

File: LockManager.sol
245:     function setLockDuration(uint256 _duration) external notPaused { //@audit this should have minimal duration
246:         if (_duration > configStorage.getUint(StorageKey.MaxLockDuration))
247:             revert MaximumLockDurationError();
248: 
249:         playerSettings[msg.sender].lockDuration = uint32(_duration);
~continue
265:                 lockedTokens[msg.sender][tokenContract].unlockTime =
266:                     lastLockTime +
267:                     uint32(_duration);
268:             }
269:         }

And then, here is the unlock function below, in which the user can unlock his token as long as the blocktimestamp is more than the unlocktime (line 409).

File: LockManager.sol
400:     function unlock(
401:         address _tokenContract,
402:         uint256 _quantity
403:     ) external notPaused nonReentrant {
404:         LockedToken storage lockedToken = lockedTokens[msg.sender][
405:             _tokenContract
406:         ];
407:         if (lockedToken.quantity < _quantity)
408:             revert InsufficientLockAmountError();
409:         if (lockedToken.unlockTime > uint32(block.timestamp))
410:             revert TokenStillLockedError();
411: 
412:         // force harvest to make sure that they get the schnibbles that they are entitled to
413:         accountManager.forceHarvest(msg.sender);
414: 
~continue
424: 
425:         emit Unlocked(msg.sender, _tokenContract, _quantity);
426:     }
427: 

The Issue

As you can see on line 413 above as part of unlock function, there is a forceHarvest called in contract accountManager. This is where the schnibbles are being harvested.

See below the detail of _harvest function as part of forceHarvest. Let's focus on line 378-379. You will notice there is 1 days denominator which is equivalent to 86,400. If the numerator (dailySchnibbles * secondsToClaim) did not reached 86,400 , the result of harvestedSchnibbles will be zero.

This could be a problem if the user decided to unlock his tokens in short period of time (few secondstoclaim) and at same time daily schnibble figure is also few in which if you multiply both figures is still not enough to reach 86,400.

File: AccountManager.sol
372:     function _harvest(address _caller) private returns (uint256 _harvested) {
373:         (uint256 dailySchnibbles, uint256 bonus) = getDailySchnibbles(_caller);
374:         dailySchnibbles += ((dailySchnibbles * bonus) / 1e18);
375: 
376:         uint256 secondsToClaim = block.timestamp -
377:             players[_caller].lastHarvestDate;
378:         uint256 harvestedSchnibbles = (dailySchnibbles * secondsToClaim) /
379:             1 days;
380: 
381:         players[_caller].unfedSchnibbles += harvestedSchnibbles;
382:         players[_caller].lastHarvestDate = uint32(block.timestamp);
383: 
384:         _harvested = harvestedSchnibbles;
385: 
386:         emit Harvested(_caller, harvestedSchnibbles);
387:     }

Impact

Lost of harvested schnibbles

Proof of Concept

Consider this scenario.

1. User A decided to lock this tokens to the contract within just a short period of time. Let's say 10,000 seconds.
2. After 10,000 seconds, User A decided to unlock his tokens. Let's say the computed sample daily schnibble is only 5, we need to multiply it to 10,000 seconds. Why 10,000 seconds? because this is considered the secondsToClaim, difference between last harvest date (date of locking token) until the unlocking date.
3. The numerator is (5*10,000) = 50,000 and then divided by 86,400, the harvestedSchnibbles will be zero.

376:         uint256 secondsToClaim = block.timestamp -
377:             players[_caller].lastHarvestDate;
378:         uint256 harvestedSchnibbles = (dailySchnibbles * secondsToClaim) /
379:             1 days;

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a minimum lock duration of 86,400 or 1 day to function setLockDuration in order to be aligned with the formula of harvest function in account manager.

Assessed type

Other

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid