sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

Udsen - `AvailBridge.receiveETH` FUNCTION DOES NOT PERFORM INPUT VALIDAITON CHECK ON RECIPIENT ADDRESS FOR `address(0)` THUS COULD LEAD TO LOSS OF FUNDS #108

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

Udsen

medium

AvailBridge.receiveETH FUNCTION DOES NOT PERFORM INPUT VALIDAITON CHECK ON RECIPIENT ADDRESS FOR address(0) THUS COULD LEAD TO LOSS OF FUNDS

Summary

The AvailBridge.receiveETH function lacks input validation check for dest address (recipient) for native eth transfers, thus leading to loss of funds.

Vulnerability Detail

The AvailBridge.receiveETH function is used for ETH transfers from Avail to Ethereum. Initially the message is verified against the merkle proof input and if succesful then the value is transferred to the dest address as shown below:

    (bool success,) = dest.call{value: value}(""); //@audit-info - send the eth value to the destination
    if (!success) { //@audit-issue - no check on `message.to != address(0)` and `value != 0`
        revert UnlockFailed();
    }

But the issue here is that there is no input validation check for the dest address for address(0).

Impact

Hence if the recipient address passed in from the message is address(0) the funds transferred via the low-level call function will be lost.

Code Snippet

    function receiveETH(Message calldata message, MerkleProofInput calldata input)
        external
        whenNotPaused
        onlySupportedDomain(message.originDomain, message.destinationDomain)
        onlyTokenTransfer(message.messageType)
        nonReentrant
    {
        (bytes32 assetId, uint256 value) = abi.decode(message.data, (bytes32, uint256));
        if (assetId != ETH_ASSET_ID) {
            revert InvalidAssetId();
        }

        _checkBridgeLeaf(message, input);

        // downcast SCALE-encoded bytes to an Ethereum address
        address dest = address(bytes20(message.to));

        emit MessageReceived(message.from, dest, message.messageId);

        // slither-disable-next-line arbitrary-send-eth,missing-zero-check,low-level-calls
        (bool success,) = dest.call{value: value}("");
        if (!success) {
            revert UnlockFailed();
        }
    }

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

Tool used

VSCode and Manual Review

Recommendation

Hence it is recommended to add an input validation check for dest address in the AvailBridge.receiveETH function before the native eth is transferred. And the transaction should revert if the dest address is address(0). Furthermore it is recommended to add an input validation check for the value variable to verify it is !=0.

sherlock-admin commented 8 months ago

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

takarez commented:

invalid because { invalid: out-of scope}