Lines of code

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

Vulnerability details

Impact

TokenOwner can increase the lock duration of the token Receiver without adding any funds on behalf of the receiver.

Proof of Concept

A player A can send 0 quantity on behalf of another player B. And as a result player B is locked with 0 amount for uint32(block.timestamp) + uint32(_lockDuration) time

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

function lockOnBehalf(
    address _tokenContract,
    uint256 _quantity,
    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);
}

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L311-L399 function _lock( .... ) private {

    (
        address _mainAccount,
        MunchablesCommonLib.Player memory _player
    ) = accountManager.getPlayer(_lockRecipient);
    if (_mainAccount != _lockRecipient) revert SubAccountCannotLockError();
    if (_player.registrationDate == 0) revert AccountNotRegisteredError();
    // check approvals and value of tx matches
    if (_tokenContract == address(0)) {
        if (msg.value != _quantity) revert ETHValueIncorrectError();
    } else {
        if (msg.value != 0) revert InvalidMessageValueError();
        IERC20 token = IERC20(_tokenContract);
        uint256 allowance = token.allowance(_tokenOwner, address(this));
        if (allowance < _quantity) revert InsufficientAllowanceError();
    }

    ....

    lockedToken.remainder = remainder;
    lockedToken.quantity += _quantity;
    lockedToken.lastLockTime = uint32(block.timestamp);
    lockedToken.unlockTime =
        uint32(block.timestamp) +
        uint32(_lockDuration);

    ....
}

Hence , one player can lock another player's fund for indefinite time if they wanted to.

Tools Used

Manual Review

Recommended Mitigation Steps

If the _quantity is 0 , revert

Assessed type

Timing

code-423n4 / 2024-05-munchables-findings

A player can lock another player's fund for indefinite time. #162