From 2f6f81640bca9fb358db12b088ff82f9b4f132bd Mon Sep 17 00:00:00 2001 From: Evgeny Medvedev Date: Tue, 14 Nov 2017 02:49:40 +0700 Subject: Add another contract with call to demonstrate re-entrancy vulnerability. Add another contract with call to demonstrate re-entrancy vulnerability as send explicitly sets gas to 2300 by default according to this commit 9ca7472 which makes it impossible to "get multiple refunds" because a non-zero CALL costs at least 9700 gas. This issue is discussed on Ethereum StackExchange https://ethereum.stackexchange.com/questions/30371/send-ether-reentrancy-attack-in-reality-how-could-fallback-function-make-a-mes/30616#30616 --- docs/security-considerations.rst | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst index 337a3d3f..197e80e5 100644 --- a/docs/security-considerations.rst +++ b/docs/security-considerations.rst @@ -72,7 +72,24 @@ The problem is not too serious here because of the limited gas as part of ``send``, but it still exposes a weakness: Ether transfer always includes code execution, so the recipient could be a contract that calls back into ``withdraw``. This would let it get multiple refunds and -basically retrieve all the Ether in the contract. +basically retrieve all the Ether in the contract. In particular, the +following contract will allow an attacker to refund multiple times +as it uses ``call`` which forwards all remaining gas by default: + +:: + + pragma solidity ^0.4.0; + + // THIS CONTRACT CONTAINS A BUG - DO NOT USE + contract Fund { + /// Mapping of ether shares of the contract. + mapping(address => uint) shares; + /// Withdraw your share. + function withdraw() { + if (msg.sender.call.value(shares[msg.sender])()) + shares[msg.sender] = 0; + } + } To avoid re-entrancy, you can use the Checks-Effects-Interactions pattern as outlined further below: -- cgit From 37b06884b26ec55fcc9bad2d6744b09dcf568006 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 12 Dec 2017 03:31:30 +0000 Subject: Correct that ether transfers _can_ always include code execution in re-entrancy example --- docs/security-considerations.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst index 197e80e5..1e2138fa 100644 --- a/docs/security-considerations.rst +++ b/docs/security-considerations.rst @@ -69,8 +69,8 @@ complete contract): } The problem is not too serious here because of the limited gas as part -of ``send``, but it still exposes a weakness: Ether transfer always -includes code execution, so the recipient could be a contract that calls +of ``send``, but it still exposes a weakness: Ether transfer can always +include code execution, so the recipient could be a contract that calls back into ``withdraw``. This would let it get multiple refunds and basically retrieve all the Ether in the contract. In particular, the following contract will allow an attacker to refund multiple times -- cgit