diff options
author | Denton Liu <liu.denton+github@gmail.com> | 2016-08-11 22:28:53 +0800 |
---|---|---|
committer | Denton Liu <liu.denton+github@gmail.com> | 2016-08-11 22:35:45 +0800 |
commit | 4737100d005e99be5b45691d304e5efe1457d3df (patch) | |
tree | 564c239fa69062252949ef207ceb25f8f6711ff7 /docs | |
parent | 666c46ac296531ba55fbd02243b4cecd3645186c (diff) | |
download | dexon-solidity-4737100d005e99be5b45691d304e5efe1457d3df.tar dexon-solidity-4737100d005e99be5b45691d304e5efe1457d3df.tar.gz dexon-solidity-4737100d005e99be5b45691d304e5efe1457d3df.tar.bz2 dexon-solidity-4737100d005e99be5b45691d304e5efe1457d3df.tar.lz dexon-solidity-4737100d005e99be5b45691d304e5efe1457d3df.tar.xz dexon-solidity-4737100d005e99be5b45691d304e5efe1457d3df.tar.zst dexon-solidity-4737100d005e99be5b45691d304e5efe1457d3df.zip |
Change withdrawal example
The example is now a "King of the Ether"-esque contract. This is
actually relevant as they suffered an attack because of an almost
identical issue.
See the post-mortem here:
https://www.kingoftheether.com/postmortem.html
Diffstat (limited to 'docs')
-rw-r--r-- | docs/common-patterns.rst | 121 |
1 files changed, 43 insertions, 78 deletions
diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 870be049..8bf9e3c0 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -18,102 +18,67 @@ introduces a potential security risk. You may read more about this on the :ref:`security_considerations` page. This is an example of the withdrawal pattern in practice in -an Ether "store" contract. +a contract where the goal is to send the most money to the +contract in order to become the "richest". -:: - - contract WithdrawalStore { - - struct Item { - uint price; - uint quantity; - } - - modifier onlyOwner { - if (msg.sender == owner) { - _ - } - else { - throw; - } - } - - address owner; - mapping (string => Item) inventory; +In the following contract, if you are usurped as the richest, +you will recieve the funds of the person who has gone on to +become the richest. - function WithdrawalStore() { - owner = msg.sender; - } +:: - function updateInventory( - string _item, - uint _price, - uint _quantity - ) onlyOwner { - inventory[_item] = Item(_price, _quantity); - } + contract WithdrawalContract { + address public richest; + uint public mostSent; - // Notice that the owner withdraws their own Ether - function withdraw() onlyOwner { - owner.send(this.balance); + mapping (address => uint) pending; + + function WithdrawalContract() { + richest = msg.sender; + mostSent = msg.value; } - function buy(string _item) returns (bool) { - if ( - inventory[_item].quantity > 0 && - inventory[_item].price <= msg.value - ) { - inventory[_item].quantity--; + function becomeRichest() returns (bool) { + if (msg.value > mostSent) { + richest = msg.sender; + mostSent = msg.value; + pending[richest] = msg.value; return true; } else { return false; } } - } - -This is as opposed to the more intuitive sending pattern. - -:: - - contract SendStore { - - struct Item { - uint price; - uint quantity; - } - modifier onlyOwner { - if (msg.sender == owner) { - _ - } - else { + function withdraw() { + uint amount = pending[msg.sender]; + // Remember to zero the pending refund before sending + // to prevent re-entrancy attacks + pending[msg.sender] = 0; + if (!msg.sender.send(amount)) { throw; } } + } - address owner; - mapping (string => Item) inventory; +This is as opposed to the more intuitive sending pattern. - function SendStore() { - owner = msg.sender; - } +:: - function updateInventory( - string _item, - uint _price, - uint _quantity - ) onlyOwner { - inventory[_item] = Item(_price, _quantity); + contract SendContract { + address public richest; + uint public mostSent; + + function SendContract() { + richest = msg.sender; + mostSent = msg.value; } - function buy(string _item) returns (bool) { - if ( - inventory[_item].quantity > 0 && - inventory[_item].price <= msg.value - ) { - inventory[_item].quantity--; - owner.send(msg.value);// WARNING - this send is unchecked!! + function becomeRichest() returns (bool) { + if (msg.value > mostSent) { + richest = msg.sender; + mostSent = msg.value; + richest.send(msg.value); return true; } else { @@ -122,9 +87,9 @@ This is as opposed to the more intuitive sending pattern. } } -Notice that, in this example, an attacker could trap -the owner's funds in the contract by causing the -execution of `send` to fail through a callstack attack. +Notice that, in this example, an attacker could trap the +previous richest person's funds in the contract by causing +the execution of `send` to fail through a callstack attack. .. index:: access;restricting |