|
1 | 1 | Title: Solidity Security: Comprehensive list of known attack vectors and common anti-patterns
|
2 | 2 | Date: 2018-05-30 10:20
|
3 |
| -Modified: 2018-07-18 15:00 |
| 3 | +Modified: 2018-09-17 22:00 |
4 | 4 | Category: Ethereum
|
5 | 5 | Tags: ethereum, solidity, security
|
6 | 6 | Slug: solidity-security
|
@@ -194,16 +194,27 @@ contract Attack {
|
194 | 194 | Let us see how this malicious contract can exploit our `EtherStore` contract. The attacker would create the above contract (let's say at the address `0x0...123`) with the `EtherStore`'s contract address as the constructor parameter. This will initialize and point the public variable `etherStore` to the contract we wish to attack.
|
195 | 195 |
|
196 | 196 | The attacker would then call the `pwnEtherStore()` function, with some amount of ether (greater than or equal to 1), lets say `1 ether` for this example. In this example we assume a number of other users have deposited ether into this contract, such that it's current balance is `10 ether`. The following would then occur:
|
| 197 | + |
197 | 198 | 1. **Attack.sol - Line \[15\]** - The `depositFunds()` function of the EtherStore contract will be called with a `msg.value` of `1 ether` (and a lot of gas). The sender (`msg.sender`) will be our malicious contract (`0x0...123`). Thus, `balances[0x0..123] = 1 ether`.
|
| 199 | + |
198 | 200 | 2. **Attack.sol - Line \[17\]** - The malicious contract will then call the `withdrawFunds()` function of the `EtherStore` contract with a parameter of `1 ether`. This will pass all the requirements (Lines \[12\]-\[16\] of the `EtherStore` contract) as we have made no previous withdrawals.
|
| 201 | + |
199 | 202 | 3. **EtherStore.sol - Line \[17\]** - The contract will then send `1 ether` back to the malicious contract.
|
| 203 | + |
200 | 204 | 4. **Attack.sol - Line \[25\]** - The ether sent to the malicious contract will then execute the fallback function.
|
| 205 | + |
201 | 206 | 5. **Attack.sol - Line \[26\]** - The total balance of the EtherStore contract was `10 ether` and is now `9 ether` so this if statement passes.
|
| 207 | + |
202 | 208 | 6. **Attack.sol - Line \[27\]** - The fallback function then calls the `EtherStore` `withdrawFunds()` function again and "*re-enters*" the `EtherStore` contract.
|
| 209 | + |
203 | 210 | 7. **EtherStore.sol - Line \[11\]** - In this second call to `withdrawFunds()`, our balance is still `1 ether` as line \[18\] has not yet been executed. Thus, we still have `balances[0x0..123] = 1 ether`. This is also the case for the `lastWithdrawTime` variable. Again, we pass all the requirements.
|
| 211 | + |
204 | 212 | 8. **EtherStore.sol - Line \[17\]** - We withdraw another `1 ether`.
|
| 213 | + |
205 | 214 | 9. **Steps 4-8 will repeat** - until `EtherStore.balance >= 1` as dictated by line \[26\] in `Attack.sol`.
|
| 215 | + |
206 | 216 | 10. **Attack.sol - Line \[26\]** - Once there less 1 (or less) ether left in the `EtherStore` contract, this if statement will fail. This will then allow lines \[18\] and \[19\] of the `EtherStore` contract to be executed (for each call to the `withdrawFunds()` function).
|
| 217 | + |
207 | 218 | 11. **EtherStore.sol - Lines \[18\] and \[19\]** - The `balances` and `lastWithdrawTime` mappings will be set and the execution will end.
|
208 | 219 |
|
209 | 220 | The final result, is that the attacker has withdrawn all (bar 1) ether from the `EtherStore` contract, instantaneously with a single transaction.
|
@@ -285,8 +296,9 @@ contract TimeLock {
|
285 | 296 | function withdraw() public {
|
286 | 297 | require(balances[msg.sender] > 0);
|
287 | 298 | require(now > lockTime[msg.sender]);
|
| 299 | + uint balance = balances[msg.sender]; |
288 | 300 | balances[msg.sender] = 0;
|
289 |
| - msg.sender.transfer(balances[msg.sender]); |
| 301 | + msg.sender.transfer(balance); |
290 | 302 | }
|
291 | 303 | }
|
292 | 304 | ```
|
|
0 commit comments