Munchables - 0xMax1mus's results

Lines of code

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

Vulnerability details

Impact

Any Malicious user can update the unlockTime of another user using LockManager.sol::lockOnBehalf function with a very small amount of token/native_currency like 1 wei as _quantity. Which can Impact in updating of user's unlockTime.This will no longer allows the user to call LockManager.sol::unlock at their original unlockTime Because it got updated due to the Malicious user Action. If the Malicious user continuous to update the innocent user's unlockTime with small amounts like 1 wei even just before the current unlockTime ends, then the user's originally lockedquantity can no longer be unlocked, it will in the contract forever. It will severely effects the integrity of the system.

Proof of Concept

Function :- lockOnBehalf( address _tokenContract,uint256 _quantity,address _onBehalfOf)

User1 calls -> lockOnBehalf ( token/native currency , quantity , User2)
 
lockDuration of User2 = _lockDuration 
lastLockTime of User2 = (block.timestamp) 
unlockTime of User2 =  (block.timestamp + _lockDuration) 

Lets Assume _lockDuration is 100 secs 
on  90th sec 

Malicious User3 calls -> lockOnBehalf ( token/native currency , quantity , User2)
(tokens/native currency quantity from Malicious user can be very small amount like 1 wei)

lockDuration of User2 = _lockDuration
lastLockTime of User2 = (block.timestamp) = 90 secs
unlockTime of User2 = (block.timestamp + _lockDuration) = 90 secs + 100 secs = 190 secs

Malicious User3 Manipulated unlockTime of User2 
User2 can no longer unlock at 100 sec, If Malicious User3 repeat this every time then User2 funds can be 
locked forever. 

// Line no. 275
@-->  function lockOnBehalf(
        address _tokenContract,
        uint256 _quantity, // @audit can call this function with 1 wei
        address _onBehalfOf
    )
        external
        payable
        notPaused
        onlyActiveToken(_tokenContract)
        onlyConfiguredToken(_tokenContract)
        nonReentrant
    {
        address tokenOwner = msg.sender;
        address lockRecipient = msg.sender;
        if (_onBehalfOf != address(0)) {
            lockRecipient = _onBehalfOf;
        }

        _lock(_tokenContract, _quantity, tokenOwner, lockRecipient);
    }

// Line no 379-384
        lockedToken.remainder = remainder;
        lockedToken.quantity += _quantity;
        lockedToken.lastLockTime = uint32(block.timestamp);
      // @audit-issue: Manipulating the lockrecepient  
 @-->   lockedToken.unlockTime =
            uint32(block.timestamp) +
            uint32(_lockDuration);

Tools Used

• Manual Analysis

Recommended Mitigation Steps

• Adding up a LockID for every lock created to a user, to keep track of all locks of an user, allowing individual unlockTime for each LockID will eventually solves the above issue.Which will allows the user to unlock their originally locked funds and can avoid the funds from malicious user. Any malicious user can no longer manipulates other user's unlockTime

Assessed type

Timing