Skip to content

Latest commit

 

History

History
506 lines (392 loc) · 61 KB

EthereumSmartContracts.md

File metadata and controls

506 lines (392 loc) · 61 KB
Raw

Ethereum smart contracts security recommendations and best practices:

  1. In ERC20 approve first change to 0 and then to value

    1. tokens should verify in approve that previously was 0 if changing not to 0 like in minime and also add increaseAllowance/decreaseAllowance preferring it over approve
    2. https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit
    3. ethereum/EIPs#738
    4. ethereum/EIPs#610 (comment) was said to not be necessary: Giveth/minime#18 (comment)
    5. minime solution is shortest and best and here is an explanation of why it is enough: ethereum/EIPs#20 (comment)

  2. Using things like block.coinbase, block.difficulty, block.gaslimit, block.number, block.timestamp, tx.gasprice, or tx.origin in constant functions is not a good idea, because it is unspecified what EVM will return, and different implementations, or even different versions of the same implementation, may behave differently

    1. ethereum/EIPs#610 (comment)

  3. re-entrancy: transferring ether can trigger another contract which triggers back current contract causing money drain. this can be solved by Checks-Effects-Interactions pattern https://solidity.readthedocs.io/en/v0.5.8/security-considerations.html#re-entrancy

    1. this caused the DAO hack https://ethereum.stackexchange.com/questions/6210/how-was-the-recursion-created-that-lead-to-thedao-hack
    2. https://medium.com/spankchain/we-got-spanked-what-we-know-so-far-d5ed3a0f38fe
    3. the smart contracts execution is single-threaded (but the order between transactions is unknown until they settle with enough confirmations) which makes it possible to use state variables as mutexes, however this adds gas cost so other solutions might be preferable
      1. https://ethereum.stackexchange.com/questions/8261/how-to-solve-solidity-asynchronous-problem/8265#8265
      2. https://github.com/sigp/solidity-security-blog#preventative-techniques
    4. https://github.com/ethereum/wiki/wiki/Safety#pitfalls-in-race-condition-solutions

  4. always first do checks and then state changes and only then external contracts calls or transfers https://solidity.readthedocs.io/en/v0.5.8/security-considerations.html#use-the-checks-effects-interactions-pattern

  5. using "transfer" can cause problems and it is better to allow withdrawals instead. ALWAYS use ERC20 approve+transferFrom instead of transfer, which will help protect from transfers to bad address and other errors, see: https://solidity.readthedocs.io/en/v0.5.8/common-patterns.html#withdrawal-pattern

  6. never trust transaction origin as identification (origin is always the EOA who made initial transaction while msg.sender can be a contract which made internal transaction as a result of the original transaction) https://solidity.readthedocs.io/en/v0.5.8/security-considerations.html#tx-origin

  7. use openZepplin safemath to protect from math overflows and underflows for any mathematic operations https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-101

    1. there is an EIP about evm opcodes protecting from overflows so use them when available https://eips.ethereum.org/EIPS/eip-1051

  8. enable SMTChecker using appropriate pragma to allow formal verification https://solidity.readthedocs.io/en/v0.5.8/layout-of-source-files.html#smt-checker https://solidity.readthedocs.io/en/v0.5.8/security-considerations.html#formal-verification

  9. dont add kill/self destruct or it can cause money loss: parity-contracts/0x863df6bfa4#2 openethereum/parity-ethereum#6995

  10. https://medium.com/loom-network/how-to-secure-your-smart-contracts-6-solidity-vulnerabilities-and-how-to-avoid-them-part-2-730db0aa4834

    1. point 4 in the link: calling selfdestruct with a contract address, sends ether to that address without calling the contract fallback function so never base decisions on the balance.
      1. another way is using coinbase transaction of mining reward
      2. another way is pre-fund an address before contract creation https://github.com/sigp/solidity-security-blog#pre-sent-ether
      3. another way is if the constructor is payable then to transfer funds on contract creation
    2. point 5 - always assume that transfers and external calls (of instance) can trigger revert
      1. calling method from contract instance propogates errors and allows to get return value while calling using .call(..) does not require knowing the abi, on failure only returns false without reverting and does not allow getting the true return value https://ethereum.stackexchange.com/questions/30383/difference-between-call-on-external-contract-address-function-and-creating-contr
      2. UPDATE: From solidity compiler version 0.5 the function .call DOES allow to get the return value, see: https://solidity.readthedocs.io/en/v0.5.8/050-breaking-changes.html#semantic-and-syntactic-changes
    3. ALWAYS avoid the use of now and block.blockhash for your contract’s business logic as their results are predictable or can be manipulated by miners https://www.reddit.com/r/ethereum/comments/483rr1/do_not_use_block_hash_as_source_of_randomness/
    4. more things covered already in other bullets in the recommendations list here https://medium.com/loom-network/how-to-secure-your-smart-contracts-6-solidity-vulnerabilities-and-how-to-avoid-them-part-1-c33048d4d17d

  11. .call() and .call().value() should be avoided (.value(..) adds ether to send and .gas(..) adds gas limit). they both dont limit gas (without calling .gas(..)) and return true/false instead of reverting on failure and they may cause re-entrance vulnerability https://ethereum.stackexchange.com/questions/43782/importance-of-call-value/43821#43821

  12. use cases of using send,transfer,.call().value(..).gas(..)()(). basically most cases use transfer and if using call then always set .gas(..) or otherwise the other contract can do a DOS by doing assert which takes away all gas preventing code from continueing execution after the call https://ethereum.stackexchange.com/questions/19341/address-send-vs-address-transfer-best-practice-usage/38642#38642 ethereum/solidity#610

  13. always prefer to throw and revert than to return true/false (more safe against reentrance attacks) as from discussion here ethereum/solidity#610

    1. some good arguments about why better to return false instead of throw: ethereum/EIPs#610 (comment)
    2. some good arguments about why better to throw than return false: ethereum/EIPs#20 (comment) ethereum/EIPs#20 (comment)

  14. in general it is safer to trigger exception than to return false because exception reverts any changes to the state while return false leaves the responsability for the caller to revert if needed https://ethereum.stackexchange.com/questions/15140/would-it-be-better-to-use-throw-instead-of-return-false/15147#15147

  15. new solidity compiler versions removed throw keyword and it now has assert,require,revert

    1. discussion on use cases of assert (in the end of 2018 open zepplelin changed safemath to use require instead of assert) OpenZeppelin/openzeppelin-contracts#1120
    2. require and revert return the rest of the gas to the user while assert uses all of the gas recevied
    3. require is for input validations while assert is mainly for static code analysis to identify situations which should NEVER happen and create compile time warnings
      1. https://ethereum.stackexchange.com/questions/27812/why-using-assert-since-it-would-consume-all-gas/27824#27824
      2. https://ethereum.stackexchange.com/questions/15166/difference-between-require-and-assert-and-the-difference-between-revert-and-thro/50957#50957
    4. require vs revert seems to be different syntax for the same thing: ethereum/solidity#6689

  16. if contract does not need to receive ether (erc20 token without buying period for example) then dont implement payable functions and dont implement fallback functions. dont create unnecessary attack interface. more code = more attack interface. transfer to the contract will revert automatically: https://ethereum.stackexchange.com/questions/34160/why-do-we-use-revert-in-payable-function/34164#34164

  17. still need a way to get ether out even if fallback function is not implemented in case where ether will still be received using selfdestruct or transfered when calling some methods

  18. should make error messages as short as possible while still being clear because error messages increase contract deployment gas (while not increasing executing gas) https://ethereum.stackexchange.com/questions/66879/does-a-string-message-increase-the-gas-usage-of-a-require-statement

  19. on division make sure the number is even or otherwise it will get rounded DOWN

  20. be careful of replay attacks and of different assumptions https://media.consensys.net/discovering-signature-verification-bugs-in-ethereum-smart-contracts-424a494c6585

    1. for example previously a replay was possible between ethereum mainnet transaction and other chains, this was addressed in eip 155: https://eips.ethereum.org/EIPS/eip-155

  21. Implement only the necessary functionality and try doing that in the shortest and most efficient way, to avoid creating unneccesary attack surface and room for bugs in the code

  22. Use audited and tested code https://security.stackexchange.com/questions/18197/why-shouldnt-we-roll-our-own

  23. Write as many unit tests as possible https://consensys.github.io/smart-contract-best-practices/software_engineering/#contract-rollout

  24. perform security audits and implement security layers architecture to protect from the unknown https://dasp.co/#item-10

  25. Inline assembly is a way to access the Ethereum Virtual Machine at a low level. This bypasses several important safety features and checks of Solidity. You should only use it for tasks that need it, and only if you are confident with using it https://solidity.readthedocs.io/en/v0.5.8/assembly.html#inline-assembly

  26. even if selfdestruct is not implemented, it is still possible using delegatecall so libraries should never call delegatecall/callcode

  27. careful with delegatecall because it preserves context and allows the called contract to modify state of original contract. so the caller is at the merci of the called contract https://ethereum.stackexchange.com/questions/11578/how-to-protect-against-delegatecall-in-a-hub-spoke-contract-model/11598#11598

  28. callcode also allows called contract to modify original contract storage just like delegatecall

    1. https://ethereum.stackexchange.com/questions/3667/difference-between-call-callcode-and-delegatecall
    2. to detect if your smart contract is called by callcode/delegatecall: https://ethereum.stackexchange.com/questions/69551/how-to-prevent-the-code-of-my-contract-being-used-in-a-callcode/69552

  29. libraries called with delegatecall to modify original contract state, SHOULD NOT STORE STATE of themselves. because their state is actually pointing to storage slots of the caller contract which used delegatecall. use "library" keyword instead of "contract" keyword for libraries to detect problems at compile time (ensures the library contract is stateless and non-self-destructable) https://github.com/sigp/solidity-security-blog/blob/master/README.md#preventative-techniques-3

  30. order of state variables matters! sort them from smallest to biggest type so that the storage allocated will be minimal which will require less gas to deploy

    1. mappings and dynamically-sized arrays don't follow normal storage rules
    2. when byte32 is converted to byte16 its taken from the left (0x922aba368bee9844aefc4b47b1d58d2857781b382dc1ad896d512e19131d108f -> 0x922aba368bee9844aefc4b47b1d58d28) https://gist.github.com/kenjirai/d1b9c135117eb39d7891d658cbd6154c
    3. when uint32 is converted to uint16 then the smallest bytes are taken 0x12345678 -> 0x5678

  31. private members of contract are accessible (for example using web3.eth.getStorageAt) so don't store any private information in the contract. zksnarks and zero knowledge proofs and private key signature can allow proof of secret without revealing the secret

    1. getStorageAt returns 32 bytes content of desired storage slot which is a 32 bytes storage unit

  32. constant variables of contract are not stored in storage

  33. even if interface states that function is view/pure, the function implementation contract can define the function without view/pure and modify state. so can't assume state is not modified by interface declaration alone

  34. when relevant to call something without letting it change state: STATICCALL opcode makes sure the called function does not modify state which new solidity compilers use for view/pure functions https://eips.ethereum.org/EIPS/eip-214

  35. careful with EXTCODESIZE and CODESIZE because they behave differently during contract initialization: contract address is calculated from contract creation info and code initialization code is executed BEFORE the code is associated to the contract address so "During initialization code execution, EXTCODESIZE on the address should return zero, which is the length of the code of the account while CODESIZE should return the length of the initialization code" https://medium.com/coinmonks/ethernaut-lvl-14-gatekeeper-2-walkthrough-how-contracts-initialize-and-how-to-do-bitwise-ddac8ad4f0fd

    1. "Put simply, if you try to check for a smart contract’s code size before or during contract construction, you will get an empty value. This is because the smart contract hasn’t been made yet, and thus cannot be self cognizant of its own code size"

  36. contract which has problems at initialization will have an address but have no code

  37. structs

    1. struct declarations default to storage so ALWAYS use "memory" keyword in initialization of temporary struct inside function to prevent it being saved to storage. also because of this better to avoid structs for temporary calculations
    2. if you declare a new storage struct in your function, it will overwrite other globally stored variables (starting from the first slot)
    3. when you directly save a memory struct into a state variable, the memory struct is automatically forced into storage
    4. function inputs are memory and not storage backed
    5. you cannot implicitly convert memory into storage in assignment of function input struct to storage variable struct
    6. although you cant return structs from functions, you can create mapping state variables with struct values and a default getter will be created for it which returns struct

38.when calling contract from remix better to always wrap big numbers with quotes to avoid problems and truncating

  1. contract address is deterministically calculated from transaction so it is possible to recover lost funds if can send transaction of contract creation which creates address where funds are at (or to prefund a contract this way before its creation)

    1. https://github.com/sigp/solidity-security-blog#keyless-ether
    2. address is calculated from sender address and nonce. for contract the nonce starts from 1 (0 used for contract self creation): address(keccak256(0xd6, 0x94, YOUR_ADDR, 0x01)). next contracts address can be calculated by incrementing nonce in the formula
    3. https://medium.com/coinmonks/ethernaut-lvl-18-recovery-walkthrough-how-to-retrieve-lost-contract-addresses-in-2-ways-aba54ab167d3

  2. if many contracts are deployed and want to optimize deployment gas by making contracts smaller then can deploy contract bytecode directly which can make contract opcode as small as for example 12 opcodes for initialization and 10 opcodes for runtime, see: https://medium.com/coinmonks/ethernaut-lvl-19-magicnumber-walkthrough-how-to-deploy-contracts-using-raw-assembly-opcodes-c50edb0f71a2

  3. arrays

    1. arrays declarations default to storage so ALWAYS use "memory" keyword in initialization of temporary arrays inside function to prevent it being saved to storage
    2. dynamic arrays are ones defined with empty []
    3. Dynamic arrays must be initialized as "storage" variables
    4. You can resize dynamic storage arrays but You cannot resize memory arrays, nor fixed size arrays
    5. when you call solidity method you pass array size and it is not checked against actual payload. this allows to pass arrays of size bigger than possible to actually be generated and might allow other exploits so it needs to be taken into account when developing contracts (that array length is not bound by solidity)
    6. arbitrary array length change and index access in array should not be allowed because it can possibly allow to access any contract storage using the array indexer
    7. changing array length can be done by changing the array length property. need to be careful of underflows
    8. https://ylv.io/ethernaut-alien-codex-solution/
    9. https://f3real.github.io/Ethernaut_wargame2022.html

  4. be aware that maps and arrays and any other variables, are not really allocated in an independent location, they are all together in the storage slots of the contract and are references to some positions in that storage (array and maps positions in the storage are calculated using a special formula), thus for example it is possible to access the same storage using a map and an array if the array is big enough

  5. be aware that revert reverts everything including events except gas spent https://ethereum.stackexchange.com/questions/4085/is-it-a-good-practice-to-log-an-event-every-time-i-throw-in-solidity

  6. https://blog.zeppelin.solutions/onward-with-ethereum-smart-contract-security-97a827e47702

    1. Avoid declaring variables using var if possible, otherwise the type will be the smallest possible (for i=1 the type will be uint8 maxing at 255)
      1. UPDATE: from solidity compiler version 0.5 using var is disallowed https://solidity.readthedocs.io/en/v0.5.8/050-breaking-changes.html#variables
    2. the recommended naming for custom events is that they should start with "Log" to make them easily identified

  7. add fail strings to require and revert commands for easier error finding and bugs detection

  8. short address attack: if user address ends with one or more zeroes then by ommiting the zeroes if the sending system does not check the address length and create a transaction then the zero will be padded and will cause the transaction value to be added a zero causing undesired amount transfer

    1. https://dasp.co/#item-9
    2. mitigation using assert(msg.data.length == numOfParams * 32 + 4) creates bugs for multisig wallets working with the token
      1. Giveth/minime#8 (comment)
      2. https://vessenes.com/notice-about-fun-token-support-for-multisignature-wallets-2/
      3. funfair token paid for gas of transition to new contract https://funfair.io/new-fun-token-contract/
    3. mitigating by >= instead of == is also problematic https://blog.coinfabrik.com/smart-contract-short-address-attack-mitigation-failure/
    4. OpenZeppelin removed the short address attack validations from smart contracts because of the bugs they caused OpenZeppelin/openzeppelin-contracts#261
    5. was fixed in solidity 0.5.0 so no need to perform anything in the smart contract however SHOULD perform address validations in the deposit/withdrawal interfaces in our system ethereum/solidity#4224

  9. must compile with latest solidity versions to get latest security fixes, for example the short address attack fix in solidity 0.5.0 ethereum/solidity#4224

    1. https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-102

  10. can't just do the hard work and security during contract development and then relax after deployment since solidity and evm upgrades many times enter new vulnerabilities to previously safe code, so need to always be up to date and ready to respond and to plan risk management for such cases. Example: https://medium.com/chainsecurity/constantinople-enables-new-reentrancy-attack-ace4088297d9

    1. another example where geth protocol change caused a bug and loss of funds https://steemit.com/cryptocurrency/@barrydutton/breaking-the-biggest-canadian-coin-exchange-quadrigacx-loses-67-000-usdeth-due-to-coding-error-funds-locked-in-an-executable
    2. example of place to periodically read is the solidity release notes to see bug fixes and behavior changes: https://github.com/ethereum/solidity/releases

  11. don't base the smart contract code (passing .gas(2301) for example) on operations taking specific amount of gas such as transfer taking 2300 gas because those may change at any moment as discussed here: https://ethereum-magicians.org/t/remediations-for-eip-1283-reentrancy-bug/2434

  12. CREATE2 opcode allows modifying contract code without modifying the address (redeploying to same address) which might be useful for some things but is considered a hack so the ability might be removed in future ethereum upgrades and can be used for attacks by redeploying malicious code in place of valid code

    1. resources
      1. https://medium.com/@jason.carver/defend-against-wild-magic-in-the-next-ethereum-upgrade-b008247839d2
      2. https://ethereum-magicians.org/t/potential-security-implications-of-create2-eip-1014/2614
      3. openzepplin zos is considering using it OpenZeppelin/openzeppelin-sdk#152
      4. code creating redeployable metamorphic contracts using CREATE2: https://github.com/0age/metamorphic
      5. https://blog.ricmoo.com/wisps-the-magical-world-of-create2-5c2177027604
      6. https://github.com/ricmoo/will-o-the-wisp
      7. https://github.com/Zoltu/deterministic-deployment-proxy
    2. ways to protect a smart contract from redeployable smart contracts (CREATE2)
      1. dont interact with destructible contracts and libraries which use selfdestruct or CALLCODE or DELEGATECALL
      2. validate that contract was created by EOA or by CREATE and the parent contract all the way up was created by EOA or by CREATE and not by CREATE2

  13. SWC (Smart Contract Weakness Classification) collects known smart contracts weaknesses

    1. resources
      1. https://ethereum-magicians.org/t/eip-1470-smart-contract-weakness-classification-swc/1532
      2. https://github.com/SmartContractSecurity/SWC-registry
      3. ethereum/EIPs#1469
      4. https://smartcontractsecurity.github.io/SWC-registry/
    2. several weaknesses which do not appear previously in this list
      1. functions and variables visibility should always be set appropriately to prevent unauthorized access https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-100
        1. All function names are in lower camelCase (eg. sendCoin) and all event names are in upper CamelCase (eg. CoinTransfer). Input variables are in underscore-prefixed lower camelCase (eg. _offerId), and output variables are _r for pure getter (ie. constant) functions, _success (always boolean) when denoting success or failure, and other values (eg. _maxValue) for methods that perform an action but need to return a value as an identifier. Addresses are referred to using _address when generic, and otherwise if a more specific description exists (eg. _from, _to) https://github.com/ethereum/wiki/wiki/Standardized_Contract_APIs
        2. UPDATE: From solidity compiler version 0.5 the compiler enforces explicit functions visibility declarations https://solidity.readthedocs.io/en/v0.5.8/050-breaking-changes.html#explicitness-requirements
      2. in non-library contracts, the solidity compiler pragma in the contract should be locked to a specific version in which the contract was tested to avoid vulnerabilities being introduced by compilation in a different compiler version (supporting multiple compiler versions is more appropriate for libraries used by other contracts) https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-103
        1. 0xjac/ERC777#61 (comment)
      3. always validate .call return value https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-104 https://github.com/sigp/solidity-security-blog#9-unchecked-call-return-values-1
      4. never use assert unless asserting a bug which should NEVER happen. don't use it for logic reverts or for validations since it consumes all the gas that was sent to the transaction https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-110
      5. fix all warnings which can be fixed and don't use deprecated functions https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-111
      6. avoid race conditions and don't trust the miner https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-114 https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-116
        1. https://dasp.co/#item-7
        2. https://dasp.co/#item-8
        3. https://github.com/sigp/solidity-security-blog#10-race-conditions--front-running-1
        4. https://github.com/sigp/solidity-security-blog#12-block-timestamp-manipulation-1
      7. don't assume that signatures are unique and don't hash the signatures https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-117
      8. avoid shadowing variables https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-119
        1. All function names are in lower camelCase (eg. sendCoin) and all event names are in upper CamelCase (eg. CoinTransfer). Input variables are in underscore-prefixed lower camelCase (eg. _offerId), and output variables are _r for pure getter (ie. constant) functions, _success (always boolean) when denoting success or failure, and other values (eg. _maxValue) for methods that perform an action but need to return a value as an identifier. Addresses are referred to using _address when generic, and otherwise if a more specific description exists (eg. _from, _to) https://github.com/ethereum/wiki/wiki/Standardized_Contract_APIs
      9. don't trust blockchain randomness without oracles https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-1204
        1. https://dasp.co/#item-6
        2. https://www.reddit.com/r/ethereum/comments/74d3dc/smartbillions_lottery_contract_just_got_hacked/
      10. when dealing with signatures make sure to protect from replay attacks https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-121
      11. do not "bring your own crypto" https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-122 https://security.stackexchange.com/questions/18197/why-shouldnt-we-roll-our-own
      12. in multiple inheritance inherit from the more general to the more specific (the compiler will linearize the inheritance from right to left) https://consensys.github.io/smart-contract-best-practices/recommendations/#multiple-inheritance-caution https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-125
      13. be careful or even avoid using function pointers or letting user input set their value https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-127
      14. avoid dynamic length loops or actions because if they increase with time then the contract can become unusable gas-wise https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-128 https://github.com/sigp/solidity-security-blog#the-vulnerability-10
        1. dynamic length loop vulnerability and proposed solution in the following audit https://mainframe.com/Mainframe_Secondary_Audit.pdf
      15. check unicode characters of contract code to make sure they are all ascii before trusting what you see because right-to-left-override and similar unicode characters can mass up the code (can't trust etherscan verified code for example without this check) https://smartcontractsecurity.github.io/SWC-registry/docs/SWC-130
      16. Use interface type instead of the address for type safety (note that this will still not protect from malicious external implementation so never trust external calls and use validation-effects-calls pattern) https://consensys.github.io/smart-contract-best-practices/recommendations/#use-interface-type-instead-of-the-address-for-type-safety

  14. when testing contract using metamask DONT FORGET to change metamask to test network or use metamask without real ether or otherwise can mistakenly use real ether and lose it

  15. be aware that state variables are mutatable under enough confirmations (12+) happen for the modification transaction block https://ethereum.stackexchange.com/questions/8261/how-to-solve-solidity-asynchronous-problem/8265#8265

    1. so to make sure a previous function call was confirmed, need to save block height at previous call and compare it with block height at current call
    2. when transactions settle the uncle blocks do not modify state even though they give mining rewards to miners https://ethereum.stackexchange.com/questions/57727/are-uncle-blocks-unnecessary-overhead-on-blockchain

  16. consider making tokens pausable to have mitigations in case of a hack such as happened for BEC token where the pause ability saved a lot of money: https://medium.com/secbit-media/a-disastrous-vulnerability-found-in-smart-contracts-of-beautychain-bec-dbf24ddbc30e https://nvd.nist.gov/vuln/detail/CVE-2018-10299

    1. MFT is another contract with vulnerability found which was saved by being pausable: https://blog.mainframe.com/important-mft-contract-redeployed-4f0b0bd8dc3b
    2. EOS, Tron, Icon, OmiseGo, Augur, Status, Aelf, Qash, and Maker tokens are pausable https://blog.cryptofin.io/what-we-learned-from-auditing-the-top-20-erc20-token-contracts-7526ef3b6fb1
    3. although discussing security tokens, the arguments in the following link about why to make the token pausable are partially valid for utility tokens too: ethereum/EIPs#1400 (comment)
    4. allows tokens to be freezed in order to move them in the future to a different blockchain if wanted/necessary which is an important ability
    5. considered an "emergency stop mechanism" OpenZeppelin/openzeppelin-contracts#401 (comment)
    6. regarding trust, one can say that users should NOT trust tokens which are NOT pausable because they can lose their money with such tokens more easily where pausable tokens can pause in emergency and the users balances are publicly known and it would be a criminal act for the token issuer to just "walk away" with the money for no reason after pausing the token
    7. the following reddit thread was opened at the time of the infamous DAO hack WHILE IT WAS HAPPENING and you can read the comments to see how the DAO users helplessly watched their money drained without any way to stop it. they would surely prefer a pause ability https://www.reddit.com/r/ethereum/comments/4oi2ta/i_think_thedao_is_getting_drained_right_now/
    8. unlike redeployable tokens which break the trust more than pausable token (because pausable tokens are part of something the user agreed to while redeployable tokens can introduce a whole new undesired contract code. and also pausable tokens dont modify balances while upgradable tokens can cause modifications), the pausing ability is even more important than upgrading ability for security reasons. that is because the pause serves as a response to attack while the upgrade can be looked at as a second step of restoration and it can also be done manually by deploying a fixed contract and updating the balances in it.
    9. it is written here to pause the contract in "prepare for failure": https://github.com/ethereum/wiki/wiki/Safety#general-philosophy

  17. make sure that smart contracts you interact with don't use dynamic address for contracts they call because then those innocent contracts could be malicious https://github.com/sigp/solidity-security-blog#preventative-techniques-6

    1. honeypot used it:
      1. https://www.reddit.com/r/ethdev/comments/7x5rwr/tricked_by_a_honeypot_contract_or_beaten_by/
      2. https://www.reddit.com/r/ethdev/comments/7xu4vr/oh_dear_somebody_just_got_tricked_on_the_same/dubakau/

  18. when testing a contract it is better to test it using a forked mainnet than testnet just in case of small critical differences. testing by forking mainnet can be done using genache: https://www.reddit.com/r/ethdev/comments/7x5rwr/tricked_by_a_honeypot_contract_or_beaten_by/du7i96z/

  19. be careful of divisions and losing precision https://github.com/sigp/solidity-security-blog#15-floating-points-and-precision

  20. if someone withdraws money from an EOA address it does not mean he has the private keys for that address (done by taking a random signature and calculating the address from it and using the signature to withdraw from that address)

    1. https://github.com/sigp/solidity-security-blog#one-time-addresses
    2. https://medium.com/@weka/how-to-send-ether-to-11-440-people-187e332566b7

  21. total amount of ether is not an invariant depending on mining if selfdestruct is called with its own address to send ether to (this was possibly fixed by now). same if after suicide the contract kills another contract which sends fund to the original suicided contract https://swende.se/blog/Ethereum_quirks_and_vulns.html quick#2

  22. checking if address is a contract by checking extcodesize doesn't work in execution of contract constructor. note that checking if original sender is a contract can be done by "msg.sender != tx.origin", see quirk4 here: https://swende.se/blog/Ethereum_quirks_and_vulns.html

  23. make sure to use wrapper around ERC20 interface when interacting with ERC20 tokens from new solidity compiler to be able to interact with many tokens (including BNB and OMG) which implemented ERC20 without return value. can use SafeERC20 library of open zeppelin for this purpose

    1. https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca
    2. https://docs.openzeppelin.org/docs/token_erc20_safeerc20
    3. some exchanges use other wrappers: ethereum/solidity#4116 (comment)

  24. "running a sale that accepts multiple currencies at a fixed exchange rate is dangerous and bad" https://vitalik.ca/general/2017/06/09/sales.html

  25. be careful about assumptions of user being in control of his account/keys because many accounts are in control of exchanges and not users ConsenSysMesh/singulardtv-contracts#59

  26. should use checkInvariants modifier like here: https://github.com/ethereum/wiki/wiki/Safety#assert-guards

  27. when deploying use solidity optimizations to lower gas

  28. must not add a check in erc20 approve of amount being not bigger than user balance because that prevents infinite approval and approval in advance

    1. https://github.com/sec-bit/awesome-buggy-erc20-tokens/blob/master/ERC20_token_issue_list.md#a19-approve-with-balance-verify

  29. when interacting with the contract sometimes it can be good to first do a call and only if success then to send transaction https://gist.github.com/ethers/2d8dfaaf7f7a2a9e4eaa

  30. consider adding validation that transfer does not transfer to contract address, see validDestination modifier implementation here (or if not adding it then add a way for owner to withdraw contract tokens balance): https://consensys.github.io/smart-contract-best-practices/tokens/

  31. consider adding a comment in code published to etherscan with information of who to contact for issues found in the contract https://consensys.github.io/smart-contract-best-practices/documentation_procedures/

  32. Beware of negation of the most negative signed integer (Probably the solution should be to check for some invariant in the end of the calculation and not just -1) https://consensys.github.io/smart-contract-best-practices/recommendations/#beware-of-negation-of-the-most-negative-signed-integer

  33. make sure that there are no shadows built ins https://consensys.github.io/smart-contract-best-practices/recommendations/#be-aware-that-built-ins-can-be-shadowed

  34. pay attention that safemath only works with uint256, so be careful when working with other types without safemath

  35. add speed bumps delays for sensitive actions which are rare enough or/and can be delayed (also make sure to add a way to pause/cancel in case of problem unlike DAO unstoppable speed bump which was useless), this will allow time to verify/detect/respond to attacks https://consensys.github.io/smart-contract-best-practices/software_engineering/#speed-bumps-delay-contract-actions

  36. consider adding rate limiting where possible from business perspective https://consensys.github.io/smart-contract-best-practices/software_engineering/#rate-limiting

  37. prefer more granural roles than just "owner". open zeppelin is doing it now:

    1. OpenZeppelin/openzeppelin-contracts#1146
    2. OpenZeppelin/openzeppelin-contracts#1274

  38. make sure gas is limited in external calls to prevent attacks such as GasToken minting https://cryptopotato.com/ethereums-malicious-gastoken-minting-could-result-in-a-disaster-for-crypto-exchanges/

  39. it is recommended to disable a contract instead of using self destruct if that contract is sent ether by users https://solidity.readthedocs.io/en/v0.5.8/introduction-to-smart-contracts.html#deactivate-and-self-destruct

  40. to securely sign data offchain and pass it to a contract use eip 712 (by using web3.eth.personal.sign) and dont forget to add anti-replay nonce to the data signed and dont forget to add contract address to signature in case contract is redeployed to prevent replay to redeployed contract (cross contract replay attack)

    1. ethereum/EIPs#712
    2. https://solidity.readthedocs.io/en/v0.5.8/solidity-by-example.html#what-to-sign

  41. to make the contract clearer to users and avoid some misunderstandings, it is recommended to use natspec for any public/external functions/variables in the code and tools can use that to display it to end users

    1. https://solidity.readthedocs.io/en/v0.5.8/style-guide.html#natspec
    2. it needs to be published to swarm after compilation (otherwise one whitespace change can make the hash different)
    3. the hash of the json metadata is embedded inside the contract bytecode suffix https://solidity.readthedocs.io/en/v0.5.8/metadata.html#encoding-of-the-metadata-hash-in-the-bytecode
    4. https://solidity.readthedocs.io/en/v0.5.8/natspec-format.html
    5. truffle does not support natspec yet: trufflesuite/truffle#1118
    6. metamask does not support natspec yet: MetaMask/metamask-extension#2501
    7. radspec is an implementation of natspec referenced by solidity documentation https://github.com/aragon/radspec#aside-why-is-natspec-unsafe
    8. here it says it is hard to find the swarm where the json was deployed: https://blog.zeppelin.solutions/deconstructing-a-solidity-contract-part-vi-the-swarm-hash-70f069e22aef
    9. Supporting natspec for state variables is stale for 1.5 years: ethereum/solidity#3418
    10. Here it says that radspec and natspec are not compatible: aragon/radspec#67
    11. seems natspec is not well supported right now but may be better supported in the future

  42. dont trust hash of msg.data because some unused bits can be used to change the hash of msg.data https://solidity.readthedocs.io/en/v0.5.8/security-considerations.html#minor-details

  43. enable SMTChecker when it will go to production https://solidity.readthedocs.io/en/v0.5.8/layout-of-source-files.html#smt-checker

  44. Take care if you perform calendar calculations using these units, because not every year equals 365 days and not even every day has 24 hours because of leap seconds. Due to the fact that leap seconds cannot be predicted, an exact calendar library has to be updated by an external oracle https://en.wikipedia.org/wiki/Leap_second

  45. careful when using blockhash. The block hashes are not available for all blocks for scalability reasons. You can only access the hashes of the most recent 256 blocks, all other values will be zero

  46. be aware that packed abi encoding can be ambigious and hash collisions are possible for it https://solidity.readthedocs.io/en/v0.5.8/units-and-global-variables.html#abi-encoding-and-decoding-functions

  47. If you use ecrecover, be aware that a valid signature can be turned into a different valid signature without requiring knowledge of the corresponding private key https://solidity.readthedocs.io/en/v0.5.8/units-and-global-variables.html#mathematical-and-cryptographic-functions

  48. ethereum blockchain implements some functions (sha256, ripemd160 or ecrecover) as "precompiled" contracts which are deployed after their first use and thus first usage takes more gas and might run out of gas. relevant only for private blockchains https://solidity.readthedocs.io/en/v0.5.8/units-and-global-variables.html#mathematical-and-cryptographic-functions

  49. note that if C is a contract then type(C).creationCode and type(C).runtimeCode have a lot of gotchas so be careful using them https://solidity.readthedocs.io/en/v0.5.8/units-and-global-variables.html#type-information

  50. You should avoid excessive recursion, as every function call uses up at least one stack slot and there are at most 1024 slots available. for external calls (external calls generate message call while internal non prefixed calls are translated into jump opcodes) only 63/64th of the gas can be forwarded in a message call, which causes a depth limit of a little less than 1000 in practice

  51. The low-level functions call, delegatecall and staticcall return true as their first return value if the called account is non-existent, as part of the design of EVM. Existence must be checked prior to calling if desired https://solidity.readthedocs.io/en/v0.5.8/control-structures.html#error-handling-assert-require-revert-and-exceptions

  52. When dealing with function arguments or memory values, there is no inherent benefit to use types less than 256 bits because the compiler does not pack these values https://solidity.readthedocs.io/en/v0.5.8/miscellaneous.html#layout-of-state-variables-in-storage

  53. use "indexed" keyword for events parameters which you wish to be indexed as topics. up to 4 indexed parameters are allowed per event. to have both the values and fast search, can add same value both as indexed parameter and as a not indexed parameter https://solidity.readthedocs.io/en/v0.5.8/abi-spec.html#events

  54. It is highly suspicious if a contract was compiled with a compiler that contains a known bug and the contract was created at a time where a newer compiler version containing a fix was already released (note that most bugs are not critical but should periodically check the bugs list anyway, especially for new versions in the bottom here https://github.com/ethereum/solidity/blob/develop/docs/bugs_by_version.json) https://solidity.readthedocs.io/en/v0.5.8/bugs.html

  55. The operators || and && apply the common short-circuiting rules. This means that in the expression f(x) || g(y), if f(x) evaluates to true, g(y) will not be evaluated even if it may have side-effects

  56. The type byte[] is an array of bytes, but due to padding rules, it wastes 31 bytes of space for each element (except in storage). It is better to use the bytes type instead. As a general rule, use bytes for arbitrary-length raw byte data and string for arbitrary-length string (UTF-8) data. If you can limit the length to a certain number of bytes, always use one of the value types bytes1 to bytes32 because they are much cheaper https://solidity.readthedocs.io/en/v0.5.8/types.html#fixed-size-byte-arrays https://solidity.readthedocs.io/en/v0.5.8/types.html#bytes-and-strings-as-arrays

  57. make sure that hardcoded literal addresses pass the checksum test or otherwise they are treated as regular rational numbers https://solidity.readthedocs.io/en/v0.5.8/types.html#address-literals

  58. Underscores should be used to separate the digits of a numeric literal to aid readability and prevent mistakes, they do not influence the number https://solidity.readthedocs.io/en/v0.5.8/types.html#rational-and-integer-literals

  59. pay attention that "string" type is UTF8 encoded so by "bytes(s)[7] = 'x';" Keep in mind that you are accessing the low-level bytes of the UTF-8 representation, and not the individual characters https://solidity.readthedocs.io/en/v0.5.8/types.html#bytes-and-strings-as-arrays

  60. note that delete has no effect on mappings https://solidity.readthedocs.io/en/v0.5.8/types.html#delete

  61. explicit overflowing conversions of negative integers is possible so be careful https://solidity.readthedocs.io/en/v0.5.8/types.html#explicit-conversions

  62. always re-check the event data and do not rely on the search result based on the indexed parameters alone because the encoding of a struct is ambiguous if it contains more than one dynamically-sized array https://solidity.readthedocs.io/en/v0.5.8/abi-spec.html#encoding-of-indexed-event-parameters

  63. readable and consistent code will lead to easier bug detection during development and eventually lead to a more secure code so follow the conventions here: https://solidity.readthedocs.io/en/v0.5.8/style-guide.html

  64. library view functions do not have run-time checks that prevent state modifications https://solidity.readthedocs.io/en/v0.5.8/contracts.html#view-functions

  65. Pure state reading are not enforced by evm https://solidity.readthedocs.io/en/v0.5.8/contracts.html#pure-functions

  66. anonymous keyword for events makes the event name not be stored which makes the contract gas cheaper to deploy and call the event cheaper too. should be used when contract has only one event where the event name is not enforced by some standard because then all logs are known to be from this event and no need to store the event name ethereum/solidity#6791 (comment)

  67. for tokens contract (and some other sensitive contracts too), in addition to being pausable, there should be an upgrading/migration strategy as described here: ethereum/EIPs#644 (comment)

    1. See SafeUpgradeableTokenERC20 contract implementation here: https://github.com/guylando/EthereumSmartContracts

  68. add events where possible/useful to make auditing and debugging and findings misbehaviors easier for deployed contracts

  69. in tokens transfer functions prevent mistaken transfers to the token address by adding something like require( _to != address(this) ), see section 3.1 here: EthereumCommonwealth/Auditing#236

  70. in tokens decreaseAllowance function set to zero if bigger than balance instead of reverting so that for example when someone wants to remove allowance from the attacker quickly using decreaseAllowance then to stop revert from happening which would give more time to the attacker to use his allowance, see section 3.2 here: EthereumCommonwealth/Auditing#236

  71. to get familiar with possible vulnerabilities and get ideas, Go over previous reports in https://github.com/EthereumCommonwealth/Auditing/issues?q=is%3Aissue+is%3Aclosed and when done developing the contract consider submitting it for auditing to https://github.com/EthereumCommonwealth/Auditing

  72. be careful when using SafeERC20 with non compliant token which has a non-reverting fallback function because SafeERC20 functions such as safeTransferFrom will give no indication that transferFrom is not implemented in the token and the fallback function will be called instead silently, see: OpenZeppelin/openzeppelin-contracts#1769

  73. be careful when calling other contracts and expecting them to fail on a problem because if the other contract does not implement the called function and implements a non-reverting fallback function then the transaction will be a success even though the desired function was not called

  74. be careful when providing a public burn function to users to burn their tokens because there is a difference between sending to irrecoverable address and burning which decreases total supply because the first is not visible to the non-technical investors while the second one will change the token total supply on coinmarketcap which might cause undesired reaction from non-technical investors who follow the token in coinmarketcap and similar tools. Another difference is that discussions about the retrieval of locked tokens refer more to tokens sent to invalid address than to burned tokens, which shows that sending tokens to invalid address may still legally make those tokens belong to their owner, see the discussion here: ethereum/EIPs#867 (comment)

  75. use modifiers to make code cleaner and understandable (modifiers are macros compiled inline). cleaner and more readable code improves chances of discovering bugs earier and writing better and more secure code.

  76. security tools and other useful tools to use to scan smart contracts and develop smart contracts (FIRST CHECK THAT SCANNERS DON'T HAVE DISTRIBUTION RIGHTS FOR THE CONTRACT CODE):

    1. https://github.com/ConsenSys/mythril-classic
      1. to run on windows
        1. first install docker desktop app
        2. if the contract is using solidity compiler 0.4.24 and contract name is DDD and it is located in C:\Users\user\Downloads\target.sol then run "docker run --rm -v C:\Users\user\Downloads:/Downloads mythril/myth -x /Downloads/target.sol:DDD -mether_thief --verbose-report --solv 0.4.24"
        3. the -v command maps the folder on the left of ':' in host to the folder on the right of ':' inside the docker
        4. the --rm command cleans the docker after finishing running the command so if want to run several related/dependent commands then omit -rm and add it only in the end for cleanup https://docs.docker.com/engine/reference/run/#clean-up---rm
      2. note: after running it need to wait about 10-20 minutes for it to finish (it does not show any output in that time so be patient) so better idea is not to wait for it and to do something else while it is running
      3. CONCLUSION: YES. it is good and should always use it to get general direction to existing vulnerabilities (it does not produce exact exploit but gives a direction if it finds a vulnerability)
    2. http://remix.ethereum.org
      1. compiling in remix perform static analysis and might detect bugs
      2. CONCLUSION: YES. it should be used but most if not all of the warnings are false positives so not to waste too much time on them
    3. https://github.com/crytic/slither
      1. requires python 3.6
      2. pip install slither-analyzer
      3. download latest solc compiler version and put it in the PATH (or otherwise slither will throw "file not found" error) https://github.com/ethereum/solidity/releases
      4. reopen cmd and run "slither contract_file_path_and_filename.sol"
      5. CONCLUSION: YES. it is good and should always use it to detect small possible optimizations and fixes
    4. https://github.com/crytic/eth-security-toolbox
      1. very heavy and takes a lot of time to download
      2. contains slither, echidna, manticore and some other tools
      3. docker pull trailofbits/eth-security-toolbox (takes 10+ minutes)
      4. docker run -it -v C:\Users\user\Downloads:/Downloads trailofbits/eth-security-toolbox (replace "user" with the windows user name)
      5. slither /Downloads/contract.sol (where contract.sol is inside the windows user downloads folder)
      6. echidna-test /Downloads/contract.sol ContractName
      7. CONCLUSION: YES. the eth-security-toolbox is slow to download but convinient if many of the tools inside it are used
    5. https://github.com/crytic/echidna
      1. use using eth-security-toolbox docker
      2. requires to write tests to make it work so it is not a drop in solution
      3. CONCLUSION: NO. not so useful for a simple quick security check since it requires development
    6. https://github.com/trailofbits/manticore
      1. use using eth-security-toolbox docker
      2. trailofbits/manticore#1382
      3. trailofbits/manticore#1315
      4. CONCLUSION: NO. has a lot of bugs making it unusable right now
    7. solidity security vscode extension https://marketplace.visualstudio.com/items?itemName=tintinweb.solidity-visual-auditor
      1. CONCLUSION: use during development to fix problems as they are typed into the code
    8. https://github.com/melonproject/oyente
      1. enzymefinance/oyente#334
      2. enzymefinance/oyente#268
      3. CONCLUSION: NO. outdated and deprecated and not maintained anymore
    9. https://github.com/raineorshine/solgraph
      1. does not support new solidity versions raineorshine/solgraph#39
      2. CONCLUSION: NO. outdated and deprecated and not maintained anymore
    10. https://github.com/MAIAN-tool/MAIAN
      1. CONCLUSION: NO. outdated and deprecated and not maintained anymore
    11. https://github.com/quoscient/octopus
      1. CONCLUSION: NO. not well documented and not actively developed
    12. https://github.com/cleanunicorn/karl
      1. CONCLUSION: NO. not actively developed and installation on windows doesn't even work. it just runs mithril on deployed contracts so if the goal is to analyze your contract then better run mithril directly
    13. https://github.com/ConsenSys/surya
      1. npm install -g surya
      2. "surya describe contract.sol" quickly displays a summary of the functions and contracts in the file
      3. "surya graph contract.sol > dot -Tpng > contract.png" creates contract calls graph (dot tool is available here: https://graphviz.gitlab.io)
      4. graph creation does not work Consensys/surya#76
      5. CONCLUSION: YES. can be good for general contract analysis, can help notice problems but does not provide vulnerabilities information
    14. SECBIT Solidity Static Analyzer
      1. https://github.com/sec-bit/adelaide/blob/secbit-ssae/README.secbit.md
      2. available as vscode extension: https://github.com/sec-bit/vscode-secbit-ssae
      3. https://marketplace.visualstudio.com/items?itemName=SECBIT.vscode-secbit-ssae
      4. CONCLUSION: NO. does not work and is not actively developed or maintained
    15. https://github.com/b-mueller/scrooge-mcetherface
      1. ONLY RUN ON TEST ENVIRONMENT
      2. doesn't have a docker and uses mithril which is not compatible with windows so easiest way to install is in case a docker is installer such as the eth-security-toolbox docker then run:
        1. if container is running
          1. "docker ps"
          2. copy the CONTAINER ID of the ubuntu docker instance
          3. "docker attach {container-id}"
        2. if container is not running (or running not as root then "exit" and run this to start as root)
          1. docker run -u 0 -v C:\Users\user\Downloads:/Downloads -it trailofbits/eth-security-toolbox
        3. then run installation steps in that shell https://github.com/b-mueller/scrooge-mcetherface#installation
      3. CONCLUSION: NO. not actively developed or maintained or supported and installation on windows doesn't work. it just runs mithril on deployed contracts so if the goal is to analyze your contract then better run mithril directly
    16. https://github.com/protofire/solhint
      1. npm install -g solhint
      2. solhint init-config
      3. solhint contract.sol
      4. CONCLUSIONS: YES. useful to get formatting tips but doesn't give security tips
    17. https://github.com/duaraghav8/Ethlint
      1. npm install -g ethlint
      2. solium --init
      3. solium -f contract.sol
      4. CONCLUSION: YES. useful to get suggestions on both formatting and coding and security
    18. https://github.com/smartdec/smartcheck
      1. npm install @smartdec/smartcheck -g
      2. smartcheck -p contract.sol
      3. can run automatically using https://tool.smartdec.net/
      4. CONCLUSION: YES. gives security suggestions which are 90%-100% false positives so use it but reverify the suggestions. dont use the command line tool because it is unclear, only use the online interface.
    19. https://github.com/eth-sri/securify https://securify.chainsecurity.com/
      1. CONCLUSION: NO. does not work. normal and docker installations fail and when attempting to use their website to scan either pasted code or github repository then it is stuck in infinite 404 XHR errors until finally displays "bad gateway" error
    20. https://github.com/tagomaru/truffle-sca2t
      1. basically only integrates running mythx using truffle run mythx command
      2. CONCLUSION: NO. should prefer to use mythx official integration (truffle-security npm package) which is officially supported and more actively developed
    21. https://github.com/rajeevgopalakrishna/Solstice
      1. https://medium.com/@rajeevgopalakrishna/solstice-solidity-security-tool-for-investigative-contract-examination-1fafda26d038
      2. CONCLUSION: NO. official installation steps give syntax error and it is not actively developed or maintained
    22. mythx + mythos + official truffle integration
      1. npm install -g truffle-security
      2. truffle run verify
      3. https://github.com/ConsenSys/truffle-security
      4. https://docs.mythx.io/en/latest/tooling/truffle.html
      5. https://medium.com/consensys-diligence/mythx-and-truffle-security-painless-smart-contract-security-testing-6d0fe5e938da
      6. https://github.com/cleanunicorn/mythos
      7. https://docs.mythx.io/en/latest/tooling/mythos.html
      8. basically mythx is a server side api and mythos and truffle-security and other tools are just client side tools integrating with mythx api so doesn't matter which to use.
      9. CONCLUSION: YES. using truffle-security produced no output so used mythos instead and it performed security analysis as desired. should use it always to get security analysis tips
    23. https://github.com/cgewecke/eth-gas-reporter
      1. if solidity-coverage throws error about eth-gas-reporter not found then make sure everything is installed in same scope (installing eth-gas-reporter globally might solve the problem)
      2. CONCLUSION: YES. useful to find problematic functions which take too much gas in unit testing of contracts
    24. https://github.com/sc-forks/solidity-coverage
      1. look at open zeppelin repository for coverage addition to truffle-config.json and for .solcover.js file contents but in .solcover.js only use norpc:true and change skipFiles to point to your truffle Migrations file and also add there your mockup contracts files
      2. npm install --save-dev solidity-coverage
      3. ./node_modules/.bin/solidity-coverage
      4. node_modules.bin\solidity-coverage
      5. by default returns errors for solidity 0.5.0+ sc-forks/solidity-coverage#316
        1. open zeppelin installed https://github.com/rotcivegaf/solidity-coverage fork for solidity 0.5.0+ support so can do the same
        2. but latest solidity-coverage development happens on this branch: https://github.com/sc-forks/solidity-coverage/tree/leapdao
        3. the active branch can be installed using: npm install --save-dev git://github.com/sc-forks/solidity-coverage#leapdao
      6. following those intructions to run on windows: https://github.com/sc-forks/solidity-coverage/blob/master/docs/faq.md#running-on-windows
        1. run "node_modules.bin\testrpc-sc -p 8555" in another terminal before running the coverage
      7. if get "Returned error: Exceeds block gas limit" error then delete gas, gasPrice configs from coverage config in truffle-config.json
      8. if get "invalid opcode" errors then better not to run testrpc-sc and instead use ganache-cli and change truffle-config.json coverage port to 8454 ganache cli port https://ethereum.stackexchange.com/questions/69866/solc-error-invalid-opcode-but-works-in-truffle
        1. UPDATE: using ganache-cli makes other errors happen and does not allow to get coverage report so the better solution is to update solidity-coverage to the active branch as described above
      9. if get "base fee exceeds gas limit" error then in truffle-config.json increase or delete gas config from coverage network config
      10. if get "out of gas" errors (because instrumentation of contracts makes them too big) then
        1. add "--allowUnlimitedContractSize" parameter to the testchain execution to allow contracts above 24KB to be deployed (otherwise there is an out of gas error even though the reason is the size and not gas https://github.com/ethereum/EIPs/blob/master/EIPS/eip-170.md)
        2. increase gas parameter of coverage network config in truffle-config.json to 0xfffffffffff (what is sent to the blockchain) AND increase ganache-cli gas limit to 0xfffffffffff (default is around 6.7 million) using -l parameter (what is supported in the blockchain): "ganache-cli -l 0xfffffffffff"
      11. creates very helpful instanbul html reports which can quickly help add the missing test cases
      12. CONCLUSION: YES. might be hard to make it work but it is a MUST HAVE tool to use to improve the testing coverage and after all configurations being set well it works as desired