Best Practices

Lessons learned from smart contract audits.

Basic Coding Bugs

Severity: Critical

• Constructor Mismatch: whether the contract name and its constructor are unidentical.
• Ownership Takeover: whether the transfer ownership function is vulnerable.
• Redundant Fallback Function: whether the contract has a redundant fallback function.
• Overflows and Underflows: whether the contarct has general overflows or underflow vulnerabilities.
• Reentrancy: an issue when code can call back into your contract and change statem such as withdrawing ETH.

Severity: High

• Money-Giving Bag: whether the contract returns funds to an arbitrary address.
• Blackhole: whether the contract locks ETH indefinitely: merely in without an out.

Severity: Medium

• Unauthorized Self-Destruct: whether the contract can be killed by any arbitrary address.
• Revert DoS: whether the contract can be killed by any arbitrary address.
• Unchecked External Call: whether the contract has any external call without checking the return value.
• Gasless Send: whether the contract is vulnerable to hasless send.
• Send Instead of Transfer: whether the contract uses the send function instead of transfer.
• Costly Loop: whether the contract has any costly loop which may lead to Out-Of-Gas exception.
• (Unsafe) Use of Untrusted Libraries: whether the contract uses any suspicious libraries.
• Transaction Ordering Dependence: whether the final state of the contract depends on the order of the transactions.
• Deprecated Uses: wether the contract uses the deprecated tx.origin to perform the authorization.

Additional Recommendations

• Avoid Use of Variadic Byte Array: use of fixed-size byte array is better than that of byte[], as the latter is a waste of space.
• Make Visibility Level Explicit: assign explicit visibility specifiers for functions and state variables.
• Make Type Inference Explicit: avoid the keyword var to specify the type ik.e. it asks the compiler to deduce the type, which is not safe, esp in a loop.
• Adhere to Function Declaration Strictly: solidity compiler (v0.4.23) enforces strict ABI length checks for return data from calls(), which may break the execution if the function implementation does NOT follow its declaration (e.g., no return in implementing transfers() of ERC20 tokens.

References

• axic. Enforcing ABI length checks for return data from calls can be breaking
• MITRE. CWE-1041: Use of Redundant Code
• MITRE. CWE-841: Improper Enforcement of Behavioral Workflow
• MITRE. CWE CATEGORY: Bad Coding Practices
• MITRE. CWE CATEGORY: Business Logic Errors
• MITRE. CWE VIEW: Development Concepts
• OWASP. Risk Rating Methodology
• PeckShield. ALERT: New batchOverflow Bug in Multiple ERC20 Smart Contracts (CVE-2018- 10299)
• PeckShield. New burnOverflow Bug Identified in Multiple ERC20 Smart Contracts (CVE-2018- 11239)
• PeckShield. New multiOverflow Bug Identified in Multiple ERC20 Smart Contracts (CVE-2018-10706)
• PeckShield. New proxyOverflow Bug in Multiple ERC20 Smart Contracts (CVE-2018-10376)
• PeckShield. PeckShield Inc
• PeckShield. Your Tokens Are Mine: A Suspicious Scam Token in A Top Exchange
• Solidity. Warnings of Expressions and Control Structures