Merge pull request #8009 from ethereum/docs_security

Docs security considerations
This commit is contained in:
chriseth 2019-12-16 18:22:22 +01:00 committed by GitHub
commit bd91c0d5c3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -20,14 +20,12 @@ to take too much care, but if you manage your bank account using that web servic
you should be more careful. you should be more careful.
This section will list some pitfalls and general security recommendations but This section will list some pitfalls and general security recommendations but
can, of course, never be complete. can, of course, never be complete. Also, keep in mind that even if your smart
Also, keep in mind that even if your contract code is bug-free, the compiler or the platform itself might have a
smart contract code is bug-free, the compiler or the platform itself might bug. A list of some publicly known security-relevant bugs of the compiler can
have a bug. A list of some publicly known security-relevant bugs of the compiler be found in the :ref:`list of known bugs<known_bugs>`, which is also
can be found in the machine-readable. Note that there is a bug bounty program that covers the code
:ref:`list of known bugs<known_bugs>`, which is also machine-readable. Note generator of the Solidity compiler.
that there is a bug bounty program that covers the code generator of the
Solidity compiler.
As always, with open source documentation, please help us extend this section As always, with open source documentation, please help us extend this section
(especially, some examples would not hurt)! (especially, some examples would not hurt)!
@ -138,12 +136,16 @@ Sending and Receiving Ether
to move Ether without creating a message call. One way is to simply "mine to" to move Ether without creating a message call. One way is to simply "mine to"
the contract address and the second way is using ``selfdestruct(x)``. the contract address and the second way is using ``selfdestruct(x)``.
- If a contract receives Ether (without a function being called), the fallback function is executed. - If a contract receives Ether (without a function being called),
If it does not have a fallback function, the Ether will be rejected (by throwing an exception). either the :ref:`receive Ether <receive-ether-function>`
During the execution of the fallback function, the contract can only rely or the :ref:`fallback <fallback-function>` function is executed.
on the "gas stipend" it is passed (2300 gas) being available to it at that time. This stipend is not enough to modify storage If it does not have a receive nor a fallback function, the Ether will be
(do not take this for granted though, the stipend might change with future hard forks). rejected (by throwing an exception). During the execution of one of these
To be sure that your contract can receive Ether in that way, check the gas requirements of the fallback function functions, the contract can only rely on the "gas stipend" it is passed (2300
gas) being available to it at that time. This stipend is not enough to modify
storage (do not take this for granted though, the stipend might change with
future hard forks). To be sure that your contract can receive Ether in that
way, check the gas requirements of the receive and fallback functions
(for example in the "details" section in Remix). (for example in the "details" section in Remix).
- There is a way to forward more gas to the receiving contract using - There is a way to forward more gas to the receiving contract using
@ -159,17 +161,22 @@ Sending and Receiving Ether
- If you want to send Ether using ``address.transfer``, there are certain details to be aware of: - If you want to send Ether using ``address.transfer``, 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 the sending contract. 1. If the recipient is a contract, it causes its receive or fallback function
2. Sending Ether can fail due to the call depth going above 1024. Since the caller is in total control of the call to be executed which can, in turn, call back the sending contract.
depth, they can force the transfer to fail; take this possibility into account or use ``send`` and make sure to always check its return value. Better yet, 2. Sending Ether can fail due to the call depth going above 1024. Since the
write your contract using a pattern where the recipient can withdraw Ether instead. caller is in total control of the call depth, they can force the
3. Sending Ether can also fail because the execution of the recipient contract transfer to fail; take this possibility into account or use ``send`` and
requires more than the allotted amount of gas (explicitly by using ``require``, make sure to always check its return value. Better yet, write your
``assert``, ``revert``, ``throw`` or contract using a pattern where the recipient can withdraw Ether instead.
because the operation is just too expensive) - it "runs out of gas" (OOG). 3. Sending Ether can also fail because the execution of the recipient
If you use ``transfer`` or ``send`` with a return value check, this might provide a contract requires more than the allotted amount of gas (explicitly by
means for the recipient to block progress in the sending contract. Again, the best practice here is to use using :ref:`require <assert-and-require>`, :ref:`assert <assert-and-require>`,
a :ref:`"withdraw" pattern instead of a "send" pattern <withdrawal_pattern>`. :ref:`revert <assert-and-require>` or because the
operation is too expensive) - it "runs out of gas" (OOG). If you
use ``transfer`` or ``send`` with a return value check, this might
provide a means for the recipient to block progress in the sending
contract. Again, the best practice here is to use a :ref:`"withdraw"
pattern instead of a "send" pattern <withdrawal_pattern>`.
Callstack Depth Callstack Depth
=============== ===============
@ -181,8 +188,7 @@ before they interact with your contract.
Note that ``.send()`` does **not** throw an exception if the call stack is Note that ``.send()`` does **not** throw an exception if the call stack is
depleted but rather returns ``false`` in that case. The low-level functions depleted but rather returns ``false`` in that case. The low-level functions
``.call()``, ``.callcode()``, ``.delegatecall()`` and ``.staticcall()`` behave ``.call()``, ``.delegatecall()`` and ``.staticcall()`` behave in the same way.
in the same way.
tx.origin tx.origin
========= =========
@ -207,7 +213,7 @@ Never use tx.origin for authorization. Let's say you have a wallet contract like
} }
} }
Now someone tricks you into sending ether to the address of this attack wallet: Now someone tricks you into sending Ether to the address of this attack wallet:
:: ::
@ -224,7 +230,7 @@ Now someone tricks you into sending ether to the address of this attack wallet:
owner = msg.sender; owner = msg.sender;
} }
fallback() external { receive() external payable {
TxUserWallet(msg.sender).transferTo(owner, msg.sender.balance); TxUserWallet(msg.sender).transferTo(owner, msg.sender.balance);
} }
} }
@ -247,8 +253,7 @@ In general, read about the limits of two's complement representation, which even
more special edge cases for signed numbers. more special edge cases for signed numbers.
Try to use ``require`` to limit the size of inputs to a reasonable range and use the Try to use ``require`` to limit the size of inputs to a reasonable range and use the
:ref:`SMT checker<smt_checker>` to find potential overflows, or :ref:`SMT checker<smt_checker>` to find potential overflows, or use a library like
use a library like
`SafeMath <https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/math/SafeMath.sol>`_ `SafeMath <https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/math/SafeMath.sol>`_
if you want all overflows to cause a revert. if you want all overflows to cause a revert.
@ -259,16 +264,16 @@ Code such as ``require((balanceOf[_to] + _value) >= balanceOf[_to])`` can also h
Clearing Mappings Clearing Mappings
================= =================
The Solidity type ``mapping`` (see :ref:`mapping-types`) is a storage-only key-value data structure that The Solidity type ``mapping`` (see :ref:`mapping-types`) is a storage-only
does not keep track of the keys that were assigned a non-zero value. key-value data structure that does not keep track of the keys that were
Because of that, cleaning a mapping without extra information about the written assigned a non-zero value. Because of that, cleaning a mapping without extra
keys is not possible. information about the written keys is not possible.
If a ``mapping`` is used as the base type of a dynamic storage array, deleting If a ``mapping`` is used as the base type of a dynamic storage array, deleting
or popping the array will have no effect over the ``mapping`` elements. or popping the array will have no effect over the ``mapping`` elements. The
The same happens, for example, if a ``mapping`` is used as the type of a member same happens, for example, if a ``mapping`` is used as the type of a member
field of a ``struct`` that is the base type of a dynamic storage array. field of a ``struct`` that is the base type of a dynamic storage array. The
The ``mapping`` is also ignored in assignments of structs or arrays containing ``mapping`` is also ignored in assignments of structs or arrays containing a
a ``mapping``. ``mapping``.
:: ::
@ -307,8 +312,7 @@ another call to ``writeMap``.
If your ``mapping`` information must be deleted, consider using a library similar to If your ``mapping`` information must be deleted, consider using a library similar to
`iterable mapping <https://github.com/ethereum/dapp-bin/blob/master/library/iterable_mapping.sol>`_, `iterable mapping <https://github.com/ethereum/dapp-bin/blob/master/library/iterable_mapping.sol>`_,
allowing you to traverse the keys and delete their values in the appropriate allowing you to traverse the keys and delete their values in the appropriate ``mapping``.
``mapping``.
Minor Details Minor Details
============= =============