User loss previous amount

Summary

On extending the lock, user will lose all the amount which he has gathered before calling the extendLock function. This is happening due to an incorrect logic

Vulnerability Detail

User has deposited amount 200 for duration 20 seconds for which mint amount was 100
Post 10 seconds user decides to use extendLock function to extend by further 5 seconds
This makes the duration as below

uint256 duration = maxLockDuration.min(uint256(userDeposit.end - block.timestamp) + increaseDuration);
// duration = maxLockDuration.min(20-10)+5)=15

uint256 mintAmount = userDeposit.amount * getMultiplier(duration) / 1e18;
// 200*getMultiplier(15)/1e18 = 60

depositsOf[_msgSender()][_depositId].shareAmount =  mintAmount;
//depositsOf[_msgSender()][_depositId].shareAmount =  60

So new mint Amount came become 60 and old was 100. But this mint Amount was calculated for last 15 seconds which does not care of mint Amount from initial 10 seconds. The mint Amount of 100-60=40 which user for for the past 10 seconds should be withdrawn to user account

Impact

User will lose funds

Code Snippet

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/TimeLockPool.sol#L165 https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/TimeLockPool.sol#L148

Tool used

Manual Review

Recommendation

Revise the calculation as shown (or simply withdraw the difference mint amount):

uint256 duration = maxLockDuration.min(increaseDuration);
...
if (mintAmount > userDeposit.shareAmount) {
            depositsOf[_msgSender()][_depositId].shareAmount+ =  mintAmount;
            _mint(_msgSender(), mintAmount);
        }
...

sherlock-audit / 2022-10-merit-circle-judging

csanuragjain - User loss previous amount #20