Security Snippets: #6

Munchables is a GameFi project with a twist.

The objective of the game is to earn as many Munch Points as possible. In crypto terms, you could call this "point farming". Built on top of Blast, Munchables leverages the unique on-chain primitives to create a reward-filled journey.

There was a nice High severity finding. Take a look at the next code snippet:

    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);
    }

The protocol allows users to donate ether and/or tokens to another user via a lockOnBehalf function. This function lets the caller specify which address should be the recipient of these funds. However issues arise because the lockOnBehalf deposit resets the receivers lockedToken.unlockTime pushing the users unlock time for that token further back.

Therefore if a user has already locked tokens in the protocol, a malicious user can repeatedly call lockOnBehalf shortly before the current unlockTime and keep delaying the users ability to withdraw their tokens. This is compounded by the fact that the lockOnBehalf function has no minimum _quantity therefore the attacker doesn’t have to give up any of their own tokens to acheive this.

So any time see a function with a onBehalfOf argument you'd better check if it can be used by a malicious user!

Here is a full report:

Link: https://code4rena.com/reports/2024-05-munchables#h-01-malicious-user-can-call-lockonbehalf-repeatedly-extend-a-users-unlocktime-removing-their-ability-to-withdraw-previously-locked-tokens