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

0 stars 0 forks source link

Unexpected Revert Behavior in receive() Function and Ether Handling Vulnerability in _lock Function #591

Open c4-bot-7 opened 4 months ago

c4-bot-7 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L93-L95 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L323-L331

Vulnerability details

Impact

The receive() function in the protocol includes a revert statement that can lead to unexpected reverts during the execution of transactions. Additionally, the _lock function, which is tasked with locking tokens or Ether, contains a conditional block that may not execute as intended due to the revert behavior of the receive() function.

Proof of Concept

The receive() function contains a revert statement, revert LockManagerRefuseETHError().

    receive() external payable {
        revert LockManagerRefuseETHError();
    }

The condition (if (_tokenContract == address(0))) in _lock function might never be fulfilled due to the revert behavior of the receive() function.

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

The unreachable condition in the _lock function may prevent the successful execution of critical logic, hindering its ability to lock tokens or Ether as intended.

Tools Used

Manual Review

Recommended Mitigation Steps

Remove the revert statement from the receive() function to ensure proper Ether handling and token locking.

Assessed type

Other