Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TOB-FUEL-6: Unable to retrieve Ether from payable contracts #55

Closed
xgreenx opened this issue Aug 26, 2023 · 0 comments · Fixed by #73
Closed

TOB-FUEL-6: Unable to retrieve Ether from payable contracts #55

xgreenx opened this issue Aug 26, 2023 · 0 comments · Fixed by #73
Labels
audit-report Related to the audit report

Comments

@xgreenx
Copy link
Contributor

xgreenx commented Aug 26, 2023

Description

The Fuel ERC20 gateway contracts are able to receive Ether without the possibility of retrieving it.
The deposit, depositWithData and finalizeWithdrawal functions of the ERC20 gateway are all marked as payable, accepting Ether as payment.

Figure 6.1: The deposit function in fuel-v2-contracts/contracts/messaging/gateway/FuelERC20Gateway.sol

106    /// @dev Made payable to reduce gas costs
107    function deposit(bytes32 to, address tokenId, bytes32 fuelTokenId, uint256
amount) external payable whenNotPaused {
108       bytes memory messageData = abi.encodePacked(
109           fuelTokenId,
110           bytes32(uint256(uint160(tokenId))),
111           bytes32(uint256(uint160(msg.sender))), //from
112           to,
113           bytes32(amount)
114       );
115       _deposit(tokenId, fuelTokenId, amount, messageData);
116    }

Even though the contracts are not designed to handle Ether payments, the comment suggests that this has been done for gas optimization purposes. However, there exists no logic that would allow Ether that was accidentally sent to the contract to be retrieved.

Exploit Scenario

Alice calls the deposit function to deposit 1 WETH to the Fuel chain, but she accidentally also sends 1 Ether with the transaction. The transaction succeeds, and her WETH will be bridged. However, Alice is unable to retrieve the 1 Ether.

Recommendations

Short term, mark the functions as non-payable or include logic that would allow Ether to be retrieved.

Long term, carefully weigh the use of gas cost saving mechanisms against the footguns that such optimizations introduce. Protecting the user should be given priority over saving a small amount of gas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-report Related to the audit report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant