Lines of code

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

Vulnerability details

Impact

The function lockOnBehalf allows locking tokens on behalf of another address _onBehalfOf without checking if the caller msg.sender has the necessary permissions or authorization to do so on behalf of the specified address (_onBehalfOf).

If Alice calls lockOnBehalf with Bob's address as the _onBehalfOf parameter, tokens will be locked for Bob's account.

The issue here is that there is no check to ensure that Alice is authorized to lock tokens on behalf of Bob. Anyone can call this function with any address as the _onBehalfOf parameter, locking tokens for accounts they do not have permission to manage.

Proof of Concept

The vulnerable function: src/managers/LockManager.sol#lockOnBehalf

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

Assume Alice wants to lock 100 tokens on behalf of Bob (address 0x...Bob). Alice calls the lockOnBehalf function with the following parameters.

_tokenContract = 0x...TokenContract
_quantity = 100
_onBehalfOf = 0x...Bob

The function will execute without checking if Alice has the necessary permissions to lock tokens on behalf of Bob. As a result, the tokens will be locked, and Bob will not be able to access them without Alice's permission.

Tools Used

Manual Audit

Recommended Mitigation Steps

The function should include a check or a modifier to ensure that the caller (msg.sender) is authorized to lock tokens on behalf of the specified address (_onBehalfOf). This could be achieved by implementing an access control mechanism, such as:

mapping (address => address[]) public approvedAddresses;

function lockOnBehalf(
        address _tokenContract,
        uint256 _quantity,
        address _onBehalfOf
    )
        external
        payable
        notPaused
        onlyActiveToken(_tokenContract)
        onlyConfiguredToken(_tokenContract)
        nonReentrant
    {
        require(approvedAddresses[msg.sender].includes(_onBehalfOf), "Unauthorized to lock on behalf of this address");
        address tokenOwner = msg.sender;
        address lockRecipient = msg.sender;
        if (_onBehalfOf != address(0)) {
            lockRecipient = _onBehalfOf;
        }

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

Adding a mapping or a data structure to store the authorized addresses for each user. Implementing a function or a modifier to check if the caller (msg.sender) is authorized to lock tokens on behalf of the specified address (_onBehalfOf). Updating the lockOnBehalf function to include the authorization check or modifier.

Assessed type

Access Control

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

Unauthorized Token Locking in `lockOnBehalf` Function Allows Malicious Actors to Lock Tokens on Behalf of Other Users Without Permission #358