Pelz - Excess ETH sent in AvailBridge::sendMessage function not refunded back to the users

[sherlock-admin]

Pelz

high

Excess ETH sent in AvailBridge::sendMessage function not refunded back to the users

Summary

The sendMessage function in the AvailBridge contract lacks proper validation, allowing users to send excess Ether without receiving a refund. This oversight could lead to potential loss of funds.

Vulnerability Detail

The vulnerability arises from the conditionif (msg.value < getFee(length)), which checks if the sent Ether is less than the calculated fee based on the length of the data field. However, it fails to restrict users from sending excess Ether, potentially resulting in unrecoverable losses.

reference: https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L305-L308

Impact

The impact of this vulnerability is significant as it exposes users to the risk of losing funds. Without proper validation, users can send more Ether than the required fee, and this excess Ether may not be refunded.

Code Snippet

    function sendMessage(bytes32 recipient, bytes calldata data) external payable whenNotPaused {
        uint256 length = data.length;
        if (length >= MAX_DATA_LENGTH) {
            revert ExceedsMaxDataLength();
        }
        // ensure that fee is above minimum amount
        if (msg.value < getFee(length)) {
            revert FeeTooLow();
        }
        uint256 id;
        unchecked {
            id = messageId++;
        }
        fees += msg.value;
        Message memory message = Message(
            MESSAGE_TX_PREFIX, bytes32(bytes20(msg.sender)), recipient, ETH_DOMAIN, AVAIL_DOMAIN, data, uint64(id)
        );
        // store message hash to be retrieved later by our light client
        isSent[id] = keccak256(abi.encode(message));

        emit MessageSent(msg.sender, recipient, id);
    }

Tool used

Manual Review

Recommendation

Implement proper validation to restrict users from sending excess Ether. Modify the condition to ensure that only the exact required fee is accepted, preventing potential loss of funds. You can modify it like this:

 // Calculate the required fee
    uint256 requiredFee = getFee(length);
// Ensure that the sent Ether is exactly equal to the required fee
    require(msg.value == requiredFee, "Incorrect fee amount");

[sherlock-admin]

2 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invalid. User mistake. See Sherlock documentation

takarez commented:

valid because { valid and a duplicate of issue 013}

[DevPelz]

I think this finding is important because the protocol needs to guide how users interact to stop them from losing money when it could be avoided.