diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c00fda8aad..5cf02136bd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ For each new detector, at least one regression tests must be present. 1. Create a folder in `tests/e2e/detectors/test_data` with the detector's argument name. 2. Create a test contract in `tests/e2e/detectors/test_data//`. -3. Update `ALL_TEST` in `tests/e2e/detectors/test_detectors.py` +3. Update `ALL_TESTS` in `tests/e2e/detectors/test_detectors.py`. 4. Run `python tests/e2e/detectors/test_detectors.py --compile` to create a zip file of the compilation artifacts. 5. `pytest tests/e2e/detectors/test_detectors.py --insta update-new`. This will generate a snapshot of the detector output in `tests/e2e/detectors/snapshots/`. If updating an existing detector, run `pytest tests/e2e/detectors/test_detectors.py --insta review` and accept or reject the updates. 6. Run `pytest tests/e2e/detectors/test_detectors.py` to ensure everything worked. Then, add and commit the files to git. @@ -97,8 +97,9 @@ For each new detector, at least one regression tests must be present. 1. Create a test in `tests/e2e/solc_parsing/` 2. Run `python tests/e2e/solc_parsing/test_ast_parsing.py --compile`. This will compile the artifact in `tests/e2e/solc_parsing/compile`. Add the compiled artifact to git. -3. Run `python tests/e2e/solc_parsing/test_ast_parsing.py --generate`. This will generate the json artifacts in `tests/e2e/solc_parsing/expected_json`. Add the generated files to git. -4. Run `pytest tests/e2e/solc_parsing/test_ast_parsing.py` and check that everything worked. +3. Update `ALL_TESTS` in `tests/e2e/solc_parsing/test_ast_parsing.py`. +4. Run `python tests/e2e/solc_parsing/test_ast_parsing.py --generate`. This will generate the json artifacts in `tests/e2e/solc_parsing/expected_json`. Add the generated files to git. +5. Run `pytest tests/e2e/solc_parsing/test_ast_parsing.py` and check that everything worked. > ##### Helpful commands for parsing tests > diff --git a/README.md b/README.md index bce20bfb07..cb815561e8 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ # Slither, the Solidity source analyzer -Logo + +Logo [![Build Status](https://img.shields.io/github/actions/workflow/status/crytic/slither/ci.yml?branch=master)](https://github.com/crytic/slither/actions?query=workflow%3ACI) [![Slack Status](https://empireslacking.herokuapp.com/badge.svg)](https://empireslacking.herokuapp.com) @@ -20,26 +21,29 @@ Slither is a Solidity static analysis framework written in Python3. It runs a su ## Features -* Detects vulnerable Solidity code with low false positives (see the list of [trophies](./trophies.md)) -* Identifies where the error condition occurs in the source code -* Easily integrates into continuous integration and Hardhat/Foundry builds -* Built-in 'printers' quickly report crucial contract information -* Detector API to write custom analyses in Python -* Ability to analyze contracts written with Solidity >= 0.4 -* Intermediate representation ([SlithIR](https://github.com/trailofbits/slither/wiki/SlithIR)) enables simple, high-precision analyses -* Correctly parses 99.9% of all public Solidity code -* Average execution time of less than 1 second per contract -* Integrates with Github's code scanning in [CI](https://github.com/marketplace/actions/slither-action) +- Detects vulnerable Solidity code with low false positives (see the list of [trophies](./trophies.md)) +- Identifies where the error condition occurs in the source code +- Easily integrates into continuous integration and Hardhat/Foundry builds +- Built-in 'printers' quickly report crucial contract information +- Detector API to write custom analyses in Python +- Ability to analyze contracts written with Solidity >= 0.4 +- Intermediate representation ([SlithIR](https://github.com/trailofbits/slither/wiki/SlithIR)) enables simple, high-precision analyses +- Correctly parses 99.9% of all public Solidity code +- Average execution time of less than 1 second per contract +- Integrates with Github's code scanning in [CI](https://github.com/marketplace/actions/slither-action) ## Usage Run Slither on a Hardhat/Foundry/Dapp/Brownie application: + ```bash slither . ``` + This is the preferred option if your project has dependencies as Slither relies on the underlying compilation framework to compile source code. However, you can run Slither on a single file that does not import dependencies: + ```bash slither tests/uninitialized.sol ``` @@ -79,118 +83,122 @@ docker run -it -v /home/share:/share trailofbits/eth-security-toolbox ``` ### Integration + - For GitHub action integration, use [slither-action](https://github.com/marketplace/actions/slither-action). - To generate a Markdown report, use `slither [target] --checklist`. - To generate a Markdown with GitHub source code highlighting, use `slither [target] --checklist --markdown-root https://github.com/ORG/REPO/blob/COMMIT/` (replace `ORG`, `REPO`, `COMMIT`) ## Detectors - Num | Detector | What it Detects | Impact | Confidence --- | --- | --- | --- | --- 1 | `abiencoderv2-array` | [Storage abiencoderv2 array](https://github.com/crytic/slither/wiki/Detector-Documentation#storage-abiencoderv2-array) | High | High 2 | `arbitrary-send-erc20` | [transferFrom uses arbitrary `from`](https://github.com/crytic/slither/wiki/Detector-Documentation#arbitrary-from-in-transferfrom) | High | High 3 | `array-by-reference` | [Modifying storage array by value](https://github.com/crytic/slither/wiki/Detector-Documentation#modifying-storage-array-by-value) | High | High -4 | `incorrect-shift` | [The order of parameters in a shift instruction is incorrect.](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-shift-in-assembly) | High | High -5 | `multiple-constructors` | [Multiple constructor schemes](https://github.com/crytic/slither/wiki/Detector-Documentation#multiple-constructor-schemes) | High | High -6 | `name-reused` | [Contract's name reused](https://github.com/crytic/slither/wiki/Detector-Documentation#name-reused) | High | High -7 | `protected-vars` | [Detected unprotected variables](https://github.com/crytic/slither/wiki/Detector-Documentation#protected-variables) | High | High -8 | `public-mappings-nested` | [Public mappings with nested variables](https://github.com/crytic/slither/wiki/Detector-Documentation#public-mappings-with-nested-variables) | High | High -9 | `rtlo` | [Right-To-Left-Override control character is used](https://github.com/crytic/slither/wiki/Detector-Documentation#right-to-left-override-character) | High | High -10 | `shadowing-state` | [State variables shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variable-shadowing) | High | High -11 | `suicidal` | [Functions allowing anyone to destruct the contract](https://github.com/crytic/slither/wiki/Detector-Documentation#suicidal) | High | High -12 | `uninitialized-state` | [Uninitialized state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-state-variables) | High | High -13 | `uninitialized-storage` | [Uninitialized storage variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-storage-variables) | High | High -14 | `unprotected-upgrade` | [Unprotected upgradeable contract](https://github.com/crytic/slither/wiki/Detector-Documentation#unprotected-upgradeable-contract) | High | High -15 | `codex` | [Use Codex to find vulnerabilities.](https://github.com/crytic/slither/wiki/Detector-Documentation#codex) | High | Low -16 | `arbitrary-send-erc20-permit` | [transferFrom uses arbitrary from with permit](https://github.com/crytic/slither/wiki/Detector-Documentation#arbitrary-from-in-transferfrom-used-with-permit) | High | Medium -17 | `arbitrary-send-eth` | [Functions that send Ether to arbitrary destinations](https://github.com/crytic/slither/wiki/Detector-Documentation#functions-that-send-ether-to-arbitrary-destinations) | High | Medium -18 | `controlled-array-length` | [Tainted array length assignment](https://github.com/crytic/slither/wiki/Detector-Documentation#array-length-assignment) | High | Medium -19 | `controlled-delegatecall` | [Controlled delegatecall destination](https://github.com/crytic/slither/wiki/Detector-Documentation#controlled-delegatecall) | High | Medium -20 | `delegatecall-loop` | [Payable functions using `delegatecall` inside a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/#payable-functions-using-delegatecall-inside-a-loop) | High | Medium -21 | `msg-value-loop` | [msg.value inside a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/#msgvalue-inside-a-loop) | High | Medium -22 | `reentrancy-eth` | [Reentrancy vulnerabilities (theft of ethers)](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities) | High | Medium -23 | `storage-array` | [Signed storage integer array compiler bug](https://github.com/crytic/slither/wiki/Detector-Documentation#storage-signed-integer-array) | High | Medium -24 | `unchecked-transfer` | [Unchecked tokens transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-transfer) | High | Medium -25 | `weak-prng` | [Weak PRNG](https://github.com/crytic/slither/wiki/Detector-Documentation#weak-PRNG) | High | Medium -26 | `domain-separator-collision` | [Detects ERC20 tokens that have a function whose signature collides with EIP-2612's DOMAIN_SEPARATOR()](https://github.com/crytic/slither/wiki/Detector-Documentation#domain-separator-collision) | Medium | High -27 | `enum-conversion` | [Detect dangerous enum conversion](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-enum-conversion) | Medium | High -28 | `erc20-interface` | [Incorrect ERC20 interfaces](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc20-interface) | Medium | High -29 | `erc721-interface` | [Incorrect ERC721 interfaces](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc721-interface) | Medium | High -30 | `incorrect-equality` | [Dangerous strict equalities](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-strict-equalities) | Medium | High -31 | `locked-ether` | [Contracts that lock ether](https://github.com/crytic/slither/wiki/Detector-Documentation#contracts-that-lock-ether) | Medium | High -32 | `mapping-deletion` | [Deletion on mapping containing a structure](https://github.com/crytic/slither/wiki/Detector-Documentation#deletion-on-mapping-containing-a-structure) | Medium | High -33 | `shadowing-abstract` | [State variables shadowing from abstract contracts](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variable-shadowing-from-abstract-contracts) | Medium | High -34 | `tautology` | [Tautology or contradiction](https://github.com/crytic/slither/wiki/Detector-Documentation#tautology-or-contradiction) | Medium | High -35 | `write-after-write` | [Unused write](https://github.com/crytic/slither/wiki/Detector-Documentation#write-after-write) | Medium | High -36 | `boolean-cst` | [Misuse of Boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#misuse-of-a-boolean-constant) | Medium | Medium -37 | `constant-function-asm` | [Constant functions using assembly code](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-using-assembly-code) | Medium | Medium -38 | `constant-function-state` | [Constant functions changing the state](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state) | Medium | Medium -39 | `divide-before-multiply` | [Imprecise arithmetic operations order](https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply) | Medium | Medium -40 | `reentrancy-no-eth` | [Reentrancy vulnerabilities (no theft of ethers)](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-1) | Medium | Medium -41 | `reused-constructor` | [Reused base constructor](https://github.com/crytic/slither/wiki/Detector-Documentation#reused-base-constructors) | Medium | Medium -42 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-usage-of-txorigin) | Medium | Medium -43 | `unchecked-lowlevel` | [Unchecked low-level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level-calls) | Medium | Medium -44 | `unchecked-send` | [Unchecked send](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-send) | Medium | Medium -45 | `uninitialized-local` | [Uninitialized local variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables) | Medium | Medium -46 | `unused-return` | [Unused return values](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return) | Medium | Medium -47 | `incorrect-modifier` | [Modifiers that can return the default value](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-modifier) | Low | High -48 | `shadowing-builtin` | [Built-in symbol shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#builtin-symbol-shadowing) | Low | High -49 | `shadowing-local` | [Local variables shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#local-variable-shadowing) | Low | High -50 | `uninitialized-fptr-cst` | [Uninitialized function pointer calls in constructors](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-function-pointers-in-constructors) | Low | High -51 | `variable-scope` | [Local variables used prior their declaration](https://github.com/crytic/slither/wiki/Detector-Documentation#pre-declaration-usage-of-local-variables) | Low | High -52 | `void-cst` | [Constructor called not implemented](https://github.com/crytic/slither/wiki/Detector-Documentation#void-constructor) | Low | High -53 | `calls-loop` | [Multiple calls in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/#calls-inside-a-loop) | Low | Medium -54 | `events-access` | [Missing Events Access Control](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-events-access-control) | Low | Medium -55 | `events-maths` | [Missing Events Arithmetic](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-events-arithmetic) | Low | Medium -56 | `incorrect-unary` | [Dangerous unary expressions](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-unary-expressions) | Low | Medium -57 | `missing-zero-check` | [Missing Zero Address Validation](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-zero-address-validation) | Low | Medium -58 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2) | Low | Medium -59 | `reentrancy-events` | [Reentrancy vulnerabilities leading to out-of-order Events](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-3) | Low | Medium -60 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp) | Low | Medium -61 | `assembly` | [Assembly usage](https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage) | Informational | High -62 | `assert-state-change` | [Assert state change](https://github.com/crytic/slither/wiki/Detector-Documentation#assert-state-change) | Informational | High -63 | `boolean-equal` | [Comparison to boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#boolean-equality) | Informational | High -64 | `cyclomatic-complexity` | [Detects functions with high (> 11) cyclomatic complexity](https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity) | Informational | High -65 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High -66 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High -67 | `function-init-state` | [Function initializing state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#function-initializing-state) | Informational | High -68 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High -69 | `missing-inheritance` | [Missing inheritance](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-inheritance) | Informational | High -70 | `naming-convention` | [Conformity to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High -71 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High -72 | `redundant-statements` | [Redundant statements](https://github.com/crytic/slither/wiki/Detector-Documentation#redundant-statements) | Informational | High -73 | `solc-version` | [Incorrect Solidity version](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity) | Informational | High -74 | `unimplemented-functions` | [Unimplemented functions](https://github.com/crytic/slither/wiki/Detector-Documentation#unimplemented-functions) | Informational | High -75 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variable) | Informational | High -76 | `costly-loop` | [Costly operations in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation#costly-operations-inside-a-loop) | Informational | Medium -77 | `dead-code` | [Functions that are not used](https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code) | Informational | Medium -78 | `reentrancy-unlimited-gas` | [Reentrancy vulnerabilities through send and transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-4) | Informational | Medium -79 | `similar-names` | [Variable names are too similar](https://github.com/crytic/slither/wiki/Detector-Documentation#variable-names-too-similar) | Informational | Medium -80 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium -81 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Optimization | High -82 | `external-function` | [Public function that could be declared external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external) | Optimization | High -83 | `immutable-states` | [State variables that could be declared immutable](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-immutable) | Optimization | High -84 | `var-read-using-this` | [Contract reads its own variable using `this`](https://github.com/crytic/slither/wiki/Vulnerabilities-Description#public-variable-read-in-external-context) | Optimization | High +4 | `encode-packed-collision` | [ABI encodePacked Collision](https://github.com/crytic/slither/wiki/Detector-Documentation#abi-encodePacked-collision) | High | High +5 | `incorrect-shift` | [The order of parameters in a shift instruction is incorrect.](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-shift-in-assembly) | High | High +6 | `multiple-constructors` | [Multiple constructor schemes](https://github.com/crytic/slither/wiki/Detector-Documentation#multiple-constructor-schemes) | High | High +7 | `name-reused` | [Contract's name reused](https://github.com/crytic/slither/wiki/Detector-Documentation#name-reused) | High | High +8 | `protected-vars` | [Detected unprotected variables](https://github.com/crytic/slither/wiki/Detector-Documentation#protected-variables) | High | High +9 | `public-mappings-nested` | [Public mappings with nested variables](https://github.com/crytic/slither/wiki/Detector-Documentation#public-mappings-with-nested-variables) | High | High +10 | `rtlo` | [Right-To-Left-Override control character is used](https://github.com/crytic/slither/wiki/Detector-Documentation#right-to-left-override-character) | High | High +11 | `shadowing-state` | [State variables shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variable-shadowing) | High | High +12 | `suicidal` | [Functions allowing anyone to destruct the contract](https://github.com/crytic/slither/wiki/Detector-Documentation#suicidal) | High | High +13 | `uninitialized-state` | [Uninitialized state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-state-variables) | High | High +14 | `uninitialized-storage` | [Uninitialized storage variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-storage-variables) | High | High +15 | `unprotected-upgrade` | [Unprotected upgradeable contract](https://github.com/crytic/slither/wiki/Detector-Documentation#unprotected-upgradeable-contract) | High | High +16 | `codex` | [Use Codex to find vulnerabilities.](https://github.com/crytic/slither/wiki/Detector-Documentation#codex) | High | Low +17 | `arbitrary-send-erc20-permit` | [transferFrom uses arbitrary from with permit](https://github.com/crytic/slither/wiki/Detector-Documentation#arbitrary-from-in-transferfrom-used-with-permit) | High | Medium +18 | `arbitrary-send-eth` | [Functions that send Ether to arbitrary destinations](https://github.com/crytic/slither/wiki/Detector-Documentation#functions-that-send-ether-to-arbitrary-destinations) | High | Medium +19 | `controlled-array-length` | [Tainted array length assignment](https://github.com/crytic/slither/wiki/Detector-Documentation#array-length-assignment) | High | Medium +20 | `controlled-delegatecall` | [Controlled delegatecall destination](https://github.com/crytic/slither/wiki/Detector-Documentation#controlled-delegatecall) | High | Medium +21 | `delegatecall-loop` | [Payable functions using `delegatecall` inside a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/#payable-functions-using-delegatecall-inside-a-loop) | High | Medium +22 | `msg-value-loop` | [msg.value inside a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/#msgvalue-inside-a-loop) | High | Medium +23 | `reentrancy-eth` | [Reentrancy vulnerabilities (theft of ethers)](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities) | High | Medium +24 | `storage-array` | [Signed storage integer array compiler bug](https://github.com/crytic/slither/wiki/Detector-Documentation#storage-signed-integer-array) | High | Medium +25 | `unchecked-transfer` | [Unchecked tokens transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-transfer) | High | Medium +26 | `weak-prng` | [Weak PRNG](https://github.com/crytic/slither/wiki/Detector-Documentation#weak-PRNG) | High | Medium +27 | `domain-separator-collision` | [Detects ERC20 tokens that have a function whose signature collides with EIP-2612's DOMAIN_SEPARATOR()](https://github.com/crytic/slither/wiki/Detector-Documentation#domain-separator-collision) | Medium | High +28 | `enum-conversion` | [Detect dangerous enum conversion](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-enum-conversion) | Medium | High +29 | `erc20-interface` | [Incorrect ERC20 interfaces](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc20-interface) | Medium | High +30 | `erc721-interface` | [Incorrect ERC721 interfaces](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc721-interface) | Medium | High +31 | `incorrect-equality` | [Dangerous strict equalities](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-strict-equalities) | Medium | High +32 | `locked-ether` | [Contracts that lock ether](https://github.com/crytic/slither/wiki/Detector-Documentation#contracts-that-lock-ether) | Medium | High +33 | `mapping-deletion` | [Deletion on mapping containing a structure](https://github.com/crytic/slither/wiki/Detector-Documentation#deletion-on-mapping-containing-a-structure) | Medium | High +34 | `shadowing-abstract` | [State variables shadowing from abstract contracts](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variable-shadowing-from-abstract-contracts) | Medium | High +35 | `tautology` | [Tautology or contradiction](https://github.com/crytic/slither/wiki/Detector-Documentation#tautology-or-contradiction) | Medium | High +36 | `write-after-write` | [Unused write](https://github.com/crytic/slither/wiki/Detector-Documentation#write-after-write) | Medium | High +37 | `boolean-cst` | [Misuse of Boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#misuse-of-a-boolean-constant) | Medium | Medium +38 | `constant-function-asm` | [Constant functions using assembly code](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-using-assembly-code) | Medium | Medium +39 | `constant-function-state` | [Constant functions changing the state](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state) | Medium | Medium +40 | `divide-before-multiply` | [Imprecise arithmetic operations order](https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply) | Medium | Medium +41 | `reentrancy-no-eth` | [Reentrancy vulnerabilities (no theft of ethers)](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-1) | Medium | Medium +42 | `reused-constructor` | [Reused base constructor](https://github.com/crytic/slither/wiki/Detector-Documentation#reused-base-constructors) | Medium | Medium +43 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-usage-of-txorigin) | Medium | Medium +44 | `unchecked-lowlevel` | [Unchecked low-level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level-calls) | Medium | Medium +45 | `unchecked-send` | [Unchecked send](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-send) | Medium | Medium +46 | `uninitialized-local` | [Uninitialized local variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables) | Medium | Medium +47 | `unused-return` | [Unused return values](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return) | Medium | Medium +48 | `incorrect-modifier` | [Modifiers that can return the default value](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-modifier) | Low | High +49 | `shadowing-builtin` | [Built-in symbol shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#builtin-symbol-shadowing) | Low | High +50 | `shadowing-local` | [Local variables shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#local-variable-shadowing) | Low | High +51 | `uninitialized-fptr-cst` | [Uninitialized function pointer calls in constructors](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-function-pointers-in-constructors) | Low | High +52 | `variable-scope` | [Local variables used prior their declaration](https://github.com/crytic/slither/wiki/Detector-Documentation#pre-declaration-usage-of-local-variables) | Low | High +53 | `void-cst` | [Constructor called not implemented](https://github.com/crytic/slither/wiki/Detector-Documentation#void-constructor) | Low | High +54 | `calls-loop` | [Multiple calls in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/#calls-inside-a-loop) | Low | Medium +55 | `events-access` | [Missing Events Access Control](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-events-access-control) | Low | Medium +56 | `events-maths` | [Missing Events Arithmetic](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-events-arithmetic) | Low | Medium +57 | `incorrect-unary` | [Dangerous unary expressions](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-unary-expressions) | Low | Medium +58 | `missing-zero-check` | [Missing Zero Address Validation](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-zero-address-validation) | Low | Medium +59 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2) | Low | Medium +60 | `reentrancy-events` | [Reentrancy vulnerabilities leading to out-of-order Events](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-3) | Low | Medium +61 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp) | Low | Medium +62 | `assembly` | [Assembly usage](https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage) | Informational | High +63 | `assert-state-change` | [Assert state change](https://github.com/crytic/slither/wiki/Detector-Documentation#assert-state-change) | Informational | High +64 | `boolean-equal` | [Comparison to boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#boolean-equality) | Informational | High +65 | `cyclomatic-complexity` | [Detects functions with high (> 11) cyclomatic complexity](https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity) | Informational | High +66 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High +67 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High +68 | `function-init-state` | [Function initializing state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#function-initializing-state) | Informational | High +69 | `incorrect-using-for` | [Detects using-for statement usage when no function from a given library matches a given type](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-using-for-usage) | Informational | High +70 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High +71 | `missing-inheritance` | [Missing inheritance](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-inheritance) | Informational | High +72 | `naming-convention` | [Conformity to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High +73 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High +74 | `redundant-statements` | [Redundant statements](https://github.com/crytic/slither/wiki/Detector-Documentation#redundant-statements) | Informational | High +75 | `solc-version` | [Incorrect Solidity version](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity) | Informational | High +76 | `unimplemented-functions` | [Unimplemented functions](https://github.com/crytic/slither/wiki/Detector-Documentation#unimplemented-functions) | Informational | High +77 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variable) | Informational | High +78 | `costly-loop` | [Costly operations in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation#costly-operations-inside-a-loop) | Informational | Medium +79 | `dead-code` | [Functions that are not used](https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code) | Informational | Medium +80 | `reentrancy-unlimited-gas` | [Reentrancy vulnerabilities through send and transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-4) | Informational | Medium +81 | `similar-names` | [Variable names are too similar](https://github.com/crytic/slither/wiki/Detector-Documentation#variable-names-too-similar) | Informational | Medium +82 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium +83 | `cache-array-length` | [Detects `for` loops that use `length` member of some storage array in their loop condition and don't modify it.](https://github.com/crytic/slither/wiki/Detector-Documentation#cache-array-length) | Optimization | High +84 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Optimization | High +85 | `external-function` | [Public function that could be declared external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external) | Optimization | High +86 | `immutable-states` | [State variables that could be declared immutable](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-immutable) | Optimization | High +87 | `var-read-using-this` | [Contract reads its own variable using `this`](https://github.com/crytic/slither/wiki/Detector-Documentation#public-variable-read-in-external-context) | Optimization | High For more information, see + - The [Detector Documentation](https://github.com/crytic/slither/wiki/Detector-Documentation) for details on each detector - The [Detection Selection](https://github.com/crytic/slither/wiki/Usage#detector-selection) to run only selected detectors. By default, all the detectors are run. - The [Triage Mode](https://github.com/crytic/slither/wiki/Usage#triage-mode) to filter individual results ## Printers - ### Quick Review Printers - `human-summary`: [Print a human-readable summary of the contracts](https://github.com/trailofbits/slither/wiki/Printer-documentation#human-summary) - `inheritance-graph`: [Export the inheritance graph of each contract to a dot file](https://github.com/trailofbits/slither/wiki/Printer-documentation#inheritance-graph) - `contract-summary`: [Print a summary of the contracts](https://github.com/trailofbits/slither/wiki/Printer-documentation#contract-summary) +- `loc`: [Count the total number lines of code (LOC), source lines of code (SLOC), and comment lines of code (CLOC) found in source files (SRC), dependencies (DEP), and test files (TEST).](https://github.com/trailofbits/slither/wiki/Printer-documentation#loc) ### In-Depth Review Printers - `call-graph`: [Export the call-graph of the contracts to a dot file](https://github.com/trailofbits/slither/wiki/Printer-documentation#call-graph) - `cfg`: [Export the CFG of each functions](https://github.com/trailofbits/slither/wiki/Printer-documentation#cfg) - `function-summary`: [Print a summary of the functions](https://github.com/trailofbits/slither/wiki/Printer-documentation#function-summary) - `vars-and-auth`: [Print the state variables written and the authorization of the functions](https://github.com/crytic/slither/wiki/Printer-documentation#variables-written-and-authorization) -- `when-not-paused`: [Print functions that do not use `whenNotPaused` modifier](https://github.com/trailofbits/slither/wiki/Printer-documentation#when-not-paused). +- `not-pausable`: [Print functions that do not use `whenNotPaused` modifier](https://github.com/trailofbits/slither/wiki/Printer-documentation#when-not-paused). To run a printer, use `--print` and a comma-separated list of printers. @@ -204,32 +212,36 @@ See the [Printer documentation](https://github.com/crytic/slither/wiki/Printer-d - `slither-check-erc`: [Check the ERC's conformance](https://github.com/crytic/slither/wiki/ERC-Conformance) - `slither-format`: [Automatic patch generation](https://github.com/crytic/slither/wiki/Slither-format) - `slither-read-storage`: [Read storage values from contracts](./slither/tools/read_storage/README.md) +- `slither-interface`: [Generate an interface for a contract](./slither/tools/interface/README.md) See the [Tool documentation](https://github.com/crytic/slither/wiki/Tool-Documentation) for additional tools. [Contact us](https://www.trailofbits.com/contact/) to get help on building custom tools. ## API Documentation + Documentation on Slither's internals is available [here](https://crytic.github.io/slither/slither.html). ## Getting Help Feel free to stop by our [Slack channel](https://empireslacking.herokuapp.com) (#ethereum) for help using or extending Slither. -* The [Printer documentation](https://github.com/trailofbits/slither/wiki/Printer-documentation) describes the information Slither is capable of visualizing for each contract. +- The [Printer documentation](https://github.com/trailofbits/slither/wiki/Printer-documentation) describes the information Slither is capable of visualizing for each contract. -* The [Detector documentation](https://github.com/trailofbits/slither/wiki/Adding-a-new-detector) describes how to write a new vulnerability analyses. +- The [Detector documentation](https://github.com/trailofbits/slither/wiki/Adding-a-new-detector) describes how to write a new vulnerability analyses. -* The [API documentation](https://github.com/crytic/slither/wiki/Python-API) describes the methods and objects available for custom analyses. +- The [API documentation](https://github.com/crytic/slither/wiki/Python-API) describes the methods and objects available for custom analyses. -* The [SlithIR documentation](https://github.com/trailofbits/slither/wiki/SlithIR) describes the SlithIR intermediate representation. +- The [SlithIR documentation](https://github.com/trailofbits/slither/wiki/SlithIR) describes the SlithIR intermediate representation. ## FAQ How do I exclude mocks or tests? + - View our documentation on [path filtering](https://github.com/crytic/slither/wiki/Usage#path-filtering). How do I fix "unknown file" or compilation issues? + - Because slither requires the solc AST, it must have all dependencies available. If a contract has dependencies, `slither contract.sol` will fail. Instead, use `slither .` in the parent directory of `contracts/` (you should see `contracts/` when you run `ls`). @@ -244,9 +256,11 @@ Slither is licensed and distributed under the AGPLv3 license. [Contact us](mailt ## Publications ### Trail of Bits publication + - [Slither: A Static Analysis Framework For Smart Contracts](https://arxiv.org/abs/1908.09878), Josselin Feist, Gustavo Grieco, Alex Groce - WETSEB '19 ### External publications + Title | Usage | Authors | Venue | Code --- | --- | --- | --- | --- [ReJection: A AST-Based Reentrancy Vulnerability Detection Method](https://www.researchgate.net/publication/339354823_ReJection_A_AST-Based_Reentrancy_Vulnerability_Detection_Method) | AST-based analysis built on top of Slither | Rui Ma, Zefeng Jian, Guangyuan Chen, Ke Ma, Yujia Chen | CTCIS 19 diff --git a/slither/__main__.py b/slither/__main__.py index ca61a82691..d9201a90d9 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -35,6 +35,7 @@ from slither.utils.output_capture import StandardOutputCapture from slither.utils.colors import red, set_colorization_enabled from slither.utils.command_line import ( + FailOnLevel, output_detectors, output_results_to_markdown, output_detectors_json, @@ -206,22 +207,22 @@ def choose_detectors( detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT) return detectors_to_run - if args.exclude_optimization and not args.fail_pedantic: + if args.exclude_optimization: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.OPTIMIZATION ] - if args.exclude_informational and not args.fail_pedantic: + if args.exclude_informational: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.INFORMATIONAL ] - if args.exclude_low and not args.fail_low: + if args.exclude_low: detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.LOW] - if args.exclude_medium and not args.fail_medium: + if args.exclude_medium: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.MEDIUM ] - if args.exclude_high and not args.fail_high: + if args.exclude_high: detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.HIGH] if args.detectors_to_exclude: detectors_to_run = [ @@ -386,41 +387,44 @@ def parse_args( default=defaults_flag_in_config["exclude_high"], ) - group_detector.add_argument( + fail_on_group = group_detector.add_mutually_exclusive_group() + fail_on_group.add_argument( "--fail-pedantic", - help="Return the number of findings in the exit code", - action="store_true", - default=defaults_flag_in_config["fail_pedantic"], + help="Fail if any findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.PEDANTIC, ) - - group_detector.add_argument( - "--no-fail-pedantic", - help="Do not return the number of findings in the exit code. Opposite of --fail-pedantic", - dest="fail_pedantic", - action="store_false", - required=False, - ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-low", - help="Fail if low or greater impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_low"], + help="Fail if any low or greater impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.LOW, ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-medium", - help="Fail if medium or greater impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_medium"], + help="Fail if any medium or greater impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.MEDIUM, ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-high", - help="Fail if high impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_high"], + help="Fail if any high impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.HIGH, + ) + fail_on_group.add_argument( + "--fail-none", + "--no-fail-pedantic", + help="Do not return the number of findings in the exit code", + action="store_const", + dest="fail_on", + const=FailOnLevel.NONE, ) + fail_on_group.set_defaults(fail_on=FailOnLevel.PEDANTIC) group_detector.add_argument( "--show-ignored-findings", @@ -896,17 +900,18 @@ def main_impl( stats = pstats.Stats(cp).sort_stats("cumtime") stats.print_stats() - if args.fail_high: + fail_on = FailOnLevel(args.fail_on) + if fail_on == FailOnLevel.HIGH: fail_on_detection = any(result["impact"] == "High" for result in results_detectors) - elif args.fail_medium: + elif fail_on == FailOnLevel.MEDIUM: fail_on_detection = any( result["impact"] in ["Medium", "High"] for result in results_detectors ) - elif args.fail_low: + elif fail_on == FailOnLevel.LOW: fail_on_detection = any( result["impact"] in ["Low", "Medium", "High"] for result in results_detectors ) - elif args.fail_pedantic: + elif fail_on == FailOnLevel.PEDANTIC: fail_on_detection = bool(results_detectors) else: fail_on_detection = False diff --git a/slither/core/solidity_types/type_alias.py b/slither/core/solidity_types/type_alias.py index 9387f511aa..ead9b5394f 100644 --- a/slither/core/solidity_types/type_alias.py +++ b/slither/core/solidity_types/type_alias.py @@ -1,10 +1,11 @@ -from typing import TYPE_CHECKING, Tuple +from typing import TYPE_CHECKING, Tuple, Dict from slither.core.declarations.top_level import TopLevel from slither.core.declarations.contract_level import ContractLevel from slither.core.solidity_types import Type, ElementaryType if TYPE_CHECKING: + from slither.core.declarations.function_top_level import FunctionTopLevel from slither.core.declarations import Contract from slither.core.scope.scope import FileScope @@ -43,6 +44,8 @@ class TypeAliasTopLevel(TypeAlias, TopLevel): def __init__(self, underlying_type: ElementaryType, name: str, scope: "FileScope") -> None: super().__init__(underlying_type, name) self.file_scope: "FileScope" = scope + # operators redefined + self.operators: Dict[str, "FunctionTopLevel"] = {} def __str__(self) -> str: return self.name diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index f7f1efaa5e..57f44bc56e 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -89,5 +89,6 @@ from .functions.permit_domain_signature_collision import DomainSeparatorCollision from .functions.codex import Codex from .functions.cyclomatic_complexity import CyclomaticComplexity +from .operations.cache_array_length import CacheArrayLength from .statements.incorrect_using_for import IncorrectUsingFor from .operations.encode_packed import EncodePackedCollision diff --git a/slither/detectors/operations/cache_array_length.py b/slither/detectors/operations/cache_array_length.py new file mode 100644 index 0000000000..da73d3fd5a --- /dev/null +++ b/slither/detectors/operations/cache_array_length.py @@ -0,0 +1,221 @@ +from typing import List, Set + +from slither.core.cfg.node import Node, NodeType +from slither.core.declarations import Function +from slither.core.expressions import BinaryOperation, Identifier, MemberAccess, UnaryOperation +from slither.core.solidity_types import ArrayType +from slither.core.source_mapping.source_mapping import SourceMapping +from slither.core.variables import StateVariable +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.slithir.operations import Length, Delete, HighLevelCall + + +class CacheArrayLength(AbstractDetector): + """ + Detects `for` loops that use `length` member of some storage array in their loop condition and don't modify it. + """ + + ARGUMENT = "cache-array-length" + HELP = ( + "Detects `for` loops that use `length` member of some storage array in their loop condition and don't " + "modify it. " + ) + IMPACT = DetectorClassification.OPTIMIZATION + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#cache-array-length" + + WIKI_TITLE = "Cache array length" + WIKI_DESCRIPTION = ( + "Detects `for` loops that use `length` member of some storage array in their loop condition " + "and don't modify it. " + ) + WIKI_EXPLOIT_SCENARIO = """ +```solidity +contract C +{ + uint[] array; + + function f() public + { + for (uint i = 0; i < array.length; i++) + { + // code that does not modify length of `array` + } + } +} +``` +Since the `for` loop in `f` doesn't modify `array.length`, it is more gas efficient to cache it in some local variable and use that variable instead, like in the following example: + +```solidity +contract C +{ + uint[] array; + + function f() public + { + uint array_length = array.length; + for (uint i = 0; i < array_length; i++) + { + // code that does not modify length of `array` + } + } +} +``` + """ + WIKI_RECOMMENDATION = ( + "Cache the lengths of storage arrays if they are used and not modified in `for` loops." + ) + + @staticmethod + def _is_identifier_member_access_comparison(exp: BinaryOperation) -> bool: + """ + Checks whether a BinaryOperation `exp` is an operation on Identifier and MemberAccess. + """ + return ( + isinstance(exp.expression_left, Identifier) + and isinstance(exp.expression_right, MemberAccess) + ) or ( + isinstance(exp.expression_left, MemberAccess) + and isinstance(exp.expression_right, Identifier) + ) + + @staticmethod + def _extract_array_from_length_member_access(exp: MemberAccess) -> StateVariable: + """ + Given a member access `exp`, it returns state array which `length` member is accessed through `exp`. + If array is not a state array or its `length` member is not referenced, it returns `None`. + """ + if exp.member_name != "length": + return None + if not isinstance(exp.expression, Identifier): + return None + if not isinstance(exp.expression.value, StateVariable): + return None + if not isinstance(exp.expression.value.type, ArrayType): + return None + return exp.expression.value + + @staticmethod + def _is_loop_referencing_array_length( + node: Node, visited: Set[Node], array: StateVariable, depth: int + ) -> True: + """ + For a given loop, checks if it references `array.length` at some point. + Will also return True if `array.length` is referenced but not changed. + This may potentially generate false negatives in the detector, but it was done this way because: + - situations when array `length` is referenced but not modified in loop are rare + - checking if `array.length` is indeed modified would require much more work + """ + visited.add(node) + if node.type == NodeType.STARTLOOP: + depth += 1 + if node.type == NodeType.ENDLOOP: + depth -= 1 + if depth == 0: + return False + + # Array length may change in the following situations: + # - when `push` is called + # - when `pop` is called + # - when `delete` is called on the entire array + # - when external function call is made (instructions from internal function calls are already in + # `node.all_slithir_operations()`, so we don't need to handle internal calls separately) + if node.type == NodeType.EXPRESSION: + for op in node.all_slithir_operations(): + if isinstance(op, Length) and op.value == array: + # op accesses array.length, not necessarily modifying it + return True + if isinstance(op, Delete): + # take into account only delete entire array, since delete array[i] doesn't change `array.length` + if ( + isinstance(op.expression, UnaryOperation) + and isinstance(op.expression.expression, Identifier) + and op.expression.expression.value == array + ): + return True + if isinstance(op, HighLevelCall) and not op.function.view and not op.function.pure: + return True + + for son in node.sons: + if son not in visited: + if CacheArrayLength._is_loop_referencing_array_length(son, visited, array, depth): + return True + return False + + @staticmethod + def _handle_loops(nodes: List[Node], non_optimal_array_len_usages: List[SourceMapping]) -> None: + """ + For each loop, checks if it has a comparison with `length` array member and, if it has, checks whether that + array size could potentially change in that loop. + If it cannot, the loop condition is added to `non_optimal_array_len_usages`. + There may be some false negatives here - see docs for `_is_loop_referencing_array_length` for more information. + """ + for node in nodes: + if node.type == NodeType.STARTLOOP: + if_node = node.sons[0] + if if_node.type != NodeType.IFLOOP: + continue + if not isinstance(if_node.expression, BinaryOperation): + continue + exp: BinaryOperation = if_node.expression + if not CacheArrayLength._is_identifier_member_access_comparison(exp): + continue + array: StateVariable + if isinstance(exp.expression_right, MemberAccess): + array = CacheArrayLength._extract_array_from_length_member_access( + exp.expression_right + ) + else: # isinstance(exp.expression_left, MemberAccess) == True + array = CacheArrayLength._extract_array_from_length_member_access( + exp.expression_left + ) + if array is None: + continue + + visited: Set[Node] = set() + if not CacheArrayLength._is_loop_referencing_array_length( + if_node, visited, array, 1 + ): + non_optimal_array_len_usages.append(if_node) + + @staticmethod + def _get_non_optimal_array_len_usages_for_function(f: Function) -> List[SourceMapping]: + """ + Finds non-optimal usages of array length in loop conditions in a given function. + """ + non_optimal_array_len_usages: List[SourceMapping] = [] + CacheArrayLength._handle_loops(f.nodes, non_optimal_array_len_usages) + + return non_optimal_array_len_usages + + @staticmethod + def _get_non_optimal_array_len_usages(functions: List[Function]) -> List[SourceMapping]: + """ + Finds non-optimal usages of array length in loop conditions in given functions. + """ + non_optimal_array_len_usages: List[SourceMapping] = [] + + for f in functions: + non_optimal_array_len_usages += ( + CacheArrayLength._get_non_optimal_array_len_usages_for_function(f) + ) + + return non_optimal_array_len_usages + + def _detect(self): + results = [] + + non_optimal_array_len_usages = CacheArrayLength._get_non_optimal_array_len_usages( + self.compilation_unit.functions + ) + for usage in non_optimal_array_len_usages: + info = [ + "Loop condition at ", + usage, + " should use cached array length instead of referencing `length` member " + "of the storage array.\n ", + ] + res = self.generate_result(info) + results.append(res) + return results diff --git a/slither/slithir/operations/index.py b/slither/slithir/operations/index.py index f38a25927f..4fcfb8a6d7 100644 --- a/slither/slithir/operations/index.py +++ b/slither/slithir/operations/index.py @@ -3,6 +3,7 @@ from slither.core.declarations import SolidityVariableComposed from slither.core.source_mapping.source_mapping import SourceMapping from slither.core.variables.variable import Variable +from slither.core.variables.top_level_variable import TopLevelVariable from slither.slithir.operations.lvalue import OperationWithLValue from slither.slithir.utils.utils import is_valid_lvalue, is_valid_rvalue, RVALUE, LVALUE from slither.slithir.variables.reference import ReferenceVariable @@ -13,8 +14,10 @@ def __init__( self, result: ReferenceVariable, left_variable: Variable, right_variable: RVALUE ) -> None: super().__init__() - assert is_valid_lvalue(left_variable) or left_variable == SolidityVariableComposed( - "msg.data" + assert ( + is_valid_lvalue(left_variable) + or left_variable == SolidityVariableComposed("msg.data") + or isinstance(left_variable, TopLevelVariable) ) assert is_valid_rvalue(right_variable) assert isinstance(result, ReferenceVariable) diff --git a/slither/slithir/variables/reference.py b/slither/slithir/variables/reference.py index 9ab51be655..2f99d322e4 100644 --- a/slither/slithir/variables/reference.py +++ b/slither/slithir/variables/reference.py @@ -2,6 +2,7 @@ from slither.core.declarations import Contract, Enum, SolidityVariable, Function from slither.core.variables.variable import Variable +from slither.core.variables.top_level_variable import TopLevelVariable if TYPE_CHECKING: from slither.core.cfg.node import Node @@ -46,7 +47,7 @@ def points_to(self, points_to): from slither.slithir.utils.utils import is_valid_lvalue assert is_valid_lvalue(points_to) or isinstance( - points_to, (SolidityVariable, Contract, Enum) + points_to, (SolidityVariable, Contract, Enum, TopLevelVariable) ) self._points_to = points_to diff --git a/slither/solc_parsing/declarations/using_for_top_level.py b/slither/solc_parsing/declarations/using_for_top_level.py index 707ad83ac7..fe72e57809 100644 --- a/slither/solc_parsing/declarations/using_for_top_level.py +++ b/slither/solc_parsing/declarations/using_for_top_level.py @@ -55,22 +55,29 @@ def analyze(self) -> None: self._propagate_global(type_name) else: for f in self._functions: - full_name_split = f["function"]["name"].split(".") - if len(full_name_split) == 1: + # User defined operator + if "operator" in f: # Top level function - function_name: str = full_name_split[0] - self._analyze_top_level_function(function_name, type_name) - elif len(full_name_split) == 2: - # It can be a top level function behind an aliased import - # or a library function - first_part = full_name_split[0] - function_name = full_name_split[1] - self._check_aliased_import(first_part, function_name, type_name) + function_name: str = f["definition"]["name"] + operator: str = f["operator"] + self._analyze_operator(operator, function_name, type_name) else: - # MyImport.MyLib.a we don't care of the alias - library_name_str = full_name_split[1] - function_name = full_name_split[2] - self._analyze_library_function(library_name_str, function_name, type_name) + full_name_split = f["function"]["name"].split(".") + if len(full_name_split) == 1: + # Top level function + function_name: str = full_name_split[0] + self._analyze_top_level_function(function_name, type_name) + elif len(full_name_split) == 2: + # It can be a top level function behind an aliased import + # or a library function + first_part = full_name_split[0] + function_name = full_name_split[1] + self._check_aliased_import(first_part, function_name, type_name) + else: + # MyImport.MyLib.a we don't care of the alias + library_name_str = full_name_split[1] + function_name = full_name_split[2] + self._analyze_library_function(library_name_str, function_name, type_name) def _check_aliased_import( self, @@ -101,6 +108,19 @@ def _analyze_top_level_function( self._propagate_global(type_name) break + def _analyze_operator( + self, operator: str, function_name: str, type_name: TypeAliasTopLevel + ) -> None: + for tl_function in self._using_for.file_scope.functions: + # The library function is bound to the first parameter's type + if ( + tl_function.name == function_name + and tl_function.parameters + and type_name == tl_function.parameters[0].type + ): + type_name.operators[operator] = tl_function + break + def _analyze_library_function( self, library_name: str, diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index 4d2cfc00fd..a0bce044c2 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -1,6 +1,6 @@ import logging import re -from typing import Union, Dict, TYPE_CHECKING +from typing import Union, Dict, TYPE_CHECKING, List, Any import slither.core.expressions.type_conversion from slither.core.declarations.solidity_variables import ( @@ -236,6 +236,24 @@ def _parse_elementary_type_name_expression( pass +def _user_defined_op_call( + caller_context: CallerContextExpression, src, function_id: int, args: List[Any], type_call: str +) -> CallExpression: + var, was_created = find_variable(None, caller_context, function_id) + + if was_created: + var.set_offset(src, caller_context.compilation_unit) + + identifier = Identifier(var) + identifier.set_offset(src, caller_context.compilation_unit) + + var.references.append(identifier.source_mapping) + + call = CallExpression(identifier, args, type_call) + call.set_offset(src, caller_context.compilation_unit) + return call + + def parse_expression(expression: Dict, caller_context: CallerContextExpression) -> "Expression": # pylint: disable=too-many-nested-blocks,too-many-statements """ @@ -274,16 +292,24 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression) if name == "UnaryOperation": if is_compact_ast: attributes = expression - else: - attributes = expression["attributes"] - assert "prefix" in attributes - operation_type = UnaryOperationType.get_type(attributes["operator"], attributes["prefix"]) - - if is_compact_ast: expression = parse_expression(expression["subExpression"], caller_context) else: + attributes = expression["attributes"] assert len(expression["children"]) == 1 expression = parse_expression(expression["children"][0], caller_context) + assert "prefix" in attributes + + # Use of user defined operation + if "function" in attributes: + return _user_defined_op_call( + caller_context, + src, + attributes["function"], + [expression], + attributes["typeDescriptions"]["typeString"], + ) + + operation_type = UnaryOperationType.get_type(attributes["operator"], attributes["prefix"]) unary_op = UnaryOperation(expression, operation_type) unary_op.set_offset(src, caller_context.compilation_unit) return unary_op @@ -291,17 +317,25 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression) if name == "BinaryOperation": if is_compact_ast: attributes = expression - else: - attributes = expression["attributes"] - operation_type = BinaryOperationType.get_type(attributes["operator"]) - - if is_compact_ast: left_expression = parse_expression(expression["leftExpression"], caller_context) right_expression = parse_expression(expression["rightExpression"], caller_context) else: assert len(expression["children"]) == 2 + attributes = expression["attributes"] left_expression = parse_expression(expression["children"][0], caller_context) right_expression = parse_expression(expression["children"][1], caller_context) + + # Use of user defined operation + if "function" in attributes: + return _user_defined_op_call( + caller_context, + src, + attributes["function"], + [left_expression, right_expression], + attributes["typeDescriptions"]["typeString"], + ) + + operation_type = BinaryOperationType.get_type(attributes["operator"]) binary_op = BinaryOperation(left_expression, right_expression, operation_type) binary_op.set_offset(src, caller_context.compilation_unit) return binary_op diff --git a/slither/solc_parsing/variables/local_variable_init_from_tuple.py b/slither/solc_parsing/variables/local_variable_init_from_tuple.py index 1a551c6957..f1c8728486 100644 --- a/slither/solc_parsing/variables/local_variable_init_from_tuple.py +++ b/slither/solc_parsing/variables/local_variable_init_from_tuple.py @@ -16,3 +16,21 @@ def underlying_variable(self) -> LocalVariableInitFromTuple: # Todo: Not sure how to overcome this with mypy assert isinstance(self._variable, LocalVariableInitFromTuple) return self._variable + + def _analyze_variable_attributes(self, attributes: Dict) -> None: + """' + Variable Location + Can be storage/memory or default + """ + if "storageLocation" in attributes: + location = attributes["storageLocation"] + self.underlying_variable.set_location(location) + else: + if "memory" in attributes["type"]: + self.underlying_variable.set_location("memory") + elif "storage" in attributes["type"]: + self.underlying_variable.set_location("storage") + else: + self.underlying_variable.set_location("default") + + super()._analyze_variable_attributes(attributes) diff --git a/slither/tools/interface/README.md b/slither/tools/interface/README.md new file mode 100644 index 0000000000..a77e780b0a --- /dev/null +++ b/slither/tools/interface/README.md @@ -0,0 +1,21 @@ +# Slither-interface + +Generates code for a Solidity interface from contract + +## Usage + +Run `slither-interface `. + +## CLI Interface +```shell +positional arguments: + contract_source The name of the contract (case sensitive) followed by the deployed contract address if verified on etherscan or project directory/filename for local contracts. + +optional arguments: + -h, --help show this help message and exit + --unroll-structs Whether to use structures' underlying types instead of the user-defined type + --exclude-events Excludes event signatures in the interface + --exclude-errors Excludes custom error signatures in the interface + --exclude-enums Excludes enum definitions in the interface + --exclude-structs Exclude struct definitions in the interface +``` \ No newline at end of file diff --git a/slither/tools/interface/__main__.py b/slither/tools/interface/__main__.py index e56c4b3ebe..0705f0373a 100644 --- a/slither/tools/interface/__main__.py +++ b/slither/tools/interface/__main__.py @@ -86,10 +86,11 @@ def main() -> None: ) # add version pragma - interface = ( - f"pragma solidity {_contract.compilation_unit.pragma_directives[0].version};\n\n" - + interface - ) + if _contract.compilation_unit.pragma_directives: + interface = ( + f"pragma solidity {_contract.compilation_unit.pragma_directives[0].version};\n\n" + + interface + ) # write interface to file export = Path("crytic-export", "interfaces") diff --git a/slither/tools/read_storage/README.md b/slither/tools/read_storage/README.md index 677b2c772c..dd7ef28658 100644 --- a/slither/tools/read_storage/README.md +++ b/slither/tools/read_storage/README.md @@ -8,20 +8,29 @@ Slither-read-storage is a tool to retrieve the storage slots and values of entir ```shell positional arguments: - contract_source (DIR) ADDRESS The deployed contract address if verified on etherscan. Prepend project directory for unverified contracts. + contract_source The deployed contract address if verified on etherscan. Prepend project directory for unverified contracts. optional arguments: - --variable-name VARIABLE_NAME The name of the variable whose value will be returned. - --rpc-url RPC_URL An endpoint for web3 requests. - --key KEY The key/ index whose value will be returned from a mapping or array. - --deep-key DEEP_KEY The key/ index whose value will be returned from a deep mapping or multidimensional array. - --struct-var STRUCT_VAR The name of the variable whose value will be returned from a struct. - --storage-address STORAGE_ADDRESS The address of the storage contract (if a proxy pattern is used). - --contract-name CONTRACT_NAME The name of the logic contract. - --json FILE Write the entire storage layout in JSON format to the specified FILE - --value Toggle used to include values in output. - --max-depth MAX_DEPTH Max depth to search in data structure. - --block BLOCK_NUMBER Block number to retrieve storage from (requires archive rpc node) + -h, --help show this help message and exit + --variable-name VARIABLE_NAME + The name of the variable whose value will be returned. + --rpc-url RPC_URL An endpoint for web3 requests. + --key KEY The key/ index whose value will be returned from a mapping or array. + --deep-key DEEP_KEY The key/ index whose value will be returned from a deep mapping or multidimensional array. + --struct-var STRUCT_VAR + The name of the variable whose value will be returned from a struct. + --storage-address STORAGE_ADDRESS + The address of the storage contract (if a proxy pattern is used). + --contract-name CONTRACT_NAME + The name of the logic contract. + --json JSON Save the result in a JSON file. + --value Toggle used to include values in output. + --table Print table view of storage layout + --silent Silence log outputs + --max-depth MAX_DEPTH + Max depth to search in data structure. + --block BLOCK The block number to read storage from. Requires an archive node to be provided as the RPC url. + --unstructured Include unstructured storage slots ``` ### Examples diff --git a/slither/utils/command_line.py b/slither/utils/command_line.py index 911609bf6d..0824725821 100644 --- a/slither/utils/command_line.py +++ b/slither/utils/command_line.py @@ -1,4 +1,5 @@ import argparse +import enum import json import os import re @@ -27,6 +28,15 @@ "list-printers", ] + +class FailOnLevel(enum.Enum): + PEDANTIC = "pedantic" + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + NONE = "none" + + # Those are the flags shared by the command line and the config file defaults_flag_in_config = { "codex": False, @@ -44,10 +54,7 @@ "exclude_low": False, "exclude_medium": False, "exclude_high": False, - "fail_pedantic": True, - "fail_low": False, - "fail_medium": False, - "fail_high": False, + "fail_on": FailOnLevel.PEDANTIC, "json": None, "sarif": None, "json-types": ",".join(DEFAULT_JSON_OUTPUT_TYPES), @@ -64,6 +71,13 @@ **DEFAULTS_FLAG_IN_CONFIG_CRYTIC_COMPILE, } +deprecated_flags = { + "fail_pedantic": True, + "fail_low": False, + "fail_medium": False, + "fail_high": False, +} + def read_config_file(args: argparse.Namespace) -> None: # No config file was provided as an argument @@ -80,6 +94,12 @@ def read_config_file(args: argparse.Namespace) -> None: with open(args.config_file, encoding="utf8") as f: config = json.load(f) for key, elem in config.items(): + if key in deprecated_flags: + logger.info( + yellow(f"{args.config_file} has a deprecated key: {key} : {elem}") + ) + migrate_config_options(args, key, elem) + continue if key not in defaults_flag_in_config: logger.info( yellow(f"{args.config_file} has an unknown key: {key} : {elem}") @@ -94,6 +114,28 @@ def read_config_file(args: argparse.Namespace) -> None: logger.error(yellow("Falling back to the default settings...")) +def migrate_config_options(args: argparse.Namespace, key: str, elem): + if key.startswith("fail_") and getattr(args, "fail_on") == defaults_flag_in_config["fail_on"]: + if key == "fail_pedantic": + pedantic_setting = elem + fail_on = FailOnLevel.PEDANTIC if pedantic_setting else FailOnLevel.NONE + setattr(args, "fail_on", fail_on) + logger.info(f"Migrating fail_pedantic: {pedantic_setting} as fail_on: {fail_on.value}") + elif key == "fail_low" and elem is True: + logger.info("Migrating fail_low: true -> fail_on: low") + setattr(args, "fail_on", FailOnLevel.LOW) + + elif key == "fail_medium" and elem is True: + logger.info("Migrating fail_medium: true -> fail_on: medium") + setattr(args, "fail_on", FailOnLevel.MEDIUM) + + elif key == "fail_high" and elem is True: + logger.info("Migrating fail_high: true -> fail_on: high") + setattr(args, "fail_on", FailOnLevel.HIGH) + else: + logger.warning(yellow(f"Key {key} was deprecated but no migration was provided")) + + def output_to_markdown( detector_classes: List[Type[AbstractDetector]], printer_classes: List[Type[AbstractPrinter]], diff --git a/tests/e2e/detectors/snapshots/detectors__detector_CacheArrayLength_0_8_17_CacheArrayLength_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_CacheArrayLength_0_8_17_CacheArrayLength_sol__0.txt new file mode 100644 index 0000000000..63d4b883c3 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_CacheArrayLength_0_8_17_CacheArrayLength_sol__0.txt @@ -0,0 +1,18 @@ +Loop condition at i < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#36) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at i_scope_22 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#166) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at j_scope_11 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#108) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at i_scope_6 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#79) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at i_scope_21 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#160) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at k_scope_9 < array2.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#98) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at k_scope_17 < array2.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#132) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at j_scope_15 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#125) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at i_scope_4 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#67) should use cached array length instead of referencing `length` member of the storage array. + diff --git a/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol b/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol new file mode 100644 index 0000000000..704d6bed9e --- /dev/null +++ b/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol @@ -0,0 +1,171 @@ +pragma solidity 0.8.17; + +contract CacheArrayLength +{ + struct S + { + uint s; + } + + S[] array; + S[] array2; + + function h() external + { + + } + + function g() internal + { + this.h(); + } + + function h_view() external view + { + + } + + function g_view() internal view + { + this.h_view(); + } + + function f() public + { + // array accessed but length doesn't change + for (uint i = 0; i < array.length; i++) // warning should appear + { + array[i] = S(0); + } + + // array.length doesn't change, but array.length not used in loop condition + for (uint i = array.length; i >= 0; i--) + { + + } + + // array.length changes in the inner loop + for (uint i = 0; i < array.length; i++) + { + for (uint j = i; j < 2 * i; j++) + array.push(S(j)); + } + + // array.length changes + for (uint i = 0; i < array.length; i++) + { + array.pop(); + } + + // array.length changes + for (uint i = 0; i < array.length; i++) + { + delete array; + } + + // array.length doesn't change despite using delete + for (uint i = 0; i < array.length; i++) // warning should appear + { + delete array[i]; + } + + // array.length changes; push used in more complex expression + for (uint i = 0; i < array.length; i++) + { + array.push() = S(i); + } + + // array.length doesn't change + for (uint i = 0; i < array.length; i++) // warning should appear + { + array2.pop(); + array2.push(); + array2.push(S(i)); + delete array2; + delete array[0]; + } + + // array.length changes; array2.length doesn't change + for (uint i = 0; i < 7; i++) + { + for (uint j = i; j < array.length; j++) + { + for (uint k = 0; k < j; k++) + { + + } + + for (uint k = 0; k < array2.length; k++) // warning should appear + { + array.pop(); + } + } + } + + // array.length doesn't change; array2.length changes + for (uint i = 0; i < 7; i++) + { + for (uint j = i; j < array.length; j++) // warning should appear + { + for (uint k = 0; k < j; k++) + { + + } + + for (uint k = 0; k < array2.length; k++) + { + array2.pop(); + } + } + } + + // none of array.length and array2.length changes + for (uint i = 0; i < 7; i++) + { + for (uint j = i; j < array.length; j++) // warning should appear + { + for (uint k = 0; k < j; k++) + { + + } + + for (uint k = 0; k < array2.length; k++) // warning should appear + { + + } + } + } + + S[] memory array3; + + // array3 not modified, but it's not a storage array + for (uint i = 0; i < array3.length; i++) + { + + } + + // array not modified, but it may potentially change in an internal function call + for (uint i = 0; i < array.length; i++) + { + g(); + } + + // array not modified, but it may potentially change in an external function call + for (uint i = 0; i < array.length; i++) + { + this.h(); + } + + // array not modified and it cannot be changed in a function call since g_view is a view function + for (uint i = 0; i < array.length; i++) // warning should appear + { + g_view(); + } + + // array not modified and it cannot be changed in a function call since h_view is a view function + for (uint i = 0; i < array.length; i++) // warning should appear + { + this.h_view(); + } + } +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol-0.8.17.zip b/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol-0.8.17.zip new file mode 100644 index 0000000000..dafdfc4316 Binary files /dev/null and b/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol-0.8.17.zip differ diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index d003e7ce00..6fc04e4e1f 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -56,7 +56,7 @@ def id_test(test_item: Test): return f"{test_item.detector.__name__}-{test_item.solc_ver}-{test_item.test_file}" -ALL_TEST_OBJECTS = [ +ALL_TESTS = [ Test( all_detectors.UninitializedFunctionPtrsConstructor, "uninitialized_function_ptr_constructor.sol", @@ -1639,6 +1639,11 @@ def id_test(test_item: Test): "LowCyclomaticComplexity.sol", "0.8.16", ), + Test( + all_detectors.CacheArrayLength, + "CacheArrayLength.sol", + "0.8.17", + ), Test( all_detectors.IncorrectUsingFor, "IncorrectUsingForTopLevel.sol", @@ -1656,7 +1661,7 @@ def id_test(test_item: Test): TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" # pylint: disable=too-many-locals -@pytest.mark.parametrize("test_item", ALL_TEST_OBJECTS, ids=id_test) +@pytest.mark.parametrize("test_item", ALL_TESTS, ids=id_test) def test_detector(test_item: Test, snapshot): test_dir_path = Path( TEST_DATA_DIR, @@ -1704,5 +1709,5 @@ def _generate_compile(test_item: Test, skip_existing=False): "To generate the zip artifacts run\n\tpython tests/e2e/tests/test_detectors.py --compile" ) elif sys.argv[1] == "--compile": - for next_test in ALL_TEST_OBJECTS: + for next_test in ALL_TESTS: _generate_compile(next_test, skip_existing=True) diff --git a/tests/e2e/solc_parsing/test_ast_parsing.py b/tests/e2e/solc_parsing/test_ast_parsing.py index b694d1044a..307e6736ff 100644 --- a/tests/e2e/solc_parsing/test_ast_parsing.py +++ b/tests/e2e/solc_parsing/test_ast_parsing.py @@ -458,6 +458,7 @@ def make_version(minor: int, patch_min: int, patch_max: int) -> List[str]: "assembly-functions.sol", ["0.6.9", "0.7.6", "0.8.16"], ), + Test("user_defined_operators-0.8.19.sol", ["0.8.19"]), ] # create the output folder if needed try: diff --git a/tests/e2e/solc_parsing/test_data/compile/user_defined_operators-0.8.19.sol-0.8.19-compact.zip b/tests/e2e/solc_parsing/test_data/compile/user_defined_operators-0.8.19.sol-0.8.19-compact.zip new file mode 100644 index 0000000000..7159a14861 Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/user_defined_operators-0.8.19.sol-0.8.19-compact.zip differ diff --git a/tests/e2e/solc_parsing/test_data/expected/user_defined_operators-0.8.19.sol-0.8.19-compact.json b/tests/e2e/solc_parsing/test_data/expected/user_defined_operators-0.8.19.sol-0.8.19-compact.json new file mode 100644 index 0000000000..bee7819a60 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/user_defined_operators-0.8.19.sol-0.8.19-compact.json @@ -0,0 +1,13 @@ +{ + "Lib": { + "f(Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n}\n" + }, + "T": { + "add_function_call(Int,Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: RETURN 2\n\"];\n}\n", + "add_op(Int,Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "lib_call(Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "neg_usertype(Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: RETURN 2\n\"];\n}\n", + "neg_int(int256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "eq_op(Int,Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/user_defined_operators-0.8.19.sol b/tests/e2e/solc_parsing/test_data/user_defined_operators-0.8.19.sol new file mode 100644 index 0000000000..e4df845fb3 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/user_defined_operators-0.8.19.sol @@ -0,0 +1,48 @@ +pragma solidity ^0.8.19; + +type Int is int; +using {add as +, eq as ==, add, neg as -, Lib.f} for Int global; + +function add(Int a, Int b) pure returns (Int) { + return Int.wrap(Int.unwrap(a) + Int.unwrap(b)); +} + +function eq(Int a, Int b) pure returns (bool) { + return true; +} + +function neg(Int a) pure returns (Int) { + return a; +} + +library Lib { + function f(Int r) internal {} +} + +contract T { + function add_function_call(Int b, Int c) public returns(Int) { + Int res = add(b,c); + return res; + } + + function add_op(Int b, Int c) public returns(Int) { + return b + c; + } + + function lib_call(Int b) public { + return b.f(); + } + + function neg_usertype(Int b) public returns(Int) { + Int res = -b; + return res; + } + + function neg_int(int b) public returns(int) { + return -b; + } + + function eq_op(Int b, Int c) public returns(bool) { + return b == c; + } +}