Handling Ether in a smart contract.

[0xSandyy]

Checklist

• I have searched the existing issues and pull requests for duplicates.

Type of Issue

• New vulnerability addition
• Feature request
• Update existing vulnerability

Description

Handling Ether

Ether is the native cryptocurrency of the Ethereum network. It serves as the fuel that powers the execution of smart contracts, enables transactions, and facilitates interactions within the Ethereum ecosystem. The following things should be considered while handling ether:

Use of Call / Send / Transfer while transferring tokens

The transfer() and send() functions forward a fixed amount of 2300 gas. Historically, it has often been recommended to use these functions for value transfers to guard against reentrancy attacks. However, the gas cost of EVM instructions may change significantly during hard forks which may break already deployed contract systems that make fixed assumptions about gas costs. For example. EIP 1884 broke several existing smart contracts due to a cost increase of the SLOAD instruction. Additionally, using higher than 2300 gas might be mandatory for some multisig wallets. Thus, using call() and checking for success is recommended but transferring ether to any untrusted address may cause gas-griefing due to unbounded return data issue.

Accounting for selfdestruct to forcibly send ether

The first assumption that is made with handling ether in a smart contract is ether can only be sent to a contract if the contract contains either a receive() or a fallback() function and these functions can be modified to track the incoming balances but this is not the only way. selfdestruct sends all remaining Ether stored in the contract to a designated address before destroying a contract. Example. Thus an attacker can use selfdestruct to send ether to a contract with no receive() or fallback() function to mess up the internal accounting if contract strictly relies on address(this).balance.

Locked Ether

The locked ether vulnerability can occur when a smart contract accepts ether but does not provide a way for users to withdraw it or having no way to recover excess ether that might be locked to due faulty business logic or some users accidentally sending ether to the contract with receive()/fallback() function.

Implement a withdraw or recover function to recover excess ether from the contract.

    function withdraw(uint256 amount) public {
        require(msg.sender == owner, "Only the contract owner can withdraw the ether.");
        (bool success,) = owner.call{value: amount}("");
        require(success, "Failed to withdraw ether.");
    }

Pull vs Push pattern for sending ether

While transferring ether in a loop, It must be considered that if any one of the iterations of sending ether to a recipient fails, the whole function reverts. This call to transfer tokens can fail due to various reasons like gas-griefing, recipient not implementing fallback(), malicious recipient force reverting fallback(), etc.
If there is no way to withdraw funds from the contract other than looping through a pre-configured list of recipients, this might cause a serious problem. Therefore, a push pattern should be used. It can be implemented by adding a function from which valid recipients can withdraw funds themselves.

    function withdraw() public {
        uint256 amount = withdrawableBalances[msg.sender];

        require(amount != 0);
        require(address(this).balance >= amount);

        withdrawableBalances[msg.sender] = 0;

        (bool success,) = msg.sender.call{value: amount}("");
        require(success);
    }

Using msg.value in a loop

The particularity with msg.value is that msg.value is set on every contract call and not on the transaction level. Thus, if the function with loop is transferring msg.value on every iteration, all the msg.value sent to the function is transferred in the first iteration and other iteration will proceed with msg.value = 0 which may cause unexpected consequences.
Also, if a function has a check like msg.value > 0.1e18, that function can be called multiple times in a same transaction as msg.value value is not subtracted unless it's transferred out of the contract.

Sources

• https://solidity-by-example.org/hacks/self-destruct/
• https://trustchain.medium.com/ethereum-msg-value-reuse-vulnerability-5afd0aa2bcef

[kadenzipfel]

This is kinda more a list of multiple different vulnerabilities and heuristics, one or two of which are kinda already included, so not sure how this fits in. I do like "Using msg.value in a loop" though, perhaps that would be worth making into it's own listed vulnerability?

[0xSandyy]

The main idea behind listing like this is this includes all the vulnerabilities regarding handling ether and msg.value in a single document. Some of these are already included but i wanted to provide a single document mentioning the assumptions, how they can be exploited and their fixes and I thought this would make reading about similar vulnerabilities more easier and intuitive.

I can see how this would not fit in the existing repo as this feels more like a blog post. Yeah, I will make msg.value a seperate one and what about self destruct one? I think I saw force feeding one but it doesn't explicitly mention self destruct.

[kadenzipfel]

I can see how this would not fit in the existing repo as this feels more like a blog post. Yeah, I will make msg.value a seperate one and what about self destruct one? I think I saw force feeding one but it doesn't explicitly mention self destruct.

If force feeding doesn't mention selfdestruct usage then we should probably add it under that vuln imo

[indeqs]

Author of the force-feeding issue here, the reason why I didn't include selfdestruct as method to forcibly send ether to a contract is because selfdestruct is a deprecated opcode as seen in the file at: use-of-deprecated-functions.md

[0xSandyy]

It is depreciated in 0.8.18 which is relatively new and a lot of contracts use version >=0.8.0 to 0.8.18. Also, selfdestruct will be used to send some amount of tokens only for force feeding and it has not been deactivated yet and can still be used for force feeding ether.

If EIP-4758 gets implemented and selfdestruct gets completely unsupported, the selfdestruct can just be replaced by new SENDALL which does the same thing for force feeding.

[kadenzipfel]

Agree with @0xSandyy here, as long as it is possible to execute an attack it should be listed. Even if selfdestruct gets unsupported entirely on some networks it's worth still including for the few that will likely still support it