sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

0xC - Unvalidated Input Decoding in `receiveETH` Function in `AvailBridge` Contract #45

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

0xC

high

Unvalidated Input Decoding in receiveETH Function in AvailBridge Contract

Summary

The receiveETH function in the AvailBridge contract suffers from a vulnerability similar to the one found in the receiveAVAIL function. It lacks proper validation of the message.data parameter before decoding it using abi.decode, leaving the contract exposed to potential malicious inputs.

Vulnerability Detail

In the receiveETH function, the following code snippet is used to decode the message.data:

(bytes32 assetId, uint256 value) = abi.decode(
    message.data,
    (bytes32, uint256)
);

There is no validation or check performed on the message.data parameter before decoding it. This lack of validation means that if an attacker crafts a message with unexpected data types or incorrect data, the contract will proceed with decoding, potentially leading to unintended behavior or vulnerabilities.

Impact

The impact of this vulnerability could be severe, as it may allow an attacker to exploit the contract by providing malicious input data. This could lead to unauthorized transfers of Ether (ETH) or other unexpected behaviors.

Code Snippet

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

I wrote my own unit test that proves my concept, so the judge can copy/paste it in AvailBridgeTest.t.sol and run it:


    function testInvalidMessageDataInReceiveETH(
        bytes32 rangeHash,
        bytes32 from,
        // uint256 amount,
        uint64 messageId,
        string calldata myString,
        string calldata myOtherString
    ) external {

        address to = makeAddr("to");

        // Create a message with invalid data (not containing (bytes32, uint256))
        IAvailBridge.Message memory message = IAvailBridge.Message(
            0x02,
            from,
            bytes32(bytes20(to)),
            1,
            2,
            abi.encode(myString, myOtherString),  // Invalid data format
            messageId
        );

        // Calculate the expected message hash
        bytes32 messageHash = keccak256(abi.encode(message));

        bytes32[] memory emptyArr;

        // Prepare the MerkleProofInput
        IAvailBridge.MerkleProofInput memory input = IAvailBridge.MerkleProofInput(
            emptyArr,
            emptyArr,
            rangeHash,
            0,
            bytes32(0),
            messageHash,
            messageHash,
            0
        );

        // Expect a revert with InvalidAssetId selector
        vm.expectRevert(IAvailBridge.InvalidAssetId.selector);

        // Call the receiveAVAIL function
        bridge.receiveETH(message, input);

        // Assert that the message is not bridged
        assertFalse(bridge.isBridged(messageHash));
    }

The testInvalidMessageDataInReceiveETH tests the behavior of the receiveETH function if message.data contains two strings, instead of bytes32 and uint256.

Tool used

Manual Review

Recommendation

To mitigate this vulnerability, it is strongly recommended to add validation and checks on the message.data parameter before attempting to decode it using abi.decode. Specifically, ensure that the data conforms to the expected (bytes32, uint256) format and handle any deviations appropriately, such as reverting the transaction if the data is invalid. This validation step should be implemented before any further processing of the data to prevent potential exploits.

sherlock-admin commented 8 months ago

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

takarez commented:

invalid because {invalid: same by watson who submitted issue 031}