Skip to content

Latest commit

 

History

History
325 lines (321 loc) · 113 KB

Security Common Pitfalls.md

File metadata and controls

325 lines (321 loc) · 113 KB

Security Pitfalls Checklists

Sources

Item Checks Further Research Description Recommendation Reference
Solidity versions Using very old versions of Solidity Consider using one of these versions: 0.8.18. link
Unlocked pragma Redflag Contracts should be deployed using the same compiler version/flags with which they have been tested. Locking the pragma (for e.g. by not using ^ in pragma solidity 0.5.10) ensures that contracts do not accidentally get deployed using an older compiler version with unfixed bugs. Lock the pragma version and also consider known bugs (https://github.com/ethereum/solidity/releases) for the compiler version that is chosen. link
Multiple Solidity pragma Redflag It is better to use one Solidity compiler version across all contracts instead of different versions with different bugs and security checks. link
Incorrect access control Redflag Contract functions executing critical logic should have appropriate access control enforced via address checks (e.g. owner, controller etc.) typically in modifiers. Missing checks allow attackers to control critical logic. These vulnerabilities can occur when contracts use the deprecated tx.origin to validate callers, handle large authorization logic with lengthy require and make reckless use of delegatecall in proxy libraries or proxy contracts.. Use modifiers like AccessControl, Ownable, TimelockController link
link 2
Unprotected withdraw function Redflag Unprotected (external/public) function calls sending Ether/tokens to user-controlled addresses may allow users to withdraw unauthorized funds. Implement controls so withdrawals can only be triggered by authorized parties or according to the specs of the smart contract system. link
Unprotected call to selfdestruct Redflag A user/attacker can mistakenly/intentionally kill the contract. Protect access to such functions. link
Modifier side-effects Redflag Modifiers should only implement checks and not make state changes and external calls which violates the checks-effects-interactions pattern. These side-effects may go unnoticed by developers/auditors because the modifier code is typically far from the function implementation. Do not make state changes and external calls in modifiers link
Incorrect modifier Redflag If a modifier does not execute _ or revert, the function using that modifier will return the default value causing unexpected behavior. All the paths in a modifier must execute _ or revert. link
Constructor names Redflag Before solc 0.4.22, constructor names had to be the same name as the contract class containing it. Misnaming it wouldn’t make it a constructor which has security implications. Solc 0.4.22 introduced the constructor keyword. Until solc 0.5.0, contracts could have both old-style and new-style constructor names with the first defined one taking precedence over the second if both existed, which also led to security issues. Solc 0.5.0 forced the use of the constructor keyword. Only declare one constructor, preferably using the new scheme constructor(...) instead of function <contractName>(...). link 1
link 2
Void constructor Redflag Calls to base contract constructors that are unimplemented leads to misplaced assumptions. Check if the constructor is implemented or remove call if not. Remove the constructor call. link
Implicit constructor callValue check Redflag The creation code of a contract that does not define a constructor but has a base that does, did not revert for calls with non-zero callValue when such a constructor was not explicitly payable. This is due to a compiler bug introduced in v0.4.5 and fixed in v0.6.8. Starting from Solidity 0.4.5 the creation code of contracts without explicit payable constructor is supposed to contain a callvalue check that results in contract creation reverting, if non-zero value is passed. However, this check was missing in case no explicit constructor was defined in a contract at all, but the contract has a base that does define a constructor. In these cases it is possible to send value in a contract creation transaction or using inline assembly without revert, even though the creation code is supposed to be non-payable. link
Controlled delegatecall Redflag delegatecall() or callcode() to an address controlled by the user allows execution of malicious contracts in the context of the caller’s state. Ensure trusted destination addresses for such calls. link
Reentrancy vulnerabilities Redflag Untrusted external contract calls could callback leading to unexpected results such as multiple withdrawals or out-of-order events. Use check-effects-interactions pattern or reentrancy guards. The best practices to avoid Reentrancy weaknesses are:

- Make sure all internal state changes are performed before the call is executed. This is known as the Checks-Effects-Interactions pattern
- Use a reentrancy lock (ie. OpenZeppelin's ReentrancyGuard).
link
ERC777 callbacks and reentrancy Redflag ERC777 tokens allow arbitrary callbacks via hooks that are called during token transfers. Malicious contract addresses may cause reentrancy on such callbacks if reentrancy guards are not used. Use a reentrancy lock (ie. OpenZeppelin's ReentrancyGuard). link
Avoid transfer()**/**send() as reentrancy mitigations Redflag Although transfer() and send() have been recommended as a security best-practice to prevent reentrancy attacks because they only forward 2300 gas, the gas repricing of opcodes may break deployed contracts. Use call() instead, without hardcoded gas limits along with checks-effects-interactions pattern or reentrancy guards for reentrancy protection. link 1

link 2
Private on-chain data Redflag Marking variables private does not mean that they cannot be read on-chain. Private data should not be stored unencrypted in contract code or state but instead stored encrypted or off-chain. link
Weak PRNG Redflag PRNG relying on block.timestamp, now or blockhash can be influenced by miners to some extent and should be avoided. - Using external sources of randomness via oracles, and cryptographically checking the outcome of the oracle on-chain. e.g. Chainlink VRF. This approach does not rely on trusting the oracle, as a falsly generated random number will be rejected by the on-chain portion of the system.
- Using commitment scheme, e.g. RANDAO.
- Using external sources of randomness via oracles, e.g. Oraclize. Note that this approach requires trusting in oracle, thus it may be reasonable to use multiple oracles.
- Using Bitcoin block hashes, as they are more expensive to mine.
link
Block values as time proxies Redflag block.timestamp and block.number are not good proxies (i.e. representations, not to be confused with smart contract proxy/implementation pattern) for time because of issues with synchronization, miner manipulation and changing block times. Developers should write smart contracts with the notion that block values are not precise, and the use of them can lead to unexpected effects. Alternatively, they may make use oracles. link
Integer overflow/underflow Redflag Not using OpenZeppelin’s SafeMath (or similar libraries) that check for overflows/underflows may lead to vulnerabilities or unexpected behavior if user/attacker can control the integer operands of such arithmetic operations. Solc v0.8.0 introduced default overflow/underflow checks for all arithmetic operations. It is recommended to use vetted safe math libraries for arithmetic operations consistently throughout the smart contract system. link 1

link 2
Divide before multiply Redflag Performing multiplication before division is generally better to avoid loss of precision because Solidity integer division might truncate. Consider ordering multiplication before division. link
Transaction order dependence Redflag Race conditions can be forced on specific Ethereum transactions by monitoring the mempool. For example, the classic ERC20 approve() change can be front-run using this method. Do not make assumptions about transaction order dependence. A commit reveal hash scheme prevents race conditions in reward submissions by storing (salt, address, answer) hashes in a contract. To claim a reward, users submit (salt, answer) and the contract checks against stored hashes. For ERC20 race issues, adding an expected value field to the "approve" function can resolve the problem, though it deviates from the ERC20 standard. Users can also mitigate the issue by setting approvals to zero before changing them. link
ERC20 approve() race condition Redflag The most common network race condition is found in the ERC20 token standard's 'approve' function, allowing one address to authorize another to spend tokens on their behalf. When Alice changes her approval from n to m tokens for Eve, Eve, who knows this change due to running an Ethereum node, submits a transferFrom request with a higher gas price, resulting in Eve receiving n + m tokens instead of the correct maximum of max(n,m). Use safeIncreaseAllowance() and safeDecreaseAllowance() from OpenZeppelin’s SafeERC20 implementation to prevent race conditions from manipulating the allowance amounts. link
Signature malleability Redflag The ecrecover function is susceptible to signature malleability which could lead to replay attacks. - A signature should never be included into a signed message hash to check if previously messages have been processed by the contract.
- Consider using OpenZeppelin’s ECDSA library.
link 1

link 2

link 3
ERC20 transfer() does not return boolean erc20-interface Incorrect return values for ERC20 functions. A contract compiled with Solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing. Set the appropriate return values and types for the defined ERC20 functions.

Use OpenZeppelin’s SafeERC20 wrappers.
link 1

link 2
Incorrect return values for ERC721 ownerOf() erc721-interface Contracts compiled with solc >= 0.4.22 interacting with ERC721 ownerOf() that returns a bool instead of address type will revert. Use OpenZeppelin’s ERC721 contracts. link
Unexpected Ether and this.balance: Redflag A contract can receive Ether via payable functions, selfdestruct(), coinbase transaction or pre-sent before creation. Contract logic depending on this.balance can therefore be manipulated. Avoid strict equality checks for the Ether balance in a contract. link

link
fallback vs receive() Redflag Check that all precautions and subtleties of fallback/receive functions related to visibility, state mutability and Ether transfers have been considered. link 1

link 2
Dangerous strict equalities Redflag Use of strict equalities (==) with tokens/Ether can accidentally/maliciously cause unexpected behavior. Consider using >= or <= instead of == for such variables depending on the contract logic. Don't use strict equality to determine if an account has enough Ether or tokens. link
Locked Ether Redflag Contracts that accept Ether via payable functions but without withdrawal mechanisms will lock up that Ether. Remove payable attribute or add withdraw function. link
Dangerous usage of tx.origin Redflag Use of tx.origin for authorization may be abused by a MITM malicious contract forwarding calls from the legitimate user who interacts with it. tx.origin should not be used for authorization. Use msg.sender instead. link
Contract check Redflag Checking if a call was made from an Externally Owned Account (EOA) or a contract account is typically done using extcodesize check which may be circumvented by a contract during construction when it does not have source code available. Checking if tx.origin == msg.sender is another option. Both have implications that need to be considered. link
Deleting a mapping within a struct mapping-deletion Deleting a struct that contains a mapping will not delete the mapping contents which may lead to unintended consequences. The remaining data may be used to compromise the contract. Use a lock mechanism instead of a deletion to disable structure containing a mapping. link
Tautology or contradiction Redflag Tautologies (always true) or contradictions (always false) indicate potential flawed logic or redundant checks. e.g. x >= 0 which is always true if x is uint. Fix the incorrect comparison by changing the value type or the comparison. link
Boolean constant boolean-cst Use of Boolean constants (true/false) in code (e.g. conditionals) is indicative of flawed logic. Verify and simplify the condition. link
Boolean equality boolean-equal Boolean variables can be checked within conditionals directly without the use of equality operators to true/false.

Severity: Informational
Remove the equality to the boolean constant. link
State-modifying functions constant-function-asm Functions that modify state (in assembly or otherwise) but are labelled constant/pure/view revert in solc >=0.5.0 (work in prior versions) because of the use of STATICCALL opcode. Ensure the attributes of contracts compiled prior to Solidity 0.5.0 are correct. link
Return values of low-level calls Redflag Ensure that return values of low-level calls (call/callcode/delegatecall/send/etc.) are checked to avoid unexpected failures. If you choose to use low-level call methods, make sure to handle the possibility that the call will fail by checking the return value. link
Account existence check for low-level calls Redflag The use of low-level calls is error-prone. Low-level calls do not check for code existence or call success.

Low-level calls call/delegatecall/staticcall return true even if the account called is non-existent (per EVM design). Account existence must be checked prior to calling if needed.
Avoid low-level calls. Check the call success. If the call is meant for a contract, check for code existence. link
Dangerous shadowing Redflag Local variables, state variables, functions, modifiers, or events with names that shadow (i.e. override) builtin Solidity symbols e.g. now or other declarations from the current scope are misleading and may lead to unexpected usages and behavior. Rename the local variables, state variables, functions, modifiers, and events that shadow a builtin symbol. link
Dangerous state variable shadowing Redflag Shadowing state variables in derived contracts may be dangerous for critical variables such as contract owner (for e.g. where modifiers in base contracts check on base variables but shadowed variables are set mistakenly) and contracts incorrectly use base/shadowed variables. Do not shadow state variables. Review storage variable layouts for your contract systems carefully and remove any ambiguities. Always check for compiler warnings as they can flag the issue within a single contract. link
Pre-declaration usage of local variables variable-scope Usage of a variable before its declaration (either declared later or in another scope) leads to unexpected behavior in solc < 0.5.0 but solc >= 0.5.0 implements C99-style scoping rules where variables can only be used after they have been declared and only in the same or nested scopes. Move all variable declarations prior to any usage of the variable, and ensure that reaching a variable declaration does not depend on some conditional if it is used unconditionally. link
Costly operations inside a loop Redflag Operations such as state variable updates (use SSTOREs) inside a loop cost a lot of gas, are expensive and may lead to out-of-gas errors. Optimizations using local variables are preferred.

Severity: Informational
Use a local variable to hold the loop computation result. link
Calls inside a loop Redflag Calls to external contracts inside a loop are dangerous (especially if the loop index can be user-controlled) because it could lead to DoS if one of the calls reverts or execution runs out of gas. Avoid calls within loops, check that loop index cannot be user-controlled or is bounded.

This is especially relevant for payments, where it is better to let users withdraw funds rather than push funds to them automatically (this also reduces the chance of problems with the gas limit).
It is recommended to follow call best practices:

- Avoid combining multiple calls in a single transaction, especially when calls are executed as part of a loop
- Always assume that external calls can fail
- Implement the contract logic to handle failed calls
link
DoS with block gas limit Redflag Smart contracts and functions in Ethereum consume gas based on computation needs, with a block gas limit setting an upper threshold. Coding practices like looping through unknown-size arrays may lead to DoS when gas costs exceed the block limit. Caution is advised when you expect to have large arrays that grow over time. Actions that require looping across the entire data structure should be avoided.

If you absolutely must loop over an array of unknown size, then you should plan for it to potentially take multiple blocks, and therefore require multiple transactions.
link
Missing events Redflag Events for critical state changes (e.g. owner and other critical parameters) should be emitted for tracking this off-chain Emit an event for critical parameter changes. link
Unindexed event parameters Redflag Parameters of certain events are expected to be indexed (e.g. ERC20 Transfer/Approval events) so that they’re included in the block’s bloom filter for faster access. Failure to do so might confuse off-chain tooling looking for such indexed events. Add the indexed keyword to event parameters that should include it, according to the ERC20 specification. link
Incorrect event signature in libraries Redflag Contract types used in events in libraries cause an incorrect event signature hash. Instead of using the type `address` in the hashed signature, the actual contract name was used, leading to a wrong hash in the logs. This is due to a compiler bug introduced in v0.5.0 and fixed in v0.5.8. link
Dangerous unary expressions Redflag Unary expressions such as x =+ 1 are likely errors where the programmer really meant to use x += 1. Unary + operator was deprecated in solc v0.5.0. The weakness can be avoided by performing pre-condition checks on any math operation or using a vetted library for arithmetic calculations such as SafeMath developed by OpenZeppelin. link
Missing zero address validation missing-zero-check Setters of address type parameters should include a zero-address check otherwise contract functionality may become inaccessible or tokens burnt forever. Check that the address is not zero. link
Critical address change Redflag Changing critical addresses in contracts should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one (i.e. claims ownership). This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible. link 1

link 2
assert()/require() state change Redflag Invariants in assert() and require() statements should not modify the state per best practices. Consider whether the condition checked in the assert() is actually an invariant. If not, replace the assert() statement with a require() statement.

If the exception is indeed caused by unexpected behaviour of the code, fix the underlying bug(s) that allow the assertion to be violated.
link
require() vs assert() Redflag require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking. Between solc 0.4.10 and 0.8.0, require() used REVERT (0xfd) opcode which refunded remaining gas on failure while assert() used INVALID (0xfe) opcode which consumed all the supplied gas. link
Deprecated keywords Redflag Use of deprecated functions/operators such as block.blockhash() for blockhash(), msg.gas for gasleft(), throw for revert(), sha3() for keccak256(), callcode() for delegatecall(), suicide() for selfdestruct(), constant for view or var for actual type name should be avoided to prevent unintended errors with newer compiler versions. Solidity provides alternatives to the deprecated constructions. Most of them are aliases, thus replacing old constructions will not break current behavior. For example, sha3 can be replaced with keccak256. link
Function default visibility Redflag Functions without a visibility type specifier are public by default in solc < 0.5.0. This can lead to a vulnerability where a malicious user may make unauthorized state changes. solc >= 0.5.0 requires explicit function visibility specifiers. Functions can be specified as being external, public, internal or private. It is recommended to make a conscious decision on which visibility type is appropriate for a function. This can dramatically reduce the attack surface of a contract system. link
Incorrect inheritance order Redflag Contracts inheriting from multiple contracts with identical functions should specify the correct inheritance order i.e. more general to more specific to avoid inheriting the incorrect function implementation. When inheriting multiple contracts, especially if they have identical functions, a developer should carefully specify inheritance in the correct order. The rule of thumb is to inherit contracts from more /general/ to more /specific/. link
Missing inheritance missing-inheritance A contract might appear (based on name or functions implemented) to inherit from another interface or abstract contract without actually doing so. Inherit from the missing interface or contract. link
Insufficient gas griefing Redflag Transaction relayers must be trusted to supply adequate gas for successful transactions. Griefing attacks can occur when a contract accepts data and uses it in a sub-call on another contract. In relayer contracts, the user initiating the transaction (the 'forwarder') can censor transactions by providing just enough gas for the transaction but not the sub-call, leading to potential issues. There are two options to prevent insufficient gas griefing:

- Only allow trusted users to relay transactions.
- Require that the forwarder provides enough gas.
link
Modifying reference type parameters array-by-reference Structs/Arrays/Mappings passed as arguments to a function may be by value (memory) or reference (storage) as specified by the data location (optional before solc 0.5.0). Ensure correct usage of memory and storage in function parameters and make all data locations explicit. Ensure the correct usage of memory and storage in the function parameters. Make all the locations explicit. link
Hash collisions with multiple variable length arguments Redflag Using abi.encodePacked() with multiple variable length arguments can, in certain situations, lead to a hash collision. Do not allow users access to parameters used in abi.encodePacked(), use fixed length arrays or use abi.encode(). When using abi.encodePacked(), it's crucial to ensure that a matching signature cannot be achieved using different parameters. To do so, either do not allow users access to parameters used in abi.encodePacked(), or use fixed length arrays. Alternatively, you can simply use abi.encode() instead. link 1

link 2
Malleability risk from dirty high order bits Redflag Types that do not occupy the full 32 bytes might contain “dirty higher order bits” which does not affect operation on types but gives different results with msg.data. link
Incorrect shift in assembly incorrect-shift Shift operators (shl(x, y), shr(x, y), sar(x, y)) in Solidity assembly apply the shift operation of x bits on y and not the other way around, which may be confusing. Check if the values in a shift operation are reversed. Swap the order of parameters. link
Assembly usage Redflag Use of EVM assembly is error-prone and should be avoided or double-checked for correctness. Do not use evm assembly. link
Right-To-Left-Override control character (U+202E) Redflag Malicious actors can use the Right-To-Left-Override unicode character to force RTL text rendering and confuse users as to the real intent of a contract. U+202E character should not appear in the source code of a smart contract. There are very few legitimate uses of the U+202E character. It should not appear in the source code of a smart contract. link
Constant state variables constable-states Constant state variables should be declared constant to save gas. Add the constant attribute to state variables that never change. link
Similar variable names Redflag Variables with similar names could be confused for each other and therefore should be avoided. Prevent variables from having similar names. link
Uninitialized state/local variables uninitialized-state Uninitialized state/local variables are assigned zero values by the compiler and may cause unintended results e.g. transferring tokens to zero address. Explicitly initialize all state/local variables. Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero to improve code readability. link 1

link 2
Uninitialized storage pointers Redflag Uninitialized local storage variables can point to unexpected storage locations in the contract, which can lead to vulnerabilities. Solc 0.5.0 and above disallow such pointers. Check if the contract requires a storage object as in many situations this is actually not the case. If a local variable is sufficient, mark the storage location of the variable explicitly with the memory attribute. If a storage variable is needed then initialise it upon declaration and additionally specify the storage location storage. link
Uninitialized function pointers in constructors uninitialized-fptr-cst Calling uninitialized function pointers in constructors of contracts compiled with solc versions 0.4.5-0.4.25 and 0.5.0-0.5.7 lead to unexpected behavior because of a compiler bug. Initialize function pointers before calling. Avoid function pointers if possible. link
Long number literals Redflag Number literals with many digits should be carefully checked as they are prone to error. Use:

- Ether suffix,
- Time suffix, or
- The scientific notation
link
Out-of-range enum enum-conversion Solc < 0.4.5 produced unexpected behavior with out-of-range enums*.* Check enum conversion or use a newer compiler. Use a recent compiler version. If solc <0.4.5 is required, check the enum conversion range. link
Uncalled public functions external-function Public functions that are never called from within the contracts should be declared external to save gas. Use the external attribute for functions never called from the contract, and change the location of immutable parameters to calldata to save gas. link
Dead/Unreachable code Redflag Dead code may be indicative of programmer error, missing logic or potential optimization opportunity, which needs to be flagged for removal or addressed appropriately. link
Unused return values Redflag Unused return values of function calls are indicative of programmer errors which may have unexpected behavior. link
Unused variables Redflag Unused state/local variables may be indicative of programmer error, missing logic or potential optimization opportunity, which needs to be flagged for removal or addressed appropriately. link
Redundant statements Redflag Statements with no effects that do not produce code may be indicative of programmer error or missing logic, which needs to be flagged for removal or addressed appropriately. It's important to carefully ensure that your contract works as intended. Write unit tests to verify correct behaviour of the code. link
Storage array with signed Integers with ABIEncoderV2 Redflag Assigning an array of signed integers to a storage array of different type can lead to data corruption in that array. This is due to a compiler bug introduced in v0.4.7 and fixed in v0.5.10. link
Dynamic constructor arguments clipped with ABIEncoderV2: Redflag A contract's constructor which takes structs or arrays that contain dynamically sized arrays reverts or decodes to invalid data when ABIEncoderV2 is used. This is due to a compiler bug introduced in v0.4.16 and fixed in v0.5.9. link
Storage array with multiSlot element with ABIEncoderV2 Redflag Storage arrays containing structs or other statically sized arrays are not read properly when directly encoded in external function calls or in abi.encode(). This is due to a compiler bug introduced in v0.4.16 and fixed in v0.5.10. link
Calldata structs with statically sized and dynamically encoded members with ABIEncoderV2 Redflag Reading from calldata structs that contain dynamically encoded, but statically sized members can result in incorrect values. This is due to a compiler bug introduced in v0.5.6 and fixed in v0.5.11. link
Packed storage with ABIEncoderV2 Redflag Storage structs and arrays with types shorter than 32 bytes can cause data corruption if encoded directly from storage using ABIEncoderV2. This is due to a compiler bug introduced in v0.5.0 and fixed in v0.5.7. link
Incorrect loads with Yul optimizer and ABIEncoderV2 Redflag The Yul optimizer incorrectly replaces MLOAD and SLOAD calls with values that have been previously written to the load location. This can only happen if ABIEncoderV2 is activated and the experimental Yul optimizer has been activated manually in addition to the regular optimizer in the compiler settings. This is due to a compiler bug introduced in v0.5.14 and fixed in v0.5.15. link
Array slice dynamically encoded base type with ABIEncoderV2: Redflag Accessing array slices of arrays with dynamically encoded base types (e.g. multi-dimensional arrays) can result in invalid data being read. This is due to a compiler bug introduced in v0.6.0 and fixed in v0.6.8. link
Missing escaping in formatting with ABIEncoderV2 Redflag String literals containing double backslash characters passed directly to external or encoding function calls can lead to a different string being used when ABIEncoderV2 is enabled. This is due to a compiler bug introduced in v0.5.14 and fixed in v0.6.8. link
Double shift size overflow Redflag Double bitwise shifts by large constants whose sum overflows 256 bits can result in unexpected values. Nested logical shift operations whose total shift size is 2**256 or more are incorrectly optimized. This only applies to shifts by numbers of bits that are compile-time constant expressions. This happens when the optimizer is used and evmVersion >= Constantinople. This is due to a compiler bug introduced in v0.5.5 and fixed in v0.5.6. link
Incorrect byte instruction optimization Redflag The optimizer incorrectly handles byte opcodes whose second argument is 31 or a constant expression that evaluates to 31. This can result in unexpected values. This can happen when performing index access on bytesNN types with a compile time constant value (not index) of 31 or when using the byte opcode in inline assembly. This is due to a compiler bug introduced in v0.5.5 and fixed in v0.5.7. link
Essential assignments removed with Yul Optimizer Redflag The Yul optimizer can remove essential assignments to variables declared inside for loops when Yul's continue or break statement is used mostly while using inline assembly with for loops and continue and break statements. This is due to a compiler bug introduced in v0.5.8/v0.6.0 and fixed in v0.5.16/v0.6.1. link
Private methods overridden Redflag While private methods of base contracts are not visible and cannot be called directly from the derived contract, it is still possible to declare a function of the same name and type and thus change the behaviour of the base contract's function. This is due to a compiler bug introduced in v0.3.0 and fixed in v0.5.17. link
Tuple assignment multi stack slot components Redflag Tuple assignments with components that occupy several stack slots, i.e. nested tuples, pointers to external functions or references to dynamically sized calldata arrays, can result in invalid values. This is due to a compiler bug introduced in v0.1.6 and fixed in v0.6.6. link
Dynamic array cleanup Redflag When assigning a dynamically sized array with types of size at most 16 bytes in storage causing the assigned array to shrink, some parts of deleted slots were not zeroed out. This is due to a compiler bug fixed in v0.7.3. link
Empty byte array copy Redflag Copying an empty byte array (or string) from memory or calldata to storage can result in data corruption if the target array's length is increased subsequently without storing new data. This is due to a compiler bug fixed in v0.7.4. link
Memory array creation overflow Redflag The creation of very large memory arrays can result in overlapping memory regions and thus memory corruption. This is due to a compiler bug introduced in v0.2.0 and fixed in v0.6.5. link
Calldata using for Redflag Function calls to internal library functions with calldata parameters called via “using for” can result in invalid data being read. This is due to a compiler bug introduced in v0.6.9 and fixed in v0.6.10. link
Free function redefinition Redflag The compiler does not flag an error when two or more free functions (functions outside of a contract) with the same name and parameter types are defined in a source unit or when an imported free function alias shadows another free function with a different name but identical parameter types. This is due to a compiler bug introduced in v0.7.1 and fixed in v0.7.2. link
Unprotected initializers in proxy-based upgradeable contracts missing-init-modifier Proxy-based upgradeable contracts need to use public initializer functions instead of constructors that need to be explicitly called only once. Preventing multiple invocations of such initializer functions (e.g. via initializer modifier from OpenZeppelin’s Initializable library) is a must. link 1

link 1
Initializing state-variables in proxy-based upgradeable contracts Redflag This should be done in initializer functions and not as part of the state variable declarations in which case they won’t be set. link
Import upgradeable contracts in proxy-based upgradeable contracts Redflag Contracts imported from proxy-based upgradeable contracts should also be upgradeable where such contracts have been modified to use initializers instead of constructors. link
Avoid selfdestruct or delegatecall in proxy-based upgradeable contracts Redflag This will cause the logic contract to be destroyed and all contract instances will end up delegating calls to an address without any code. link
State variables in proxy-based upgradeable contracts Redflag The declaration order/layout and type/mutability of state variables in such contracts should be preserved exactly while upgrading to prevent critical storage layout mismatch errors. link
Function ID collision between proxy/implementation in proxy-based upgradeable contracts Redflag Malicious proxy contracts may exploit function ID collision to invoke unintended proxy functions instead of delegating to implementation functions. Check for function ID collisions. Rename the function. Avoid public functions in the proxy. link 1

link 2
Function shadowing between proxy/contract in proxy-based upgradeable contracts Redflag Shadow functions in proxy contract prevent functions in logic contract from being invoked. link
Arbitrary from in transferFrom arbitrary-send-erc20 High Detect when msg.sender is not used as from in transferFrom. Use msg.sender as from in transferFrom. link
Modifying storage array by value Detect arrays passed to a function that expects reference to a storage array Use msg.sender as from in transferFrom. link
ERC20 transfer and transferFrom Redflag Should return a boolean. Several tokens do not return a boolean on these functions. As a result, their calls in the contract might fail. link
ERC20 name, decimals, and symbol functions Redflag Are present if used. These functions are optional in the ERC20 standard and might not be present. link
ERC20 decimals returns a uint8 Redflag Several tokens incorrectly return a uint256. If this is the case, ensure the value returned is below 255. link
ERC20 approve race-condition Redflag The ERC20 standard has a known ERC20 race condition that must be mitigated to prevent attackers from stealing tokens. link
ERC777 hooks Redflag ERC777 tokens have the concept of a hook function that is called before any calls to send, transfer, operatorSend, minting and burning. While these hooks enable a lot of interesting use cases, care should be taken to make sure they do not make external calls because that can lead to reentrancies. link
Token Deflation via fees Redflag Transfer and transferFrom should not take a fee. Deflationary tokens can lead to unexpected behavior. link
Token Inflation via interest Redflag Potential interest earned from the token should be taken into account. Some tokens distribute interest to token holders. This interest might be trapped in the contract if not taken into account. link
Token contract avoids unneeded complexity Redflag The token should be a simple contract; a token with complex code requires a higher standard of review. link
Token contract has only a few non–token-related functions Redflag Non–token-related functions increase the likelihood of an issue in the contract. link
Token contract has only a few non–token-related functions Redflag Non–token-related functions increase the likelihood of an issue in the contract. link
Token only has one address Redflag Tokens with multiple entry points for balance updates can break internal bookkeeping based on the address (e.g. balances[token_address][msg.sender] might not reflect the actual balance). link
Token is not upgradeable Redflag Upgradeable contracts might change their rules over time. link
Token owner has limited minting capabilities Redflag Malicious or compromised owners can abuse minting capabilities. link
Token is not pausable Redflag Malicious or compromised owners can trap contracts relying on pausable tokens link
Token owner cannot blacklist the contract Redflag Malicious or compromised owners can trap contracts relying on tokens with a blacklist. link
Token development team is known and can be held responsible for abuse Redflag Contracts with anonymous development teams, or that reside in legal shelters should require a higher standard of review. link
No token user owns most of the supply Redflag If a few users own most of the tokens, they can influence operations based on the token's repartition. link
Token total supply is sufficient Redflag Tokens with a low total supply can be easily manipulated. link
Tokens are located in more than a few exchanges Redflag If all the tokens are in one exchange, a compromise of the exchange can compromise the contract relying on the token. link
Token balance and Flash loans Redflag Users understand the associated risks of large funds or flash loans. Contracts relying on the token balance must carefully take in consideration attackers with large funds or attacks through flash loans. link
Token does not allow flash minting Redflag Flash minting can lead to substantial swings in the balance and the total supply, which necessitate strict and comprehensive overflow checks in the operation of the token. link
ERC1400 permissioned addresses Redflag Can block transfers from/to specific addresses. link
ERC1400 forced transfers Redflag Trusted actors have the ability to transfer funds however they choose. link
ERC1644 forced transfers Redflag Controller has the ability to steal funds. link
ERC621 control of totalSupply Redflag totalSupply can be changed by trusted actors link
ERC884 cancel and reissue Redflag Token implementers have the ability to cancel an address and move its tokens to a new address link
ERC884 whitelisting Redflag Tokens can only be sent to whitelisted addresses link
Guarded launch via asset limits Redflag Limiting the total asset value managed by a system initially upon launch and gradually increasing it over time may reduce impact due to initial vulnerabilities or exploits. link
Guarded launch via asset types Redflag Limiting types of assets that can be used in the protocol initially upon launch and gradually expanding to other assets over time may reduce impact due to initial vulnerabilities or exploits. link
Guarded launch via user limits Redflag Limiting the total number of users that can interact with a system initially upon launch and gradually increasing it over time may reduce impact due to initial vulnerabilities or exploits. Initial users may also be whitelisted to limit to trusted actors before opening the system to everyone. link
Guarded launch via composability limits Redflag Restricting the composability of the system to interface only with whitelisted trusted contracts before expanding to arbitrary external contracts may reduce impact due to initial vulnerabilities or exploits. link
Guarded launch via escrows Redflag Escrowing high value transactions/operations with time locks and a governance capability to nullify or revert transactions may reduce impact due to initial vulnerabilities or exploits. link
Guarded launch via circuit breakers Redflag Implementing capabilities to pause/unpause a system in extreme scenarios may reduce impact due to initial vulnerabilities or exploits. link
Guarded launch via emergency shutdown Redflag Implement capabilities that allow governance to shutdown new activity in the system and allow users to reclaim assets may reduce impact due to initial vulnerabilities or exploits. link
System specification Redflag Ensure that the specification of the entire system is considered, written and evaluated to the greatest detail possible. Specification describes how (and why) the different components of the system behave to achieve the design requirements. Without specification, a system implementation cannot be evaluated against the requirements for correctness. link
System documentation Redflag Ensure that roles, functionalities and interactions of the entire system are well documented to the greatest detail possible. Documentation describes what (and how) the implementation of different components of the system does to achieve the specification goals. Without documentation, a system implementation cannot be evaluated against the specification for correctness and one will have to rely on analyzing the implementation itself. link
Function parameters Redflag Ensure input validation for all function parameters especially if the visibility is external/public where (untrusted) users can control values. This is especially required for address parameters where maliciously/accidentally used incorrect/zero addresses can cause vulnerabilities or unexpected behavior. link
Function arguments Redflag Ensure that the arguments to function calls at the caller sites are the correct ones and in the right order as expected by the function definition. link
Function visibility Redflag Ensure that the strictest visibility is used for the required functionality. An accidental external/public visibility will allow (untrusted) users to invoke functionality that is supposed to be restricted internally. link
Function modifiers Redflag Ensure that the right set of function modifiers are used (in the correct order) for the specific functions so that the expected access control or validation is correctly enforced. link
Function return values Redflag Ensure that the appropriate return value(s) are returned from functions and checked without ignoring at function call sites, so that error conditions are caught and handled appropriately. link
Function invocation timeliness Redflag Externally accessible functions (external/public visibility) may be called at any time (or never). It is not safe to assume they will only be called at specific system phases (e.g. after initialization, when unpaused, during liquidation) that is meaningful to the system design. The reason for this can be accidental or malicious. Function implementation should be robust enough to track system state transitions, determine meaningful states for invocations and withstand arbitrary calls. For e.g., initialization functions (used with upgradeable contracts that cannot use constructors) are meant to be called atomically along with contract deployment to prevent anyone else from initializing with arbitrary values. link
Function invocation repetitiveness Redflag Externally accessible functions (external/public visibility) may be called any number of times. It is not safe to assume they will only be called only once or a specific number of times that is meaningful to the system design. Function implementation should be robust enough to track, prevent, ignore or account for arbitrarily repetitive invocations. For e.g., initialization functions (used with upgradeable contracts that cannot use constructors) are meant to be called only once. link
Function invocation order Redflag Externally accessible functions (external/public visibility) may be called in any order (with respect to other defined functions). It is not safe to assume they will only be called in the specific order that makes sense to the system design or is implicitly assumed in the code. For e.g., initialization functions (used with upgradeable contracts that cannot use constructors) are meant to be called before other system functions can be called. link
Function invocation arguments Redflag Externally accessible functions (external/public visibility) may be called with any possible arguments. Without complete and proper validation (e.g. zero address checks, bound checks, threshold checks etc.), they cannot be assumed to comply with any assumptions made about them in the code. link
Conditionals Redflag Ensure that in conditional expressions (e.g. if statements), the correct variables are being checked and the correct operators, if any, are being used to combine them. For e.g. using | instead of && is a common error. link
Access control specification Redflag Ensure that the various system actors, their access control privileges and trust assumptions are accurately specified in great detail so that they are correctly implemented and enforced across different contracts, functions and system transitions/flows. link
Access control implementation Redflag Ensure that the specified access control is implemented uniformly across all the subjects (actors) seeking access and objects (variables, functions) being accessed so that there are no paths/flows where the access control is missing or may be side-stepped. link
Missing modifiers Redflag Access control is typically enforced on functions using modifiers that check if specific addresses/roles are calling these functions. Ensure that such modifiers are present on all relevant functions which require that specific access control. link
Incorrectly implemented modifiers Redflag Access control is typically enforced on functions using modifiers that check if specific addresses/roles are calling these functions. A system can have multiple roles with different privileges. Ensure that modifiers are implementing the expected checks on the correct roles/addresses with the right composition e.g. incorrect use of | instead of && is a common vulnerability while composing access checks. link
Incorrectly used modifiers Redflag Access control is typically enforced on functions using modifiers that check if specific addresses/roles are calling these functions. A system can have multiple roles with different privileges. Ensure that correct modifiers are used on functions requiring specific access control enforced by that modifier. link
Access control changes Redflag Ensure that changes to access control (e.g. change of ownership to new addresses) are handled with extra security so that such transitions happen smoothly without contracts getting locked out or compromised due to use of incorrect credentials. link
Comments Redflag Ensure that the code is well commented both with NatSpec and inline comments for better readability and maintainability. The comments should accurately reflect what the corresponding code does. Stale comments should be removed. Discrepancies between code and comments should be addressed. Any TODO’s indicated by comments should be addressed. Commented code should be removed. link
Tests Redflag Tests indicate that the system implementation has been validated against the specification. Unit tests, functional tests and integration tests should have been performed to achieve good test coverage across the entire codebase. Any code or parameterisation used specifically for testing should be removed from production code. link
Unused constructs Redflag Any unused imports, inherited contracts, functions, parameters, variables, modifiers, events or return values should be removed (or used appropriately) after careful evaluation. This will not only reduce gas costs but improve readability and maintainability of the code. link
Redundant constructs Redflag Redundant code and comments can be confusing and should be removed (or changed appropriately) after careful evaluation. This will not only reduce gas costs but improve readability and maintainability of the code. link
ETH Handling Redflag Contracts that accept/manage/transfer ETH should ensure that functions handling ETH are using msg.value appropriately, logic that depends on ETH value accounts for less/more ETH sent, logic that depends on contract ETH balance accounts for the different direct/indirect (e.g. coinbase transaction, selfdestruct recipient) ways of receiving ETH and transfers are reentrancy safe. Functions handling ETH should be checked extra carefully for access control, input validation and error handling. link
Token Handling Redflag Contracts that accept/manage/transfer ERC tokens should ensure that functions handling tokens account for different types of ERC tokens (e.g. ERC20 vs ERC777), deflationary/inflationary tokens, rebasing tokens and trusted/external tokens. Functions handling tokens should be checked extra carefully for access control, input validation and error handling. link
Trusted actors Redflag Ideally there should be no trusted actors while interacting with smart contracts. However, in guarded launch scenarios, the goal is to start with trusted actors and then progressively decentralise towards automated governance by community/DAO. For the trusted phase, all the trusted actors, their roles and capabilities should be clearly specified, implemented accordingly and documented for user information and examination. link
Privileged roles and EOAs Redflag Trusted actors who have privileged roles with capabilities to deploy contracts, change critical parameters, pause/unpause system, trigger emergency shutdown, withdraw/transfer/drain funds and allow/deny other actors should be addresses controlled by multiple, independent, mutually distrusting entities. They should not be controlled by private keys of EOAs but with Multisigs with a high threshold (e.g. 5-of-7, 9-of-11) and eventually by a DAO of token holders. EOA has a single point of failure. link
Two-step change of privileged roles Redflag When privileged roles are being changed, it is recommended to follow a two-step approach: 1) The current privileged role proposes a new address for the change 2) The newly proposed address then claims the privileged role in a separate transaction. This two-step change allows accidental proposals to be corrected instead of leaving the system operationally with no/malicious privileged role. For e.g., in a single-step change, if the current admin accidentally changes the new admin to a zero-address or an incorrect address (where the private keys are not available), the system is left without an operational admin and will have to be redeployed link
Time-delayed change of critical parameters Redflag When critical parameters of systems need to be changed, it is required to broadcast the change via event emission and recommended to enforce the changes after a time-delay. This is to allow system users to be aware of such critical changes and give them an opportunity to exit or adjust their engagement with the system accordingly. For e.g. reducing the rewards or increasing the fees in a system might not be acceptable to some users who may wish to withdraw their funds and exit. link
Explicit over Implicit Redflag While Solidity has progressively adopted explicit declarations of intent for e.g. with function visibility and variable storage, it is recommended to do the same at the application level where all requirements should be explicitly validated (e.g. input parameters) and assumptions should be documented and checked. Implicit requirements and assumptions should be flagged as dangerous. link
Configuration issues Redflag Misconfiguration of system components such contracts, parameters, addresses and permissions may lead to security issues. link
Initialization issues Redflag Lack of initialization, initializing with incorrect values or allowing untrusted actors to initialize system parameters may lead to security issues. link
Cleanup issues Redflag Missing to clean up old state or cleaning up incorrectly/insufficiently will lead to reuse of stale state which may lead to security issues. link
Data processing issues Redflag Processing data incorrectly will cause unexpected behavior which may lead to security issues. link
Data validation issues Redflag Missing validation of data or incorrectly/insufficiently validating data, especially tainted data from untrusted users, will cause untrustworthy system behavior which may lead to security issues. link
Numerical issues Redflag Incorrect numerical computation will cause unexpected behavior which may lead to security issues. link
Accounting issues Redflag Incorrect or insufficient tracking or accounting of business logic related aspects such as states, phases, permissions, roles, funds (deposits/withdrawals) and tokens (mints/burns/transfers) may lead to security issues. link
Access control issues Redflag Incorrect or insufficient access control or authorization related to system actors, roles, assets and permissions may lead to security issues. link
Auditing/logging issues Redflag Incorrect or insufficient emission of events will impact off-chain monitoring and incident response capabilities which may lead to security issues. link
Cryptography issues Redflag Incorrect or insufficient cryptography especially related to on-chain signature verification or off-chain key management will impact access control and may lead to security issues. link
Error-reporting issues Redflag Incorrect or insufficient detecting, reporting and handling of error conditions will cause exceptional behavior to go unnoticed which may lead to security issues. link
Denial-of-Service (DoS) issues Redflag Preventing other users from successfully accessing system services by modifying system parameters or state causes denial-of-service issues which affects the availability of the system. This may also manifest as security issues if users are not able to access their funds locked in the system. link
Timing issues Redflag Incorrect assumptions on timing of user actions, system state transitions or blockchain state/blocks/transactions may lead to security issues. link
Ordering issues Redflag Incorrect assumptions on ordering of user actions or system state transitions may lead to security issues. For e.g., a user may accidentally/maliciously call a finalization function even before the initialization function if the system allows. link
Undefined behavior issues Redflag Any behavior that is undefined in the specification but is allowed in the implementation will result in unexpected outcomes which may lead to security issues. link
External interaction issues Redflag Interacting with external components (e.g. tokens, contracts, oracles) forces system to trust or make assumptions on their correctness/availability requiring validation of their existence and outputs without which may lead to security issues. link
Trust issues Redflag Incorrect or Insufficient trust assumption about/among system actors and external entities will lead to privilege escalation/abuse which may lead to security issues. link
Gas issues Redflag Incorrect assumptions about gas requirements especially for loops or external calls will lead to out-of-gas exceptions which may lead to security issues such as failed transfers or locked funds. link
Dependency issues Redflag Dependencies on external actors or software (imports, contracts, libraries, tokens etc.) will lead to trust/availability/correctness assumptions which if/when broken may lead to security issues. link
Constant issues Redflag Incorrect assumptions about system actors, entities or parameters being constant may lead to security issues if/when such factors change unexpectedly. link
Freshness issues Redflag Incorrect assumptions about the status of or data from system actors or entities being fresh (i.e. not stale), because of lack of updation or availability, may lead to security issues if/when such factors have been updated. For e.g., getting a stale price from an Oracle. link
Scarcity issues Redflag Incorrect assumptions about the scarcity of tokens/funds available to any system actor will lead to unexpected outcomes which may lead to security issues. For e.g., flash loans. link
Incentive issues Redflag Incorrect assumptions about the incentives of system/external actors to perform or not perform certain actions will lead to unexpected behavior being triggered or expected behavior not being triggered, both of which may lead to security issues. For e.g., incentive to liquidate positions, lack of incentive to DoS or grief system. link
Clarity issues Redflag Lack of clarity in system specification, documentation, implementation or UI/UX will lead to incorrect expectations/outcome which may lead to security issues. link
Privacy issues Redflag Data and transactions on the Ethereum blockchain are not private. Anyone can observe contract state and track transactions (both included in a block and pending in the mempool). Incorrect assumptions about privacy aspects of data or transactions can be abused which may lead to security issues. link
Cloning issues Redflag Copy-pasting code from other libraries, contracts or even different parts of the same contract may result in incorrect code semantics for the context being copied to, copy over any vulnerabilities or miss any security fixes applied to the original code. All these may lead to security issues. link
Business logic issues Redflag Incorrect or insufficient assumptions about the higher-order business logic being implemented in the application will lead to differences in expected and actual behavior, which may result in security issues. link
Principle of Least Privilege Redflag “Every program and every user of the system should operate using the least set of privileges necessary to complete the job” — Ensure that various system actors have the least amount of privilege granted as required by their roles to execute their specified tasks. Granting excess privilege is prone to misuse/abuse when trusted actors misbehave or their access is hijacked by malicious entities. See Saltzer and Schroeder's Secure Design Principles
Principle of Separation of Privilege Redflag “Where feasible, a protection mechanism that requires two keys to unlock it is more robust and flexible than one that allows access to the presenter of only a single key” — Ensure that critical privileges are separated across multiple actors so that there are no single points of failure/abuse. A good example of this is to require a multisig address (not EOA) for privileged actors (e.g. owner, admin, governor, deployer) who control key contract functionality such as pause/unpause/shutdown, emergency fund drain, upgradeability, allow/deny list and critical parameters. The multisig address should be composed of entities that are different and mutually distrusting/verifying. See Saltzer and Schroeder's Secure Design Principles
Principle of Least Common Mechanism Redflag “Minimize the amount of mechanism common to more than one user and depended on by all users” — Ensure that only the least number of security-critical modules/paths as required are shared amongst the different actors/code so that impact from any vulnerability/compromise in shared components is limited and contained to the smallest possible subset. See Saltzer and Schroeder's Secure Design Principles
Principle of Fail-safe Defaults Redflag “Base access decisions on permission rather than exclusion” — Ensure that variables or permissions are initialized to fail-safe default values which can be made more inclusive later instead of opening up the system to everyone including untrusted actors. See Saltzer and Schroeder's Secure Design Principles
Principle of Complete Mediation Redflag “Every access to every object must be checked for authority.” — Ensure that any required access control is enforced along all access paths to the object or function being protected. See Saltzer and Schroeder's Secure Design PrinciplesSee Saltzer and Schroeder's Secure Design Principles
Principle of Economy of Mechanism Redflag “Keep the design as simple and small as possible” — Ensure that contracts and functions are not overly complex or large so as to reduce readability or maintainability. Complexity typically leads to insecurity. See Saltzer and Schroeder's Secure Design Principles
Principle of Open Design Redflag “The design should not be secret” — Smart contracts are expected to be open-sourced and accessible to everyone. Security by obscurity of code or underlying algorithms is not an option. Security should be derived from the strength of the design and implementation under the assumption that (byzantine) attackers will study their details and try to exploit them in arbitrary ways. See Saltzer and Schroeder's Secure Design Principles
Principle of Psychological Acceptability Redflag “It is essential that the human interface be designed for ease of use, so that users routinely and automatically apply the protection mechanisms correctly” — Ensure that security aspects of smart contract interfaces and system designs/flows are user-friendly and intuitive so that users can interact with minimal risk. See Saltzer and Schroeder's Secure Design Principles
Principle of Work Factor Redflag “Compare the cost of circumventing the mechanism with the resources of a potential attacker” — Given the magnitude of value managed by smart contracts, it is safe to assume that byzantine attackers will risk the greatest amounts of intellectual/financial/social capital possible to subvert such systems. Therefore, the mitigation mechanisms must factor in the highest levels of risk. See Saltzer and Schroeder's Secure Design Principles
Principle of Compromise Recording Redflag “Mechanisms that reliably record that a compromise of information has occurred can be used in place of more elaborate mechanisms that completely prevent loss” — Ensure that smart contracts and their accompanying operational infrastructure can be monitored/analyzed at all times (development/deployment/runtime) for minimizing loss from any compromise due to vulnerabilities/exploits. For e.g., critical operations in contracts should necessarily emit events to facilitate monitoring at runtime. See Saltzer and Schroeder's Secure Design Principles
Protected Variables protected-vars Detect unprotected variable that are marked protected e.g. owner must be always written by function using onlyOwner Add access controls to the vulnerable function link
Public mappings with nested variables Redflag Prior to Solidity 0.5, a public mapping with nested structures returned incorrect values. Do not use public mapping with nested structures. link
## Unprotected upgradeable contract Redflag Detects logic contract that can be destructed. Add a constructor to ensure initialize cannot be called on the logic contract. link
## Arbitrary from in transferFrom used with permit Redflag Detect when msg.sender is not used as from in transferFrom and permit is used. Ensure that the underlying ERC20 token correctly implements a permit function. link
## Functions that send Ether to arbitrary destinations Redflag Unprotected call to a function sending Ether to an arbitrary address. Ensure that an arbitrary user cannot withdraw unauthorized funds. link
## Array Length Assignment Redflag Detects the direct assignment of an array's length. Do not allow array lengths to be set directly set; instead, opt to add values as needed. Otherwise, thoroughly review the contract to ensure a user-controlled variable cannot reach an array length assignment. link
## Payable functions using delegatecall inside a loop Redflag Detect the use of delegatecall inside a loop in a payable function. Carefully check that the function called by delegatecall is not payable/doesn't use msg.value. link
## msg.value inside a loop Redflag Detect the use of msg.value inside a loop. Provide an explicit array of amounts alongside the receivers array, and check that the sum of all amounts matches msg.value. link
## Storage Signed Integer Array Redflag solc versions 0.4.7-0.5.9 contain a compiler bug leading to incorrect values in signed integer arrays. Use a compiler version >= 0.5.10. link
## Unchecked transfer Redflag The return value of an external transfer/transferFrom call is not checked Use SafeERC20, or ensure that the transfer/transferFrom return value is checked link
## Codex Redflag Use codex to find vulnerabilities Review codex's message. link
## Domain separator collision Redflag An ERC20 token has a function whose signature collides with EIP-2612's DOMAIN_SEPARATOR(), causing unanticipated behavior for contracts using permit functionality. some_collision clashes with EIP-2612's DOMAIN_SEPARATOR() and will interfere with contract's using permit. link
## Incorrect erc20 interface Redflag Incorrect return values for ERC20 functions. A contract compiled with Solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing. Set the appropriate return values and types for the defined ERC20 functions link
## Incorrect erc721 interface Redflag Incorrect return values for ERC721 functions. A contract compiled with solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing. Set the appropriate return values and vtypes for the defined ERC721 functions. link
## Write after write Redflag Detects variables that are written but never read and written again. Fix or remove the writes. link
## Constant functions using assembly code Redflag Functions declared as constant/pure/view using assembly code.

constant/pure/view was not enforced prior to Solidity 0.5. Starting from Solidity 0.5, a call to a constant/pure/view function uses the STATICCALL opcode, which reverts in case of state modification.

As a result, a call to an incorrectly labeled function may trap a contract compiled with Solidity 0.5.
Ensure the attributes of contracts compiled prior to Solidity 0.5.0 are correct. link
## Constant functions changing the state Redflag Functions declared as constant/pure/view change the state.

constant/pure/view was not enforced prior to Solidity 0.5. Starting from Solidity 0.5, a call to a constant/pure/view function uses the STATICCALL opcode, which reverts in case of state modification.

As a result, a call to an incorrectly labeled function may trap a contract compiled with Solidity 0.5.
Ensure that attributes of contracts compiled prior to Solidity 0.5.0 are correct. link
## Reused base constructors Redflag Detects if the same base constructor is called with arguments from two different locations in the same inheritance hierarchy. Remove the duplicate constructor call. link
## Unchecked low-level calls Redflag The return value of a low-level call is not checked. Ensure that the return value of a low-level call is checked or logged. link
## Unchecked Send Redflag The return value of a send is not checked. Ensure that the return value of send is checked or logged. link
## Builtin Symbol Shadowing Redflag Detection of shadowing built-in symbols using local variables, state variables, functions, modifiers, or events. Rename the local variables, state variables, functions, modifiers, and events that shadow a builtin symbol. link
## Missing events arithmetic Redflag Detect missing events for critical arithmetic parameters Emit an event for critical parameter changes. link
## Dangerous unary expressions Redflag Unary expressions such as x=+1 probably typos. Remove the unary expression. link
## Reentrancy vulnerabilities Redflag Detection of the reentrancy bug. Only report reentrancy that acts as a double call (see reentrancy-eth, reentrancy-no-eth). Apply the check-effects-interactions pattern. link
## Reentrancy vulnerabilities Redflag Detects reentrancies that allow manipulation of the order or value of events. Apply the check-effects-interactions pattern. link
## Reentrancy vulnerabilities Redflag Detection of the reentrancy bug. Only report reentrancy that is based on transfer or send. Apply the check-effects-interactions pattern. link
## Cyclomatic complexity Redflag Detects functions with high (> 11) cyclomatic complexity. Reduce cyclomatic complexity by splitting the function into several smaller subroutines. link
## Deprecated standards Redflag Detect the usage of deprecated standards. Replace all uses of deprecated symbols. link
## Function Initializing State Redflag Detects the immediate initialization of state variables through function calls that are not pure/constant, or that use non-constant state variable. Remove any initialization of state variables via non-constant state variables or function calls. If variables must be set upon contract deployment, locate initialization in the constructor instead. link
## Incorrect usage of using-for statement Redflag In Solidity, it is possible to use libraries for certain types, by the using-for statement (using <library> for <type>). However, the Solidity compiler doesn't check whether a given library has at least one function matching a given type. If it doesn't, such a statement has no effect and may be confusing. Make sure that the libraries used in using-for statements have at least one function matching a type used in these statements. link
## Low-level calls Redflag The use of low-level calls is error-prone. Low-level calls do not check for code existence or call success. Avoid low-level calls. Check the call success. If the call is meant for a contract, check for code existence. link
## Conformance to Solidity naming conventions Redflag Solidity defines a naming convention that should be followed.

#### Rule exceptions

- Allow constant variable name/symbol/decimals to be lowercase (ERC20).
- Allow _ at the beginning of the mixed_case match for private variables and unused parameters
Follow the Solidity naming convention. link
## Different pragma directives are used Redflag Detect whether different Solidity versions are used. Use one Solidity version. link
## Redundant Statements Redflag Detect the usage of redundant statements that have no effect. Remove redundant statements if they congest code but offer no value. link
## Incorrect versions of Solidity Redflag solc frequently releases new compiler versions. Using an old version prevents access to new Solidity security checks. We also recommend avoiding complex pragma statement. Deploy with any of the following Solidity versions:

- 0.8.18

The recommendations take into account:

- Risks related to recent releases
- Risks of complex code generation changes
- Risks of new language features
- Risks of known bugs

Use a simple pragma version that allows any of these versions. Consider using the latest version of Solidity for testing.
link
## Unimplemented functions

###
Redflag Detect functions that are not implemented on derived-most contracts.
All unimplemented functions must be implemented on a contract that is meant to be used.
Implement all unimplemented functions in any contract you intend to use directly (not simply inherit from). link
## Reentrancy vulnerabilities Redflag Detection of the reentrancy bug. Only report reentrancy that is based on transfer or send. send and transfer do not protect from reentrancies in case of gas price changes. Apply the check-effects-interactions pattern. link
## Cache array length Redflag Detects for loops that use length member of some storage array in their loop condition and don't modify it. Cache the lengths of storage arrays if they are used and not modified in for loops. link
## State variables that could be declared constant Redflag State variables that are not updated following deployment should be declared constant to save gas. Add the constant attribute to state variables that never change. link
## Public function that could be declared external Redflag public functions that are never called by the contract should be declared external, and its immutable parameters should be located in calldata to save gas. Use the external attribute for functions never called from the contract, and change the location of immutable parameters to calldata to save gas. link
## State variables that could be declared immutable Redflag State variables that are not updated following deployment should be declared immutable to save gas. Add the immutable attribute to state variables that never change or are set only in the constructor. link
## Public variable read in external context Redflag The contract reads its own variable using this, adding overhead of an unnecessary STATICCALL. Read the variable directly from storage instead of calling the contract. link
## Missing Protection against Signature Replay Attacks Redflag Sometimes in smart contracts it is necessary to perform signature verification to improve usability and gas cost. However, consideration needs to be taken when implementing signature verification. To protect against Signature Replay Attacks, the contract should only be allowing new hashes to be processed. This prevents malicious users from replaying another users signature multiple times. To be extra safe with signature verification, follow these recommendations:

- Store every message hash processed by the contract, then check messages hashes against the existing ones before executing the function.
- Include the address of the contract in the hash to ensure that the message is only used in a single contract.
- Never generate the message hash including the signature. See Signature Malleability
- https://swcregistry.io/docs/SWC-121
- https://medium.com/cypher-core/replay-attack-vulnerability-in-ethereum-smart-contracts-introduced-by-transferproxy-124bf3694e25
## Assert Violation Redflag In Solidity 0.4.10, the following functions were created: assert(), require(), and revert(). Here we'll discuss the assert function and how to use it.

Formally said, the assert() funtion is meant to assert invariants; informally said, assert() is an overly assertive bodyguard that protects your contract, but steals your gas in the process. Properly functioning contracts should never reach a failing assert statement. If you've reached a failing assert statement, you've either improperly used assert(), or there is a bug in your contract that puts it in an invalid state.
If the condition checked in the assert() is not actually an invariant, it's suggested that you replace it with a require() statement. - https://swcregistry.io/docs/SWC-110
- https://media.consensys.net/when-to-use-revert-assert-and-require-in-solidity-61fb2c0e5a57
## Requirement Violation Redflag The require() method is meant to validate conditions, such as inputs or contract state variables, or to validate return values from external contract calls. For validating external calls, inputs can be provided by callers, or they can be returned by callees. In the case that an input violation has occured by the return value of a callee, likely one of two things has gone wrong:

- There is a bug in the contract that provided the input.
- The requirement condition is too strong.
To solve this issue, first consider whether the requirement condition is too strong. If necessary, weaken it to allow any valid external input. If the problem isn't the requirement condition, there must be a bug in the contract providing external input. Ensure that this contract is not providing invalid inputs. - https://swcregistry.io/docs/SWC-123
- https://media.consensys.net/when-to-use-revert-assert-and-require-in-solidity-61fb2c0e5a57
Inadherence to Standards Redflag In terms of smart contract development, it's important to follow standards. Standards are set to prevent vulnerabilities, and ignoring them can lead to unexpected effects.

Take for example binance's original BNB token. It was marketed as an ERC20 token, but it was later pointed out that it wasn't actually ERC20 compliant for a few reasons:

- It prevented sending to 0x0
- It blocked transfers of 0 value
- It didn't return true or false for success or fail

The main cause for concern with this improper implementation is that if it is used with a smart contract that expects an ERC-20 token, it will behave in unexpected ways. It could even get locked in the contract forever.
Although standards aren't always perfect, and may someday become antiquated, they foster proper expectations to provide for secure smart contracts. - https://finance.yahoo.com/news/bnb-really-erc-20-token-160013314.html
- https://blog.goodaudience.com/binance-isnt-erc-20-7645909069a4
## DoS with Block Gas Limit Redflag The block gas limit serves as a safeguard against infinite transaction loops and potential attacks in Ethereum. While it prevents excessive gas usage, it can also pose challenges. Unbounded operations, like executing logic in an endless loop or handling large arrays of users for fund transfers, can exceed the gas limit, potentially locking up funds. Moreover, malicious actors can exploit the block gas limit to block transactions by filling blocks with high-priced transactions. To address these issues, a pull payment system, separate transactions, and gas-efficient loops should be considered. It's also crucial to assess the safety of time-based actions in applications to avoid block stuffing attacks, as seen in the case of Fomo3D. link
## DoS with (Unexpected) revert Redflag Denial of Service (DoS) vulnerabilities can arise in Solidity smart contracts when unexpected revert conditions prevent essential logic from executing. This can occur due to various factors, such as reverting fund transfers, which can be exploited by attackers creating contracts with a reversion fallback, rendering refund attempts futile. Additionally, without malicious intent, attempting to pay an array of users in a single transaction can lead to a full function revert if just one payment fails, causing all others to be unpaid. A recommended solution is transitioning from a push payment system to a pull payment system, where each payment is handled in a separate transaction initiated by the recipient. Overflows and underflows, especially with smaller integer types, can also result in reverts, emphasizing the importance of input validation. Finally, unexpected balance manipulations, whether through ERC20 token transfers or forced Ether deposits, should be cautiously monitored to avoid unexpected reverts, potentially disrupting contract functionality. link
## Unexpected ecrecover Null Address Redflag The Solidity precompiled function ecrecover is used to recover an address from an elliptic curve signature, returning zero on error, where the signature parameters include r, s, and v. However, a security concern arises when uninitialized or abandoned authorization logic sets the owner/admin address as address(0), which can be deterministically returned by ecrecover when v is set to any positive number other than 27 or 28. This vulnerability can enable attackers to spoof authorized-only methods to execute as if the authorized account were the signer, potentially taking control of the contract. To address this, developers should mitigate the issue by adding a check to ensure that the recovered signer address is not null before allowing owner changes, preventing unauthorized takeovers. - https://kadenzipfel.github.io/smart-contract-vulnerabilities/vulnerabilities/unexpected-ecrecover-null-address.html
- https://docs.soliditylang.org/en/latest/units-and-global-variables.html#mathematical-and-cryptographic-functions
- https://ethereum.stackexchange.com/a/69329
## Off-By-One Redflag Off-by-one errors are a frequent programming mistake where intended boundaries are erroneously set off by just one, even though they may seem minor, these errors can have significant consequences. They often occur when determining array lengths, as developers may overlook the fact that in 0-indexed arrays, the final element is at index array.length - 1, leading to unintended exclusions. Likewise, comparison operators can be misused, such as using > instead of >=, especially when negations are involved, causing confusion between the intended and implemented boundaries. These errors can result in unintended behavior, like missing the last element in an array during a loop or erroneously triggering actions when certain conditions are met, leading to potentially serious issues in software functionality. - link
- https://github.com/OpenCoreCH/smart-contract-auditing-heuristics#off-by-one-errors
## Lack of Precision Redflag In Solidity, there's a limited range of number types, with no support for floating-point numbers and only partial support for fixed-point numbers, which cannot be assigned to or from. Integers are the primary number type, and results of calculations are always rounded down. When performing division with integers, precision can be lost due to remainders, leading to potential flaws. For instance, in a scenario where a fee for early withdrawals is calculated based on the number of days early, the lack of precision can become problematic. If a user withdraws 1.99 days early, the rounding down causes them to pay only about half of the intended fee. To mitigate such precision errors, it's advisable to ensure that numerators are significantly larger than denominators, and a common solution involves using fixed-point logic by raising integers to a sufficient number of decimals, often referred to as "WAD" or 1e18, to minimize the impact of precision on contract logic. link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link
Redflag Notes link