unlock with partial quantity can cause user loss of rewards.

[c4-bot-1]

Lines of code

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

Vulnerability details

Impact

unlock with partial quantity will reset the last harvest time in account manager. So, the user will lose rewards ...etc.

Proof of Concept

forceHarvest at accountManager.forceHarvest(msg.sender) can be an issue since it updates lastHarvestDate to the current block.timestamp.

A user called unlock with partial _quantity, a forceHarvest is done at:

accountManager.forceHarvest(msg.sender);

and now all the rewards harvested according to secondsToClaim, which is:

block.timestamp - players[_caller].lastHarvestDate

The issue here when a user try to claim the remaining of lockedToken.quantity, there will be no harvest rewards for it, and users now need to wait for a period of time so they can harvest rewards for the remining quantity.

Assuming Bob has lockedToken.quantity = 100e18.

• Bob calls unlock for _quantity as 10e18.
• A forceHarvest is done for 10e18 according to secondsToClaim
• lastHarvestDate now is updated to the current block.timestamp.
• Bob calls unlock again for the remaining _quantity as 90e18, but doesn't harvest any rewards, since forceHarvest relys on lastHarvestDate.

Tools Used

Manual Review

Recommended Mitigation Steps

Pass _quantity to forceHarvest method and update the code.

Assessed type

Other

[evokid]

Hey @alex-ppg, I noticed this is a valid issue, could you please explain why it's invalid, thank you.

[alex-ppg]

Hey @evokid, thanks for your input. This submission is invalid as the time should be appropriately updated whenever a harvest occurs to avoid incorrect reward disbursement. I will not elaborate further, and advise you to investigate the codebase as well as online resources to understand why.