Munchables - TheFabled's results

Lines of code

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

Vulnerability details

Description

The lockOnBehalf function allows any user to lock tokens on behalf of another user. A bad actor can exploit this function to perpetually extend the unlock time of a legitimate user's locked tokens through repeated small deposits. This action can result in a denial of service (DoS) for two critical functions: unlock and setLockDuration, thereby locking the user's funds indefinitely.

Impact

A malicious actor can repeatedly lock small amounts of tokens (even as small as 1 wei) on behalf of a legitimate user, causing the unlock time of all previously locked tokens to be extended with each new deposit. This malicious activity can effectively prevent the legitimate user from ever accessing their tokens or setting a new lock duration, leading to permanent loss of funds and functionality.

Proof of Concept

1. Initial lock by a legitimate user:

   
   // Target user’s tokens already locked
   
   address target = 0x123...;
   
   uint256 initialLockedAmount = 1000 * 10**18;
   
   uint32 initialLockTime = uint32(block.timestamp);
   
   lockedTokens[target][tokenAddress] = LockedToken({
   
       quantity: initialLockedAmount,
   
       remainder: 0,
   
       lastLockTime: initialLockTime,
   
       unlockTime: initialLockTime + 30 days
   
   });
   

2. Attacker's action to extend the lock period:

   
   // Attacker locks 1 wei of tokens on behalf of the target user
   
   lockManager.lockOnBehalf(tokenAddress, 1, target);
   

   
   // Attacker extends 
   
   function _lock(    
   
    @>    lockedToken.unlockTime = 
           uint32(block.timestamp) +
           uint32(_lockDuration);
   

3. Resulting state:

   • The lockedTokens[target][tokenAddress] will now aggregate the new quantity.

   • The unlockTime is reset based on the current block time plus the lock duration in playerSettings.

   • The unlock time for the legitimate user's tokens is now extended indefinitely each time a small deposit is made.

Impact on Critical Functions

Unlock Function:

function unlock(

    address _tokenContract,

    uint256 _quantity

) external notPaused nonReentrant {

    LockedToken storage lockedToken = lockedTokens[msg.sender][_tokenContract];

    if (lockedToken.quantity < _quantity) revert InsufficientLockAmountError();

 @>   if (lockedToken.unlockTime > uint32(block.timestamp)) revert 
      TokenStillLockedError(); // Tokens will never be available for unlocking 
      due to extended unlockTime

    accountManager.forceHarvest(msg.sender);

    lockedToken.quantity -= _quantity;

    if (_tokenContract == address(0)) {

        payable(msg.sender).transfer(_quantity);

    } else {

        IERC20 token = IERC20(_tokenContract);

        token.transfer(msg.sender, _quantity);

    }

    emit Unlocked(msg.sender, _tokenContract, _quantity);

}

Set Lock Duration Function:

function setLockDuration(uint256 _duration) external notPaused {

    if (_duration > configStorage.getUint(StorageKey.MaxLockDuration)) revert MaximumLockDurationError();

    playerSettings[msg.sender].lockDuration = uint32(_duration); 

    uint256 configuredTokensLength = configuredTokenContracts.length;

    for (uint256 i; i < configuredTokensLength; i++) {

        address tokenContract = configuredTokenContracts[i];

        if (lockedTokens[msg.sender][tokenContract].quantity > 0) {

   @>       if (uint32(block.timestamp) + uint32(_duration) < 
   @>           lockedTokens[msg.sender][tokenContract].unlockTime) {

   @>           revert LockDurationReducedError();
            // Will always revert if owner is trying to set a lower _duration. 
            }

            lockedTokens[msg.sender][tokenContract].unlockTime = uint32(block.timestamp) + uint32(_duration);

        }

    }

    emit LockDuration(msg.sender, _duration);

}

Tools Used

• Manual Review

Recommended Mitigation Steps

Approval Mechanism: Allow users to approve specific addresses that can lock tokens on their behalf, preventing unauthorized locking actions.

```solidity

mapping(address => mapping(address => bool)) public isApprovedLocker;



function approveLocker(address _locker) external {

    isApprovedLocker[msg.sender][_locker] = true;

}



function revokeLocker(address _locker) external {

    isApprovedLocker[msg.sender][_locker] = false;

}



function lockOnBehalf(

    address _tokenContract,

    uint256 _quantity,

    address _onBehalfOf

) external payable notPaused onlyActiveToken(_tokenContract) onlyConfiguredToken(_tokenContract) nonReentrant {

    require(isApprovedLocker[_onBehalfOf][msg.sender], "Caller not approved");

    _lock(_tokenContract, _quantity, msg.sender, _onBehalfOf);

}

```

Assessed type

Access Control