sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

Jaraxxus - Excess msg.value is not refunded when calling sendMessage, which will affect fees calculation as well #113

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

Jaraxxus

medium

Excess msg.value is not refunded when calling sendMessage, which will affect fees calculation as well

Summary

Excess msg.value is not refunded in sendMessage.

Vulnerability Detail

Users can send a message on Avail by calling sendMessage(). The function checks for the fee amount and adds it into the fee variable. If the user sends more msg.value than intended, the msg.value will not be refunded and the fees amount will be calculated incorrectly.

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

Impact

Users will lose fees. Also, the fees calculation will be wrong.

Code Snippet

https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L300-L322

Tool used

Manual Review

Recommendation

Recommend checking the fee price and having an equivalent sign for msg.value and getFee(length) so that the exact fee is paid.

sherlock-admin commented 8 months ago

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}