Excess funds are lost when using sendMessage

Summary

sendMessage is used to bridge a transaction of arbitrary data to a given recipient. A fee is calculated but if excess Ether is sent, it will not be returned.

Vulnerability Detail

The user can send an arbitrary amount of Ether to sendMessage and there is a single check to determine that it exceeds the required fee. No excess is returned.

Impact

Since onAvailMessage is not payable, it is assumed that the recipient does not get these funds with the message. The sender will pay a higher fee for no apparent reason.

Code Snippet

https://github.com/sherlock-audit/2023-12-avail/blob/1afb56b8d4dfbf5d3f21bed0ddf80a04730204b5/contracts/src/AvailBridge.sol#L300-L321

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

Calculate the excess fee paid and return it.

sherlock-audit / 2023-12-avail-judging

m4ttm - Excess funds are lost when using sendMessage #121