diff options
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/contracts.rst | 2 | ||||
| -rw-r--r-- | docs/control-structures.rst | 12 | ||||
| -rw-r--r-- | docs/index.rst | 1 | ||||
| -rw-r--r-- | docs/miscellaneous.rst | 28 | ||||
| -rw-r--r-- | docs/security-considerations.rst | 208 | ||||
| -rw-r--r-- | docs/solidity-by-example.rst | 24 | ||||
| -rw-r--r-- | docs/types.rst | 7 |
7 files changed, 250 insertions, 32 deletions
diff --git a/docs/contracts.rst b/docs/contracts.rst index e7b71c3d..c02c1490 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -356,7 +356,7 @@ all). Furthermore, this function is executed whenever the contract receives plain Ether (without data). In such a context, there is very little gas available to -the function call, so it is important to make fallback functions as cheap as +the function call (to be precise, 2300 gas), so it is important to make fallback functions as cheap as possible. :: diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 2f867cb0..6d615caf 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -69,6 +69,18 @@ this does not execute a constructor. We could also have used ``function setFeed( only (locally) sets the value and amount of gas sent with the function call and only the parentheses at the end perform the actual call. +.. warning:: + Any interaction with another contract imposes a certain danger, especially + if the source code of the contract is not known in advance. The current + contract hands over control to the called contract and that might do + just about anything. Be prepared that it calls into other contracts of + your system and perhaps even back into the calling contract before your + call returns. This means + that the called contract can change state variables of the calling contract + via its functions. Write your functions in a way that e.g. calls to + external functions happen after any changes to state variables in your contract, + so your contract is not vulnerable to a recursive call exploit. + Named Calls and Anonymous Function Parameters --------------------------------------------- diff --git a/docs/index.rst b/docs/index.rst index 32303fe4..a683a4dc 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -92,6 +92,7 @@ Contents installing-solidity.rst solidity-by-example.rst solidity-in-depth.rst + security-considerations.rst style-guide.rst common-patterns.rst frequently-asked-questions.rst diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst index c883815c..85fc286c 100644 --- a/docs/miscellaneous.rst +++ b/docs/miscellaneous.rst @@ -145,34 +145,6 @@ Tips and Tricks * If you do **not** want your contracts to receive ether when called via ``send``, you can add a throwing fallback function ``function() { throw; }``. * Initialise storage structs with a single assignment: ``x = MyStruct({a: 1, b: 2});`` -******** -Pitfalls -******** - -Unfortunately, there are some subtleties the compiler does not yet warn you about. - -- In ``for (var i = 0; i < arrayName.length; i++) { ... }``, the type of ``i`` will be ``uint8``, because this is the smallest type that is required to hold the value ``0``. If the array has more than 255 elements, the loop will not terminate. -- If a contract receives Ether (without a function being called), the fallback function is executed. The contract can only rely - on the "gas stipend" (2300 gas) being available to it at that time. This stipend is not enough to access storage in any way. - To be sure that your contract can receive Ether in that way, check the gas requirements of the fallback function. -- If you want to send ether using ``address.send``, there are certain details to be aware of: - - 1. If the recipient is a contract, it causes its fallback function to be executed which can in turn call back into the sending contract - 2. Sending Ether can fail due to the call depth going above 1024. Since the caller is in total control of the call - depth, they can force the transfer to fail, so make sure to always check the return value of ``send``. Better yet, - write your contract using a pattern where the recipient can withdraw Ether instead. - 3. Sending Ether can also fail because the recipient runs out of gas (either explicitly by using ``throw`` or - because the operation is just too expensive). If the return value of ``send`` is checked, this might provide a - means for the recipient to block progress in the sending contract. Again, the best practise here is to use - a "withdraw" pattern instead of a "send" pattern. - -- Loops that do not have a fixed number of iterations, e.g. loops that depends on storage values, have to be used carefully: - Due to the block gas limit, transactions can only consume a certain amount of gas. Either explicitly or just due to - normal operation, the number of iterations in a loop can grow beyond the block gas limit, which can cause the complete - contract to be stalled at a certain point. This does not apply at full extent to ``constant`` functions that are only executed - to read data from the blockchain. Still, such functions may be called by other contracts as part of on-chain operations - and stall those. Please be explicit about such cases in the documentation of your contracts. - ********** Cheatsheet ********** diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst new file mode 100644 index 00000000..d6cd9dee --- /dev/null +++ b/docs/security-considerations.rst @@ -0,0 +1,208 @@ +####################### +Security Considerations +####################### + +While it is usually quite easy to build software that works as expected, +it is much harder to check that nobody can use it in a way that was **not** anticipated. + +In Solidity, this is even more important because you can use smart contracts +to handle tokens or even more valuable things and every execution of a smart +contract happens in public as is mostly open source. + +Of course you always have to consider how much is at stake: +You can compare a smart contract with a web service that is open to the +public (and thus also to malicous actors) and perhaps even open source. +If you only store your grocery list on that web service, you might not have +take too much care, but if you manage your bank account using that web service, +you should be more careful. + +This section will list some pitfalls and general security recommendations but +can of course never be complete. Also keep in mind that even if your +smart contract code is bug-free, the compiler or the platform itself might +have a bug. + +As always with open source documentation, please help us extend this section +(especially, some examples would not hurt)! + +******** +Pitfalls +******** + +Private Information and Randomness +================================== + +Everything you use in a smart contract is publicly visible, even +local variables and state variables marked ``private``. + +Using random numbers in smart contracts is quite tricky if you do not want +miners to be able to cheat. + +Re-Entrancy +=========== + +Any interaction from a contract (A) with another contract (B) and any transfer +of Ether hands over control to that contract (B). This makes it possible for B +to call back into A before this interaction is completed. To give an example, +the following code contains a bug (it is just a snippet and not a +complete contract): + +:: + + // 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.send(shares[msg.sender])) + shares[msg.sender] = 0; + } + } + +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 enable it to get a multiple refund and +basically retrieve all the Ether in the contract. + +To avoid re-entrancy, you can use the Checks-Effects-Interactions pattern as +outlined further below: + +:: + + contract Fund { + /// Mapping of ether shares of the contract. + mapping(address => uint) shares; + /// Withdraw your share. + function withdraw() { + var share = shares[msg.sender]; + shares[msg.sender] = 0; + if (!msg.sender.send(share)) + throw; + } + } + + +Note that re-entrancy is not only an effect of Ether transfer but of any +function call on another contract. Furthermore, you also have to take +multi-contract situations into account. A called contract could modify the +state of another contract you depend on. + +Gas Limit and Loops +=================== + +Loops that do not have a fixed number of iterations, e.g. loops that depends on storage values, have to be used carefully: +Due to the block gas limit, transactions can only consume a certain amount of gas. Either explicitly or just due to +normal operation, the number of iterations in a loop can grow beyond the block gas limit, which can cause the complete +contract to be stalled at a certain point. This does not apply at full extent to ``constant`` functions that are only executed +to read data from the blockchain. Still, such functions may be called by other contracts as part of on-chain operations +and stall those. Please be explicit about such cases in the documentation of your contracts. + +Sending and Receiving Ether +=========================== + +- If a contract receives Ether (without a function being called), the fallback function is executed. The contract can only rely + on the "gas stipend" (2300 gas) being available to it at that time. This stipend is not enough to access storage in any way. + To be sure that your contract can receive Ether in that way, check the gas requirements of the fallback function + (for example in the "details" section in browser-solidity). + +- If you want to send ether using ``address.send``, there are certain details to be aware of: + + 1. If the recipient is a contract, it causes its fallback function to be executed which can in turn call back into the sending contract + 2. Sending Ether can fail due to the call depth going above 1024. Since the caller is in total control of the call + depth, they can force the transfer to fail, so make sure to always check the return value of ``send``. Better yet, + write your contract using a pattern where the recipient can withdraw Ether instead. + 3. Sending Ether can also fail because the recipient runs out of gas (either explicitly by using ``throw`` or + because the operation is just too expensive). If the return value of ``send`` is checked, this might provide a + means for the recipient to block progress in the sending contract. Again, the best practise here is to use + a "withdraw" pattern instead of a "send" pattern. + +Callstack Depth +=============== + +External function calls can fail any time because they exceed the maximum +call stack of 1024. In such situations, Solidity throws an exception. +Malicious actors might be able to force the call stack to a high value +before they interact with your contract. + +Minor Details +============= + +- In ``for (var i = 0; i < arrayName.length; i++) { ... }``, the type of ``i`` will be ``uint8``, because this is the smallest type that is required to hold the value ``0``. If the array has more than 255 elements, the loop will not terminate. +- The ``constant`` keyword is currently not enforced by the compiler. + Furthermore, it is not enforced by the EVM, so a contract function that "claims" + to be constant might still cause changes to the state. +- Types that do not occupy the full 32 bytes might contain "dirty higher order bits". + This is especially important if you access ``msg.data`` - it poses a malleability risk. + +*************** +Recommendations +*************** + +Restrict the Amount of Ether +============================ + +Restrict the amount of Ether (or other tokens) that can be stored in a smart +contract. If your source code, the compiler or the platform has a bug, these +funds might be gone. If you want to limit your loss, limit the amount of Ether. + +Keep it Small and Modular +========================= + +Keep your contracts small and easily understandable. Single out unrelated +functionality in other contracts or into libraries. General recommendations +about source code quality of course apply: Limit the amount of local variables, +the length of functions and so on. Document your functions so that others +can see what your intention was and whether it is different than what the code does. + +Program in Checks-Effects-Interactions-way +=========================================== + +Most functions will first perform some checks (who called the function, +are the arguments in range, did they send enough Ether, does the person +have tokens, ...). These checks should be done first. + +As the second step, if all checks passed, effects to the state variables +of the current contract should be made. Interaction with other contracts +should be the very last step in any function. + +Early contracts delayed some effects and waited for external function +calls to return in a non-error state. This is often a serious mistake, +because of the re-entrancy problem explained above. + +Note that also calls to known contracts might in turn cause calls to +unknown contracts, so it is probably better to just always apply this pattern. + +Include a Failsafe-Mode +======================= + +While making your system fully decentralised will remove any intermediary, +it might be a good idea, especially for new code, to include some kind +of fail-safe-mechanism: + +You can add a function in your smart contract that performs some +self-checks like "Has any Ether leaked?", +"Is the sum of the tokens equal to the balance of the contract?" or simila things. +Keep in mind that you cannot use too much gas for that, so help though off-chain +computations might be needed there. + +If the self-check fails, the contract automatically switches into some kind +of "failsafe" mode, which e.g. disables most of the features, hands over +control to a fixed and trusted third party or just converts the contract into +a simple "give me back my money"-contract. + + +******************* +Formal Verification +******************* + +Using formal verification, it is possible to perform an automated mathematical +proof that your source code fulfills a certain formal specification. +The specification is still formal (just as the source code), but usually much +simpler. There is a prototype in Solidity that performs formal verification and +it will be better documented soon. + +Note that formal verification itself can only help you understand the +difference between what you did (the specification) and how you did it +(the actual implementation). You still need to check whether the specification +is what you wanted and that you did not miss any unintended effects of it.
\ No newline at end of file diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst index 6fa70be4..7dd51f00 100644 --- a/docs/solidity-by-example.rst +++ b/docs/solidity-by-example.rst @@ -291,15 +291,32 @@ activate themselves. /// End the auction and send the highest bid /// to the beneficiary. function auctionEnd() { + // It is a good guideline to structure functions that interact + // with other contracts (i.e. they call functions or send Ether) + // into three phases: + // 1. checking conditions + // 2. performing actions (potentially changing conditions) + // 3. interacting with other contracts + // If these phases are mixed up, the other contract could call + // back into the current contract and modify the state or cause + // effects (ether payout) to be perfromed multiple times. + // If functions called internally include interaction with external + // contracts, they also have to be considered interaction with + // external contracts. + + // 1. Conditions if (now <= auctionStart + biddingTime) throw; // auction did not yet end if (ended) throw; // this function has already been called + + // 2. Effects + ended = true; AuctionEnded(highestBidder, highestBid); + // 3. Interaction if (!beneficiary.send(highestBid)) throw; - ended = true; } function () { @@ -476,7 +493,8 @@ high or low invalid bids. var amount = pendingReturns[msg.sender]; // It is important to set this to zero because the recipient // can call this function again as part of the receiving call - // before `send` returns. + // before `send` returns (see the remark above about + // conditions -> effects -> interaction). pendingReturns[msg.sender] = 0; if (!msg.sender.send(amount)) throw; // If anything fails, this will revert the changes above @@ -490,11 +508,11 @@ high or low invalid bids. if (ended) throw; AuctionEnded(highestBidder, highestBid); + ended = true; // We send all the money we have, because some // of the refunds might have failed. if (!beneficiary.send(this.balance)) throw; - ended = true; } function () { diff --git a/docs/types.rst b/docs/types.rst index dbbd84d3..1a0de358 100644 --- a/docs/types.rst +++ b/docs/types.rst @@ -113,6 +113,13 @@ All three functions ``call``, ``delegatecall`` and ``callcode`` are very low-lev All contracts inherit the members of address, so it is possible to query the balance of the current contract using ``this.balance``. +.. warning:: + All these functions are low-level functions and should be used with care. + Specifically, any unknown contract might be malicious and if you call it, you + hand over control to that contract which could in turn call back into + your contract, so be prepared for changes to your state variables + when the call returns. + .. index:: byte array, bytes32 |