Skip to content

Commit d9004ed

Browse files
committed
Add extra DOS attack vector
1 parent b2a460d commit d9004ed

File tree

1 file changed

+79
-4
lines changed

1 file changed

+79
-4
lines changed

README.md

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Title: Solidity Security: Comprehensive list of known attack vectors and common anti-patterns
22
Date: 2018-05-30 10:20
3-
Modified: 2018-10-10 12:00
3+
Modified: 2018-10-20 14:00
44
Category: Ethereum
55
Tags: ethereum, solidity, security
66
Slug: solidity-security
@@ -1229,7 +1229,82 @@ This category is very broad, but fundamentally consists of attacks where users c
12291229

12301230
There are various ways a contract can become inoperable. Here I will only highlight some potentially less-obvious Blockchain nuanced Solidity coding patterns that can lead to attackers performing DOS attacks.
12311231

1232-
**1. Looping through externally manipulated mappings or arrays** - In my adventures I've seen various forms of this kind of pattern. Typically it appears in scenarios where an `owner` wishes to distribute tokens amongst their investors, and do so with a `distribute()`-like function as can be seen in the example contract:
1232+
**1. External calls without gas stipends** - It may be the case that you wish
1233+
to make an external call to an unknown contract and continue processing the
1234+
transaction regardless whether that call fails or not. Typically this is
1235+
achieved by using the `CALL` opcode, which does not revert the transaction
1236+
if the call fails (see [Unchecked CALL Return Values](#unchecked-calls) for further details and examples).
1237+
Let us consider a simple example, where we have a contract wallet, that slowly
1238+
trickles out ether when the `withdraw()` function is called. A `partner` can
1239+
add their address and spend gas to call the withdraw, giving both the
1240+
`partner` and the `owner` 1% of the total contract balance.
1241+
1242+
```solidity
1243+
contract TrickleWallet {
1244+
1245+
address public partner; // withdrawal partner - pay the gas, split the withdraw
1246+
address public constant owner = 0xA9E;
1247+
uint timeLastWithdrawn;
1248+
mapping(address => uint) withdrawPartnerBalances; // keep track of partners balances
1249+
1250+
function setWithdrawPartner(address _partner) public {
1251+
require(partner == '0x0' || msg.sender == partner);
1252+
partner = _partner;
1253+
}
1254+
1255+
// withdraw 1% to recipient and 1% to owner
1256+
function withdraw() public {
1257+
uint amountToSend = address(this).balance/100;
1258+
// perform a call without checking return
1259+
// the recipient can revert, the owner will still get their share
1260+
partner.call.value(amountToSend)();
1261+
owner.transfer(amountToSend);
1262+
// keep track of last withdrawal time
1263+
timeLastWithdrawn = now;
1264+
withdrawPartnerBalances[partner] += amountToSend;
1265+
}
1266+
1267+
// allow deposit of funds
1268+
function() payable {}
1269+
1270+
// convenience function
1271+
function contractBalance() view returns (uint) {
1272+
return address(this).balance;
1273+
}
1274+
}
1275+
```
1276+
1277+
Notice that on line \[17\] we perform an external call sending 1% of the
1278+
contract balance to a user-specified account. The reason the `CALL` opcode is used, is to ensure that
1279+
the owner still gets paid, even if the external call reverts. The issue is that
1280+
the transaction will send all of its gas (in reality, only most of the transaction gas is sent, some is left to finish processing the call) to the external call. If the user were malicious they could create a contract that would consume all the gas, and force all transactions to `withdraw()` to fail, due to running out of gas.
1281+
1282+
For example, consider the following malicious contract that consumes all gas,
1283+
```solidity
1284+
contract ConsumeAllGas {
1285+
function () payable {
1286+
// an assert consumes all transaction gas, unlike a
1287+
//revert which returns the remaining gas
1288+
assert(1==2);
1289+
}
1290+
}
1291+
```
1292+
If a withdrawal partner decided they didn't like the owner of the contract.
1293+
They could set the partner address to this contract and lock all the funds in
1294+
the `TrickleWallet` contract forever.
1295+
1296+
To prevent such DOS attack vectors, ensure a gas stipend is specified in an
1297+
external call, to limit the amount of gas that that transaction can use. In our
1298+
example, we could remedy this attack by changing line \[17\] to:
1299+
```solidity
1300+
partner.call.gas(50000).value(amountToSend)();
1301+
```
1302+
This modification allows only 50,000 gas to be spent on the external
1303+
transaction. The `owner` may set a gas price larger than this, in order to have
1304+
their transaction complete, regardless of how much the external transaction
1305+
uses.
1306+
1307+
**2. Looping through externally manipulated mappings or arrays** - In my adventures I've seen various forms of this kind of pattern. Typically it appears in scenarios where an `owner` wishes to distribute tokens amongst their investors, and do so with a `distribute()`-like function as can be seen in the example contract:
12331308

12341309
```solidity
12351310
contract DistributeTokens {
@@ -1256,7 +1331,7 @@ contract DistributeTokens {
12561331

12571332
Notice that the loop in this contract runs over an array which can be artificially inflated. An attacker can create many user accounts making the `investor` array large. In principle this can be done such that the gas required to execute the for loop exceeds the block gas limit, essentially making the `distribute()` function inoperable.
12581333

1259-
**2. Owner operations** - Another common pattern is where owners have specific privileges in contracts and must perform some task in order for the contract to proceed to the next state. One example would be an ICO contract that requires the owner to `finalize()` the contract which then allows tokens to be transferable, i.e.
1334+
**3. Owner operations** - Another common pattern is where owners have specific privileges in contracts and must perform some task in order for the contract to proceed to the next state. One example would be an ICO contract that requires the owner to `finalize()` the contract which then allows tokens to be transferable, i.e.
12601335
``` solidity
12611336
bool public isFinalized = false;
12621337
address public owner; // gets set somewhere
@@ -1279,7 +1354,7 @@ function transfer(address _to, uint _value) returns (bool) {
12791354
```
12801355
In such cases, if a privileged user loses their private keys, or becomes inactive, the entire token contract becomes inoperable. In this case, if the `owner` cannot call `finalize()` no tokens can be transferred; i.e. the entire operation of the token ecosystem hinges on a single address.
12811356

1282-
**3. Progressing state based on external calls** - Contracts are sometimes written such that in order to progress to a new state
1357+
**4. Progressing state based on external calls** - Contracts are sometimes written such that in order to progress to a new state
12831358
requires sending ether to an address, or waiting for some input from an
12841359
external source. These patterns can lead to DOS attacks, when the external
12851360
call fails or is prevented for external reasons. In the example of sending

0 commit comments

Comments
 (0)