External Calls
Avoid external calls when possible
Calls to untrusted contracts can introduce several unexpected risks or errors. External calls may execute malicious code in that contract or any other contract that it depends upon. As such, every external call should be treated as a potential security risk, and removed if possible. When it is not possible to remove external calls, use the recommendations in the rest of this section to minimize the danger.
Be aware of the tradeoffs between send()
, transfer()
, and call.value()()
When sending Ether be aware of the relative tradeoffs between the use of
someAddress.send()
, someAddress.transfer()
, and someAddress.call.value()()
.
x.transfer(y)
is equivalent toif (!x.send(y)) throw;
Send is the low level counterpart of transfer, and it's advisable to use transfer when possible.someAddress.send()
andsomeAddress.transfer()
are considered safe against reentrancy. While these methods still trigger code execution, the called contract is only given a stipend of 2,300 gas which is currently only enough to log an event.someAddress.call.value()()
will send the provided ether and trigger code execution. The executed code is given all available gas for execution making this type of value transfer unsafe against reentrancy.
Using send()
or transfer()
will prevent reentrancy but it does so at the cost of being
incompatible with any contract whose fallback function requires more than 2,300
gas.
One pattern that attempts to balance this trade-off is to implement both
a push and pull mechanism, using send()
or transfer()
for the push component and call.value()()
for the pull component.
It is worth pointing out that exclusive use of send()
or transfer()
for value transfers
does not itself make a contract safe against reentrancy, but only makes those
specific value transfers safe against reentrancy.
Handle errors in external calls
Solidity offers low-level call methods that work on raw addresses: address.call()
, address.callcode()
, address.delegatecall()
, and address.send
. These low-level methods never throw an exception, but will return false
if the call encounters an exception. On the other hand, contract calls (e.g., ExternalContract.doSomething()
) will automatically propagate a throw (for example, ExternalContract.doSomething()
will also throw
if doSomething()
throws).
If you choose to use the low-level call methods, make sure to handle the possibility that the call will fail, by checking the return value.
// bad
someAddress.send(55);
someAddress.call.value(55)(); // this is doubly dangerous, as it will forward all remaining gas and doesn't check for result
someAddress.call.value(100)(bytes4(sha3("deposit()"))); // if deposit throws an exception, the raw call() will only return false and transaction will NOT be reverted
// good
if(!someAddress.send(55)) {
// Some failure code
}
ExternalContract(someAddress).deposit.value(100);
Don't make control flow assumptions after external calls
Whether using raw calls or contract calls, assume that malicious code will execute if ExternalContract
is untrusted. Even if ExternalContract
is not malicious, malicious code can be executed by any contracts it calls. One particular danger is malicious code may hijack the control flow, leading to race conditions. (See Race Conditions for a fuller discussion of this problem).
Favor pull over push for external calls
External calls can fail accidentally or deliberately. To minimize the damage caused by such failures, it is often better to isolate each external call into its own transaction that can be initiated by the recipient of the call. This is especially relevant for payments, where it is better to let users withdraw funds rather than push funds to them automatically. (This also reduces the chance of problems with the gas limit.) Avoid combining multiple send()
calls in a single transaction.
// bad
contract auction {
address highestBidder;
uint highestBid;
function bid() payable {
if (msg.value < highestBid) throw;
if (highestBidder != 0) {
if (!highestBidder.send(highestBid)) { // if this call consistently fails, no one else can bid
throw;
}
}
highestBidder = msg.sender;
highestBid = msg.value;
}
}
// good
contract auction {
address highestBidder;
uint highestBid;
mapping(address => uint) refunds;
function bid() payable external {
if (msg.value < highestBid) throw;
if (highestBidder != 0) {
refunds[highestBidder] += highestBid; // record the refund that this user can claim
}
highestBidder = msg.sender;
highestBid = msg.value;
}
function withdrawRefund() external {
uint refund = refunds[msg.sender];
refunds[msg.sender] = 0;
if (!msg.sender.send(refund)) {
refunds[msg.sender] = refund; // reverting state because send failed
}
}
}