diff --git a/.github/workflows/black_auto.yml b/.github/workflows/black_auto.yml index 4c0dca0c63..ad1ad2ea4b 100644 --- a/.github/workflows/black_auto.yml +++ b/.github/workflows/black_auto.yml @@ -30,12 +30,14 @@ jobs: with: python-version: 3.8 - - name: Install Python dependencies - run: pip install .[dev] + - name: Run black + uses: psf/black@stable + with: + options: "" + summary: false + version: "~= 22.3.0" - - name: Run linters - uses: wearerequired/lint-action@v2 + - name: Annotate diff changes using reviewdog + uses: reviewdog/action-suggester@v1 with: - auto_fix: true - black: true - black_auto_fix: true + tool_name: blackfmt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d2ba83d0d..0bac900ab2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,26 +25,9 @@ jobs: strategy: fail-fast: false matrix: - os: ["ubuntu-latest", "windows-2022"] + os: ${{ (github.event_name == 'pull_request' && fromJSON('["ubuntu-latest"]')) || fromJSON('["ubuntu-latest", "windows-2022"]') }} python: ${{ (github.event_name == 'pull_request' && fromJSON('["3.8", "3.12"]')) || fromJSON('["3.8", "3.9", "3.10", "3.11", "3.12"]') }} - type: ["cli", - "dapp", - "data_dependency", - "path_filtering", - # "embark", - "erc", - # "etherlime", - "etherscan", - "find_paths", - "flat", - "interface", - "kspec", - "printers", - # "prop" - "simil", - "slither_config", - "truffle", - "upgradability"] + type: ${{ (github.event_name == 'pull_request' && fromJSON('["data_dependency", "path_filtering","erc","find_paths","flat","interface", "printers","slither_config","upgradability"]')) || fromJSON('["data_dependency", "path_filtering","erc","find_paths","flat","interface", "printers","slither_config","upgradability", "cli", "dapp", "etherscan", "kspec", "simil", "truffle"]') }} exclude: # Requires nix - os: windows-2022 @@ -67,7 +50,7 @@ jobs: - name: Set up nix if: matrix.type == 'dapp' - uses: cachix/install-nix-action@V27 + uses: cachix/install-nix-action@v30 - name: Set up cachix if: matrix.type == 'dapp' diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 0a0f04f2bc..c196d365e1 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -44,7 +44,7 @@ jobs: path: dist/ - name: publish - uses: pypa/gh-action-pypi-publish@v1.9.0 + uses: pypa/gh-action-pypi-publish@v1.12.3 - name: sign uses: sigstore/gh-action-sigstore-python@v3.0.0 diff --git a/CODEOWNERS b/CODEOWNERS index c92f0d79df..496da0c304 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,5 +1,4 @@ -* @montyly @0xalpharush @smonicas -/slither/tools/read_storage/ @0xalpharush +* @montyly @smonicas /slither/tools/doctor/ @elopez /slither/slithir/ @montyly /slither/analyses/ @montyly diff --git a/Makefile b/Makefile index a94c3eeb87..17697385ed 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ TEST_MODULE := tests ALL_PY_SRCS := $(shell find $(PY_MODULE) -name '*.py') \ $(shell find test -name '*.py') -# Optionally overriden by the user, if they're using a virtual environment manager. +# Optionally overridden by the user, if they're using a virtual environment manager. VENV ?= env # On Windows, venv scripts/shims are under `Scripts` instead of `bin`. diff --git a/README.md b/README.md index 660f4f8e84..ebbc6faab6 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ * 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 +* Intermediate representation ([SlithIR](https://github.com/crytic/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) @@ -76,6 +76,12 @@ If you're **not** going to use one of the [supported compilation frameworks](htt python3 -m pip install slither-analyzer ``` +#### How to upgrade + +```console +python3 -m pip install --upgrade slither-analyzer +``` + ### Using Git ```bash @@ -83,11 +89,11 @@ git clone https://github.com/crytic/slither.git && cd slither python3 -m pip install . ``` -We recommend using a Python virtual environment, as detailed in the [Developer Installation Instructions](https://github.com/trailofbits/slither/wiki/Developer-installation), if you prefer to install Slither via git. +We recommend using a Python virtual environment, as detailed in the [Developer Installation Instructions](https://github.com/crytic/slither/wiki/Developer-installation), if you prefer to install Slither via git. ### Using Docker -Use the [`eth-security-toolbox`](https://github.com/trailofbits/eth-security-toolbox/) docker image. It includes all of our security tools and every major version of Solidity in a single image. `/home/share` will be mounted to `/share` in the container. +Use the [`eth-security-toolbox`](https://github.com/crytic/eth-security-toolbox/) docker image. It includes all of our security tools and every major version of Solidity in a single image. `/home/share` will be mounted to `/share` in the container. ```bash docker pull trailofbits/eth-security-toolbox @@ -152,63 +158,69 @@ Num | Detector | What it Detects | Impact | Confidence 34 | `incorrect-equality` | [Dangerous strict equalities](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-strict-equalities) | Medium | High 35 | `locked-ether` | [Contracts that lock ether](https://github.com/crytic/slither/wiki/Detector-Documentation#contracts-that-lock-ether) | Medium | High 36 | `mapping-deletion` | [Deletion on mapping containing a structure](https://github.com/crytic/slither/wiki/Detector-Documentation#deletion-on-mapping-containing-a-structure) | Medium | High -37 | `shadowing-abstract` | [State variables shadowing from abstract contracts](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variable-shadowing-from-abstract-contracts) | Medium | High -38 | `tautological-compare` | [Comparing a variable to itself always returns true or false, depending on comparison](https://github.com/crytic/slither/wiki/Detector-Documentation#tautological-compare) | Medium | High -39 | `tautology` | [Tautology or contradiction](https://github.com/crytic/slither/wiki/Detector-Documentation#tautology-or-contradiction) | Medium | High -40 | `write-after-write` | [Unused write](https://github.com/crytic/slither/wiki/Detector-Documentation#write-after-write) | Medium | High -41 | `boolean-cst` | [Misuse of Boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#misuse-of-a-boolean-constant) | Medium | Medium -42 | `constant-function-asm` | [Constant functions using assembly code](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-using-assembly-code) | Medium | Medium -43 | `constant-function-state` | [Constant functions changing the state](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state) | Medium | Medium -44 | `divide-before-multiply` | [Imprecise arithmetic operations order](https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply) | Medium | Medium -45 | `out-of-order-retryable` | [Out-of-order retryable transactions](https://github.com/crytic/slither/wiki/Detector-Documentation#out-of-order-retryable-transactions) | Medium | Medium -46 | `reentrancy-no-eth` | [Reentrancy vulnerabilities (no theft of ethers)](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-1) | Medium | Medium -47 | `reused-constructor` | [Reused base constructor](https://github.com/crytic/slither/wiki/Detector-Documentation#reused-base-constructors) | Medium | Medium -48 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-usage-of-txorigin) | Medium | Medium -49 | `unchecked-lowlevel` | [Unchecked low-level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level-calls) | Medium | Medium -50 | `unchecked-send` | [Unchecked send](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-send) | Medium | Medium -51 | `uninitialized-local` | [Uninitialized local variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables) | Medium | Medium -52 | `unused-return` | [Unused return values](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return) | Medium | Medium -53 | `incorrect-modifier` | [Modifiers that can return the default value](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-modifier) | Low | High -54 | `shadowing-builtin` | [Built-in symbol shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#builtin-symbol-shadowing) | Low | High -55 | `shadowing-local` | [Local variables shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#local-variable-shadowing) | Low | High -56 | `uninitialized-fptr-cst` | [Uninitialized function pointer calls in constructors](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-function-pointers-in-constructors) | Low | High -57 | `variable-scope` | [Local variables used prior their declaration](https://github.com/crytic/slither/wiki/Detector-Documentation#pre-declaration-usage-of-local-variables) | Low | High -58 | `void-cst` | [Constructor called not implemented](https://github.com/crytic/slither/wiki/Detector-Documentation#void-constructor) | Low | High -59 | `calls-loop` | [Multiple calls in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/#calls-inside-a-loop) | Low | Medium -60 | `events-access` | [Missing Events Access Control](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-events-access-control) | Low | Medium -61 | `events-maths` | [Missing Events Arithmetic](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-events-arithmetic) | Low | Medium -62 | `incorrect-unary` | [Dangerous unary expressions](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-unary-expressions) | Low | Medium -63 | `missing-zero-check` | [Missing Zero Address Validation](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-zero-address-validation) | Low | Medium -64 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2) | Low | Medium -65 | `reentrancy-events` | [Reentrancy vulnerabilities leading to out-of-order Events](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-3) | Low | Medium -66 | `return-bomb` | [A low level callee may consume all callers gas unexpectedly.](https://github.com/crytic/slither/wiki/Detector-Documentation#return-bomb) | Low | Medium -67 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp) | Low | Medium -68 | `assembly` | [Assembly usage](https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage) | Informational | High -69 | `assert-state-change` | [Assert state change](https://github.com/crytic/slither/wiki/Detector-Documentation#assert-state-change) | Informational | High -70 | `boolean-equal` | [Comparison to boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#boolean-equality) | Informational | High -71 | `cyclomatic-complexity` | [Detects functions with high (> 11) cyclomatic complexity](https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity) | Informational | High -72 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High -73 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High -74 | `function-init-state` | [Function initializing state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#function-initializing-state) | Informational | High -75 | `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 -76 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High -77 | `missing-inheritance` | [Missing inheritance](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-inheritance) | Informational | High -78 | `naming-convention` | [Conformity to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High -79 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High -80 | `redundant-statements` | [Redundant statements](https://github.com/crytic/slither/wiki/Detector-Documentation#redundant-statements) | Informational | High -81 | `solc-version` | [Incorrect Solidity version](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity) | Informational | High -82 | `unimplemented-functions` | [Unimplemented functions](https://github.com/crytic/slither/wiki/Detector-Documentation#unimplemented-functions) | Informational | High -83 | `unused-import` | [Detects unused imports](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-imports) | Informational | High -84 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variable) | Informational | High -85 | `costly-loop` | [Costly operations in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation#costly-operations-inside-a-loop) | Informational | Medium -86 | `dead-code` | [Functions that are not used](https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code) | Informational | Medium -87 | `reentrancy-unlimited-gas` | [Reentrancy vulnerabilities through send and transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-4) | Informational | Medium -88 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium -89 | `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 -90 | `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 -91 | `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 -92 | `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 -93 | `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 +37 | `pyth-deprecated-functions` | [Detect Pyth deprecated functions](https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-deprecated-functions) | Medium | High +38 | `pyth-unchecked-confidence` | [Detect when the confidence level of a Pyth price is not checked](https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-unchecked-confidence) | Medium | High +39 | `pyth-unchecked-publishtime` | [Detect when the publishTime of a Pyth price is not checked](https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-unchecked-publishtime) | Medium | High +40 | `shadowing-abstract` | [State variables shadowing from abstract contracts](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variable-shadowing-from-abstract-contracts) | Medium | High +41 | `tautological-compare` | [Comparing a variable to itself always returns true or false, depending on comparison](https://github.com/crytic/slither/wiki/Detector-Documentation#tautological-compare) | Medium | High +42 | `tautology` | [Tautology or contradiction](https://github.com/crytic/slither/wiki/Detector-Documentation#tautology-or-contradiction) | Medium | High +43 | `write-after-write` | [Unused write](https://github.com/crytic/slither/wiki/Detector-Documentation#write-after-write) | Medium | High +44 | `boolean-cst` | [Misuse of Boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#misuse-of-a-boolean-constant) | Medium | Medium +45 | `chronicle-unchecked-price` | [Detect when Chronicle price is not checked.](https://github.com/crytic/slither/wiki/Detector-Documentation#chronicle-unchecked-price) | Medium | Medium +46 | `constant-function-asm` | [Constant functions using assembly code](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-using-assembly-code) | Medium | Medium +47 | `constant-function-state` | [Constant functions changing the state](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state) | Medium | Medium +48 | `divide-before-multiply` | [Imprecise arithmetic operations order](https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply) | Medium | Medium +49 | `gelato-unprotected-randomness` | [Call to `_requestRandomness` within an unprotected function](https://github.com/crytic/slither/wiki/Detector-Documentation#gelato-unprotected-randomness) | Medium | Medium +50 | `out-of-order-retryable` | [Out-of-order retryable transactions](https://github.com/crytic/slither/wiki/Detector-Documentation#out-of-order-retryable-transactions) | Medium | Medium +51 | `reentrancy-no-eth` | [Reentrancy vulnerabilities (no theft of ethers)](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-1) | Medium | Medium +52 | `reused-constructor` | [Reused base constructor](https://github.com/crytic/slither/wiki/Detector-Documentation#reused-base-constructors) | Medium | Medium +53 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-usage-of-txorigin) | Medium | Medium +54 | `unchecked-lowlevel` | [Unchecked low-level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level-calls) | Medium | Medium +55 | `unchecked-send` | [Unchecked send](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-send) | Medium | Medium +56 | `uninitialized-local` | [Uninitialized local variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables) | Medium | Medium +57 | `unused-return` | [Unused return values](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return) | Medium | Medium +58 | `chainlink-feed-registry` | [Detect when chainlink feed registry is used](https://github.com/crytic/slither/wiki/Detector-Documentation#chainlink-feed-registry) | Low | High +59 | `incorrect-modifier` | [Modifiers that can return the default value](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-modifier) | Low | High +60 | `optimism-deprecation` | [Detect when deprecated Optimism predeploy or function is used.](https://github.com/crytic/slither/wiki/Detector-Documentation#optimism-deprecation) | Low | High +61 | `shadowing-builtin` | [Built-in symbol shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#builtin-symbol-shadowing) | Low | High +62 | `shadowing-local` | [Local variables shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#local-variable-shadowing) | Low | High +63 | `uninitialized-fptr-cst` | [Uninitialized function pointer calls in constructors](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-function-pointers-in-constructors) | Low | High +64 | `variable-scope` | [Local variables used prior their declaration](https://github.com/crytic/slither/wiki/Detector-Documentation#pre-declaration-usage-of-local-variables) | Low | High +65 | `void-cst` | [Constructor called not implemented](https://github.com/crytic/slither/wiki/Detector-Documentation#void-constructor) | Low | High +66 | `calls-loop` | [Multiple calls in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/#calls-inside-a-loop) | Low | Medium +67 | `events-access` | [Missing Events Access Control](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-events-access-control) | Low | Medium +68 | `events-maths` | [Missing Events Arithmetic](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-events-arithmetic) | Low | Medium +69 | `incorrect-unary` | [Dangerous unary expressions](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-unary-expressions) | Low | Medium +70 | `missing-zero-check` | [Missing Zero Address Validation](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-zero-address-validation) | Low | Medium +71 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2) | Low | Medium +72 | `reentrancy-events` | [Reentrancy vulnerabilities leading to out-of-order Events](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-3) | Low | Medium +73 | `return-bomb` | [A low level callee may consume all callers gas unexpectedly.](https://github.com/crytic/slither/wiki/Detector-Documentation#return-bomb) | Low | Medium +74 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp) | Low | Medium +75 | `assembly` | [Assembly usage](https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage) | Informational | High +76 | `assert-state-change` | [Assert state change](https://github.com/crytic/slither/wiki/Detector-Documentation#assert-state-change) | Informational | High +77 | `boolean-equal` | [Comparison to boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#boolean-equality) | Informational | High +78 | `cyclomatic-complexity` | [Detects functions with high (> 11) cyclomatic complexity](https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity) | Informational | High +79 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High +80 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High +81 | `function-init-state` | [Function initializing state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#function-initializing-state) | Informational | High +82 | `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 +83 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High +84 | `missing-inheritance` | [Missing inheritance](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-inheritance) | Informational | High +85 | `naming-convention` | [Conformity to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High +86 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High +87 | `redundant-statements` | [Redundant statements](https://github.com/crytic/slither/wiki/Detector-Documentation#redundant-statements) | Informational | High +88 | `solc-version` | [Incorrect Solidity version](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity) | Informational | High +89 | `unimplemented-functions` | [Unimplemented functions](https://github.com/crytic/slither/wiki/Detector-Documentation#unimplemented-functions) | Informational | High +90 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variable) | Informational | High +91 | `costly-loop` | [Costly operations in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation#costly-operations-inside-a-loop) | Informational | Medium +92 | `dead-code` | [Functions that are not used](https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code) | Informational | Medium +93 | `reentrancy-unlimited-gas` | [Reentrancy vulnerabilities through send and transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-4) | Informational | Medium +94 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium +95 | `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 +96 | `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 +97 | `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 +98 | `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 +99 | `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 @@ -220,18 +232,18 @@ For more information, see ### 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) +* `human-summary`: [Print a human-readable summary of the contracts](https://github.com/crytic/slither/wiki/Printer-documentation#human-summary) +* `inheritance-graph`: [Export the inheritance graph of each contract to a dot file](https://github.com/crytic/slither/wiki/Printer-documentation#inheritance-graph) +* `contract-summary`: [Print a summary of the contracts](https://github.com/crytic/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/crytic/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) +* `call-graph`: [Export the call-graph of the contracts to a dot file](https://github.com/crytic/slither/wiki/Printer-documentation#call-graph) +* `cfg`: [Export the CFG of each functions](https://github.com/crytic/slither/wiki/Printer-documentation#cfg) +* `function-summary`: [Print a summary of the functions](https://github.com/crytic/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) -* `not-pausable`: [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/crytic/slither/wiki/Printer-documentation#when-not-paused). To run a printer, use `--print` and a comma-separated list of printers. @@ -259,13 +271,13 @@ Documentation on Slither's internals is available [here](https://crytic.github.i 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/crytic/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/crytic/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 [SlithIR documentation](https://github.com/trailofbits/slither/wiki/SlithIR) describes the SlithIR intermediate representation. +* The [SlithIR documentation](https://github.com/crytic/slither/wiki/SlithIR) describes the SlithIR intermediate representation. ## FAQ @@ -297,7 +309,7 @@ Slither is licensed and distributed under the AGPLv3 license. [Contact us](mailt 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 | - -[MPro: Combining Static and Symbolic Analysis forScalable Testing of Smart Contract](https://arxiv.org/pdf/1911.00570.pdf) | Leverage data dependency through Slither | William Zhang, Sebastian Banescu, Leodardo Pasos, Steven Stewart, Vijay Ganesh | ISSRE 2019 | [MPro](https://github.com/QuanZhang-William/M-Pro) +[MPro: Combining Static and Symbolic Analysis for Scalable Testing of Smart Contract](https://arxiv.org/pdf/1911.00570.pdf) | Leverage data dependency through Slither | William Zhang, Sebastian Banescu, Leodardo Pasos, Steven Stewart, Vijay Ganesh | ISSRE 2019 | [MPro](https://github.com/QuanZhang-William/M-Pro) [ETHPLOIT: From Fuzzing to Efficient Exploit Generation against Smart Contracts](https://wcventure.github.io/FuzzingPaper/Paper/SANER20_ETHPLOIT.pdf) | Leverage data dependency through Slither | Qingzhao Zhang, Yizhuo Wang, Juanru Li, Siqi Ma | SANER 20 | - [Verification of Ethereum Smart Contracts: A Model Checking Approach](http://www.ijmlc.org/vol10/977-AM0059.pdf) | Symbolic execution built on top of Slither’s CFG | Tam Bang, Hoang H Nguyen, Dung Nguyen, Toan Trieu, Tho Quan | IJMLC 20 | - [Smart Contract Repair](https://arxiv.org/pdf/1912.05823.pdf) | Rely on Slither’s vulnerabilities detectors | Xiao Liang Yu, Omar Al-Bataineh, David Lo, Abhik Roychoudhury | TOSEM 20 | [SCRepair](https://github.com/xiaoly8/SCRepair/) diff --git a/scripts/ci_test_printers.sh b/scripts/ci_test_printers.sh index 3306c134e2..d811c19f91 100755 --- a/scripts/ci_test_printers.sh +++ b/scripts/ci_test_printers.sh @@ -5,7 +5,7 @@ cd tests/e2e/solc_parsing/test_data/compile/ || exit # Do not test the evm printer,as it needs a refactoring -ALL_PRINTERS="cfg,constructor-calls,contract-summary,data-dependency,echidna,function-id,function-summary,modifiers,call-graph,halstead,human-summary,inheritance,inheritance-graph,loc,martin,slithir,slithir-ssa,vars-and-auth,require,variable-order,declaration,ck" +ALL_PRINTERS="cfg,constructor-calls,contract-summary,data-dependency,echidna,function-id,function-summary,modifiers,call-graph,halstead,human-summary,inheritance,inheritance-graph,loc,martin,slithir,slithir-ssa,vars-and-auth,require,variable-order,declaration,ck,cheatcode" # Only test 0.5.17 to limit test time for file in *0.5.17-compact.zip; do diff --git a/setup.py b/setup.py index ef9d81f203..c76516f69a 100644 --- a/setup.py +++ b/setup.py @@ -8,14 +8,14 @@ description="Slither is a Solidity and Vyper static analysis framework written in Python 3.", url="https://github.com/crytic/slither", author="Trail of Bits", - version="0.10.4", + version="0.11.0", packages=find_packages(), python_requires=">=3.8", install_requires=[ "packaging", "prettytable>=3.10.2", "pycryptodome>=3.4.6", - "crytic-compile>=0.3.7,<0.4.0", + "crytic-compile>=0.3.8,<0.4.0", # "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile", "web3>=6.20.2, <7", "eth-abi>=4.0.0", diff --git a/slither/analyses/write/are_variables_written.py b/slither/analyses/write/are_variables_written.py index 2f8f83063d..2524897d27 100644 --- a/slither/analyses/write/are_variables_written.py +++ b/slither/analyses/write/are_variables_written.py @@ -86,7 +86,7 @@ def _visit( lvalue = refs_lvalues ret: List[Variable] = [] - if not node.sons and node.type not in [NodeType.THROW, NodeType.RETURN]: + if not node.sons and node.type is not NodeType.THROW: ret += [v for v in variables_to_write if v not in variables_written] # Explore sons if diff --git a/slither/core/cfg/node.py b/slither/core/cfg/node.py index 87d0e16a2e..fc178db4a7 100644 --- a/slither/core/cfg/node.py +++ b/slither/core/cfg/node.py @@ -46,12 +46,6 @@ if TYPE_CHECKING: from slither.slithir.variables.variable import SlithIRVariable from slither.core.compilation_unit import SlitherCompilationUnit - from slither.utils.type_helpers import ( - InternalCallType, - HighLevelCallType, - LibraryCallType, - LowLevelCallType, - ) from slither.core.cfg.scope import Scope from slither.core.scope.scope import FileScope @@ -153,11 +147,11 @@ def __init__( self._ssa_vars_written: List["SlithIRVariable"] = [] self._ssa_vars_read: List["SlithIRVariable"] = [] - self._internal_calls: List[Union["Function", "SolidityFunction"]] = [] - self._solidity_calls: List[SolidityFunction] = [] - self._high_level_calls: List["HighLevelCallType"] = [] # contains library calls - self._library_calls: List["LibraryCallType"] = [] - self._low_level_calls: List["LowLevelCallType"] = [] + self._internal_calls: List[InternalCall] = [] # contains solidity calls + self._solidity_calls: List[SolidityCall] = [] + self._high_level_calls: List[Tuple[Contract, HighLevelCall]] = [] # contains library calls + self._library_calls: List[LibraryCall] = [] + self._low_level_calls: List[LowLevelCall] = [] self._external_calls_as_expressions: List[Expression] = [] self._internal_calls_as_expressions: List[Expression] = [] self._irs: List[Operation] = [] @@ -226,8 +220,9 @@ def type(self, new_type: NodeType) -> None: @property def will_return(self) -> bool: if not self.sons and self.type != NodeType.THROW: - if SolidityFunction("revert()") not in self.solidity_calls: - if SolidityFunction("revert(string)") not in self.solidity_calls: + solidity_calls = [ir.function for ir in self.solidity_calls] + if SolidityFunction("revert()") not in solidity_calls: + if SolidityFunction("revert(string)") not in solidity_calls: return True return False @@ -373,44 +368,38 @@ def variables_written_as_expression(self, exprs: List[Expression]) -> None: ################################################################################### @property - def internal_calls(self) -> List["InternalCallType"]: + def internal_calls(self) -> List[InternalCall]: """ - list(Function or SolidityFunction): List of internal/soldiity function calls + list(InternalCall): List of IR operations with internal/solidity function calls """ return list(self._internal_calls) @property - def solidity_calls(self) -> List[SolidityFunction]: + def solidity_calls(self) -> List[SolidityCall]: """ - list(SolidityFunction): List of Soldity calls + list(SolidityCall): List of IR operations with solidity calls """ return list(self._solidity_calls) @property - def high_level_calls(self) -> List["HighLevelCallType"]: + def high_level_calls(self) -> List[HighLevelCall]: """ - list((Contract, Function|Variable)): - List of high level calls (external calls). - A variable is called in case of call to a public state variable + list(HighLevelCall): List of IR operations with high level calls (external calls). Include library calls """ return list(self._high_level_calls) @property - def library_calls(self) -> List["LibraryCallType"]: + def library_calls(self) -> List[LibraryCall]: """ - list((Contract, Function)): - Include library calls + list(LibraryCall): List of IR operations with library calls. """ return list(self._library_calls) @property - def low_level_calls(self) -> List["LowLevelCallType"]: + def low_level_calls(self) -> List[LowLevelCall]: """ - list((Variable|SolidityVariable, str)): List of low_level call - A low level call is defined by - - the variable called - - the name of the function (call/delegatecall/codecall) + list(LowLevelCall): List of IR operations with low_level call """ return list(self._low_level_calls) @@ -529,8 +518,9 @@ def contains_require_or_assert(self) -> bool: bool: True if the node has a require or assert call """ return any( - c.name in ["require(bool)", "require(bool,string)", "assert(bool)"] - for c in self.internal_calls + ir.function.name + in ["require(bool)", "require(bool,string)", "require(bool,error)", "assert(bool)"] + for ir in self.internal_calls ) def contains_if(self, include_loop: bool = True) -> bool: @@ -894,11 +884,11 @@ def _find_read_write_call(self) -> None: # pylint: disable=too-many-statements self._vars_written.append(var) if isinstance(ir, InternalCall): - self._internal_calls.append(ir.function) + self._internal_calls.append(ir) if isinstance(ir, SolidityCall): # TODO: consider removing dependancy of solidity_call to internal_call - self._solidity_calls.append(ir.function) - self._internal_calls.append(ir.function) + self._solidity_calls.append(ir) + self._internal_calls.append(ir) if ( isinstance(ir, SolidityCall) and ir.function == SolidityFunction("sstore(uint256,uint256)") @@ -916,22 +906,22 @@ def _find_read_write_call(self) -> None: # pylint: disable=too-many-statements self._vars_read.append(ir.arguments[0]) if isinstance(ir, LowLevelCall): assert isinstance(ir.destination, (Variable, SolidityVariable)) - self._low_level_calls.append((ir.destination, str(ir.function_name.value))) + self._low_level_calls.append(ir) elif isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall): # Todo investigate this if condition # It does seem right to compare against a contract # This might need a refactoring if isinstance(ir.destination.type, Contract): - self._high_level_calls.append((ir.destination.type, ir.function)) + self._high_level_calls.append((ir.destination.type, ir)) elif ir.destination == SolidityVariable("this"): func = self.function # Can't use this in a top level function assert isinstance(func, FunctionContract) - self._high_level_calls.append((func.contract, ir.function)) + self._high_level_calls.append((func.contract, ir)) else: try: # Todo this part needs more tests and documentation - self._high_level_calls.append((ir.destination.type.type, ir.function)) + self._high_level_calls.append((ir.destination.type.type, ir)) except AttributeError as error: # pylint: disable=raise-missing-from raise SlitherException( @@ -940,8 +930,8 @@ def _find_read_write_call(self) -> None: # pylint: disable=too-many-statements elif isinstance(ir, LibraryCall): assert isinstance(ir.destination, Contract) assert isinstance(ir.function, Function) - self._high_level_calls.append((ir.destination, ir.function)) - self._library_calls.append((ir.destination, ir.function)) + self._high_level_calls.append((ir.destination, ir)) + self._library_calls.append(ir) self._vars_read = list(set(self._vars_read)) self._state_vars_read = [v for v in self._vars_read if isinstance(v, StateVariable)] diff --git a/slither/core/compilation_unit.py b/slither/core/compilation_unit.py index df652dab0c..f4bd07e556 100644 --- a/slither/core/compilation_unit.py +++ b/slither/core/compilation_unit.py @@ -73,7 +73,8 @@ def __init__(self, core: "SlitherCore", crytic_compilation_unit: CompilationUnit # Memoize self._all_state_variables: Optional[Set[StateVariable]] = None - self._storage_layouts: Dict[str, Dict[str, Tuple[int, int]]] = {} + self._persistent_storage_layouts: Dict[str, Dict[str, Tuple[int, int]]] = {} + self._transient_storage_layouts: Dict[str, Dict[str, Tuple[int, int]]] = {} self._contract_with_missing_inheritance: Set[Contract] = set() @@ -297,33 +298,52 @@ def get_scope(self, filename_str: str) -> FileScope: def compute_storage_layout(self) -> None: assert self.is_solidity + for contract in self.contracts_derived: - self._storage_layouts[contract.name] = {} - - slot = 0 - offset = 0 - for var in contract.stored_state_variables_ordered: - assert var.type - size, new_slot = var.type.storage_size - - if new_slot: - if offset > 0: - slot += 1 - offset = 0 - elif size + offset > 32: + self._compute_storage_layout(contract.name, contract.storage_variables_ordered, False) + self._compute_storage_layout(contract.name, contract.transient_variables_ordered, True) + + def _compute_storage_layout( + self, contract_name: str, state_variables_ordered: List[StateVariable], is_transient: bool + ): + if is_transient: + self._transient_storage_layouts[contract_name] = {} + else: + self._persistent_storage_layouts[contract_name] = {} + + slot = 0 + offset = 0 + for var in state_variables_ordered: + assert var.type + size, new_slot = var.type.storage_size + + if new_slot: + if offset > 0: slot += 1 offset = 0 + elif size + offset > 32: + slot += 1 + offset = 0 - self._storage_layouts[contract.name][var.canonical_name] = ( + if is_transient: + self._transient_storage_layouts[contract_name][var.canonical_name] = ( slot, offset, ) - if new_slot: - slot += math.ceil(size / 32) - else: - offset += size + else: + self._persistent_storage_layouts[contract_name][var.canonical_name] = ( + slot, + offset, + ) + + if new_slot: + slot += math.ceil(size / 32) + else: + offset += size def storage_layout_of(self, contract: Contract, var: StateVariable) -> Tuple[int, int]: - return self._storage_layouts[contract.name][var.canonical_name] + if var.is_stored: + return self._persistent_storage_layouts[contract.name][var.canonical_name] + return self._transient_storage_layouts[contract.name][var.canonical_name] # endregion diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 3f97a33ed2..8dccc007f9 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -29,7 +29,6 @@ # pylint: disable=too-many-lines,too-many-instance-attributes,import-outside-toplevel,too-many-nested-blocks if TYPE_CHECKING: - from slither.utils.type_helpers import LibraryCallType, HighLevelCallType, InternalCallType from slither.core.declarations import ( Enum, EventContract, @@ -39,6 +38,7 @@ FunctionContract, CustomErrorContract, ) + from slither.slithir.operations import HighLevelCall, LibraryCall from slither.slithir.variables.variable import SlithIRVariable from slither.core.variables import Variable, StateVariable from slither.core.compilation_unit import SlitherCompilationUnit @@ -106,7 +106,7 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit", scope: "FileScope self._is_incorrectly_parsed: bool = False self._available_functions_as_dict: Optional[Dict[str, "Function"]] = None - self._all_functions_called: Optional[List["InternalCallType"]] = None + self._all_functions_called: Optional[List["Function"]] = None self.compilation_unit: "SlitherCompilationUnit" = compilation_unit self.file_scope: "FileScope" = scope @@ -440,55 +440,43 @@ def variables_as_dict(self) -> Dict[str, "StateVariable"]: def state_variables(self) -> List["StateVariable"]: """ Returns all the accessible variables (do not include private variable from inherited contract). - Use state_variables_ordered for all the variables following the storage order + Use stored_state_variables_ordered for all the storage variables following the storage order + Use transient_state_variables_ordered for all the transient variables following the storage order list(StateVariable): List of the state variables. """ return list(self._variables.values()) @property - def stored_state_variables(self) -> List["StateVariable"]: + def state_variables_entry_points(self) -> List["StateVariable"]: """ - Returns state variables with storage locations, excluding private variables from inherited contracts. - Use stored_state_variables_ordered to access variables with storage locations in their declaration order. - - This implementation filters out state variables if they are constant or immutable. It will be - updated to accommodate any new non-storage keywords that might replace 'constant' and 'immutable' in the future. - - Returns: - List[StateVariable]: A list of state variables with storage locations. + list(StateVariable): List of the state variables that are public. """ - return [variable for variable in self.state_variables if variable.is_stored] + return [var for var in self._variables.values() if var.visibility == "public"] @property - def stored_state_variables_ordered(self) -> List["StateVariable"]: + def state_variables_ordered(self) -> List["StateVariable"]: """ - list(StateVariable): List of the state variables with storage locations by order of declaration. - - This implementation filters out state variables if they are constant or immutable. It will be - updated to accommodate any new non-storage keywords that might replace 'constant' and 'immutable' in the future. - - Returns: - List[StateVariable]: A list of state variables with storage locations ordered by declaration. + list(StateVariable): List of the state variables by order of declaration. """ - return [variable for variable in self.state_variables_ordered if variable.is_stored] + return self._variables_ordered + + def add_state_variables_ordered(self, new_vars: List["StateVariable"]) -> None: + self._variables_ordered += new_vars @property - def state_variables_entry_points(self) -> List["StateVariable"]: + def storage_variables_ordered(self) -> List["StateVariable"]: """ - list(StateVariable): List of the state variables that are public. + list(StateVariable): List of the state variables in storage location by order of declaration. """ - return [var for var in self._variables.values() if var.visibility == "public"] + return [v for v in self._variables_ordered if v.is_stored] @property - def state_variables_ordered(self) -> List["StateVariable"]: + def transient_variables_ordered(self) -> List["StateVariable"]: """ - list(StateVariable): List of the state variables by order of declaration. + list(StateVariable): List of the state variables in transient location by order of declaration. """ - return list(self._variables_ordered) - - def add_variables_ordered(self, new_vars: List["StateVariable"]) -> None: - self._variables_ordered += new_vars + return [v for v in self._variables_ordered if v.is_transient] @property def state_variables_inherited(self) -> List["StateVariable"]: @@ -1023,15 +1011,21 @@ def get_functions_overridden_by(self, function: "Function") -> List["Function"]: ################################################################################### @property - def all_functions_called(self) -> List["InternalCallType"]: + def all_functions_called(self) -> List["Function"]: """ list(Function): List of functions reachable from the contract Includes super, and private/internal functions not shadowed """ + from slither.slithir.operations import Operation + if self._all_functions_called is None: all_functions = [f for f in self.functions + self.modifiers if not f.is_shadowed] # type: ignore all_callss = [f.all_internal_calls() for f in all_functions] + [list(all_functions)] - all_calls = [item for sublist in all_callss for item in sublist] + all_calls = [ + item.function if isinstance(item, Operation) else item + for sublist in all_callss + for item in sublist + ] all_calls = list(set(all_calls)) all_constructors = [c.constructor for c in self.inheritance if c.constructor] @@ -1069,18 +1063,18 @@ def all_state_variables_read(self) -> List["StateVariable"]: return list(set(all_state_variables_read)) @property - def all_library_calls(self) -> List["LibraryCallType"]: + def all_library_calls(self) -> List["LibraryCall"]: """ - list((Contract, Function): List all of the libraries func called + list(LibraryCall): List all of the libraries func called """ all_high_level_callss = [f.all_library_calls() for f in self.functions + self.modifiers] # type: ignore all_high_level_calls = [item for sublist in all_high_level_callss for item in sublist] return list(set(all_high_level_calls)) @property - def all_high_level_calls(self) -> List["HighLevelCallType"]: + def all_high_level_calls(self) -> List[Tuple["Contract", "HighLevelCall"]]: """ - list((Contract, Function|Variable)): List all of the external high level calls + list(Tuple("Contract", "HighLevelCall")): List all of the external high level calls """ all_high_level_callss = [f.all_high_level_calls() for f in self.functions + self.modifiers] # type: ignore all_high_level_calls = [item for sublist in all_high_level_callss for item in sublist] diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py index 6e8968dfb2..54aec60ac6 100644 --- a/slither/core/declarations/function.py +++ b/slither/core/declarations/function.py @@ -31,19 +31,20 @@ # pylint: disable=import-outside-toplevel,too-many-instance-attributes,too-many-statements,too-many-lines if TYPE_CHECKING: - from slither.utils.type_helpers import ( - InternalCallType, - LowLevelCallType, - HighLevelCallType, - LibraryCallType, - ) from slither.core.declarations import Contract, FunctionContract from slither.core.cfg.node import Node, NodeType from slither.core.variables.variable import Variable from slither.slithir.variables.variable import SlithIRVariable from slither.slithir.variables import LocalIRVariable from slither.core.expressions.expression import Expression - from slither.slithir.operations import Operation + from slither.slithir.operations import ( + HighLevelCall, + InternalCall, + LibraryCall, + LowLevelCall, + SolidityCall, + Operation, + ) from slither.core.compilation_unit import SlitherCompilationUnit from slither.core.scope.scope import FileScope @@ -149,11 +150,11 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit") -> None: self._vars_read_or_written: List["Variable"] = [] self._solidity_vars_read: List["SolidityVariable"] = [] self._state_vars_written: List["StateVariable"] = [] - self._internal_calls: List["InternalCallType"] = [] - self._solidity_calls: List["SolidityFunction"] = [] - self._low_level_calls: List["LowLevelCallType"] = [] - self._high_level_calls: List["HighLevelCallType"] = [] - self._library_calls: List["LibraryCallType"] = [] + self._internal_calls: List["InternalCall"] = [] + self._solidity_calls: List["SolidityCall"] = [] + self._low_level_calls: List["LowLevelCall"] = [] + self._high_level_calls: List[Tuple["Contract", "HighLevelCall"]] = [] + self._library_calls: List["LibraryCall"] = [] self._external_calls_as_expressions: List["Expression"] = [] self._expression_vars_read: List["Expression"] = [] self._expression_vars_written: List["Expression"] = [] @@ -169,11 +170,11 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit") -> None: self._all_expressions: Optional[List["Expression"]] = None self._all_slithir_operations: Optional[List["Operation"]] = None - self._all_internals_calls: Optional[List["InternalCallType"]] = None - self._all_high_level_calls: Optional[List["HighLevelCallType"]] = None - self._all_library_calls: Optional[List["LibraryCallType"]] = None - self._all_low_level_calls: Optional[List["LowLevelCallType"]] = None - self._all_solidity_calls: Optional[List["SolidityFunction"]] = None + self._all_internals_calls: Optional[List["InternalCall"]] = None + self._all_high_level_calls: Optional[List[Tuple["Contract", "HighLevelCall"]]] = None + self._all_library_calls: Optional[List["LibraryCall"]] = None + self._all_low_level_calls: Optional[List["LowLevelCall"]] = None + self._all_solidity_calls: Optional[List["SolidityCall"]] = None self._all_variables_read: Optional[List["Variable"]] = None self._all_variables_written: Optional[List["Variable"]] = None self._all_state_variables_read: Optional[List["StateVariable"]] = None @@ -776,14 +777,14 @@ def add_explicit_base_constructor_calls_statements(self, modif: ModifierStatemen def variables(self) -> List[LocalVariable]: """ Return all local variables - Include paramters and return values + Include parameters and return values """ return list(self._variables.values()) @property def local_variables(self) -> List[LocalVariable]: """ - Return all local variables (dont include paramters and return values) + Return all local variables (dont include parameters and return values) """ return list(set(self.variables) - set(self.returns) - set(self.parameters)) @@ -857,43 +858,42 @@ def slithir_variables(self) -> List["SlithIRVariable"]: ################################################################################### @property - def internal_calls(self) -> List["InternalCallType"]: + def internal_calls(self) -> List["InternalCall"]: """ - list(Function or SolidityFunction): List of function calls (that does not create a transaction) + list(InternalCall): List of IR operations for internal calls """ return list(self._internal_calls) @property - def solidity_calls(self) -> List[SolidityFunction]: + def solidity_calls(self) -> List["SolidityCall"]: """ - list(SolidityFunction): List of Soldity calls + list(SolidityCall): List of IR operations for Solidity calls """ return list(self._solidity_calls) @property - def high_level_calls(self) -> List["HighLevelCallType"]: + def high_level_calls(self) -> List[Tuple["Contract", "HighLevelCall"]]: """ - list((Contract, Function|Variable)): - List of high level calls (external calls). + list(Tuple(Contract, "HighLevelCall")): List of call target contract and IR of the high level call A variable is called in case of call to a public state variable Include library calls """ return list(self._high_level_calls) @property - def library_calls(self) -> List["LibraryCallType"]: + def library_calls(self) -> List["LibraryCall"]: """ - list((Contract, Function)): + list(LibraryCall): List of IR operations for library calls """ return list(self._library_calls) @property - def low_level_calls(self) -> List["LowLevelCallType"]: + def low_level_calls(self) -> List["LowLevelCall"]: """ - list((Variable|SolidityVariable, str)): List of low_level call + list(LowLevelCall): List of IR operations for low level calls A low level call is defined by - the variable called - - the name of the function (call/delegatecall/codecall) + - the name of the function (call/delegatecall/callcode) """ return list(self._low_level_calls) @@ -1121,10 +1121,14 @@ def _explore_functions(self, f_new_values: Callable[["Function"], List]) -> List values = f_new_values(self) explored = [self] to_explore = [ - c for c in self.internal_calls if isinstance(c, Function) and c not in explored + ir.function + for ir in self.internal_calls + if isinstance(ir.function, Function) and ir.function not in explored ] to_explore += [ - c for (_, c) in self.library_calls if isinstance(c, Function) and c not in explored + ir.function + for ir in self.library_calls + if isinstance(ir.function, Function) and ir.function not in explored ] to_explore += [m for m in self.modifiers if m not in explored] @@ -1138,14 +1142,18 @@ def _explore_functions(self, f_new_values: Callable[["Function"], List]) -> List values += f_new_values(f) to_explore += [ - c - for c in f.internal_calls - if isinstance(c, Function) and c not in explored and c not in to_explore + ir.function + for ir in f.internal_calls + if isinstance(ir.function, Function) + and ir.function not in explored + and ir.function not in to_explore ] to_explore += [ - c - for (_, c) in f.library_calls - if isinstance(c, Function) and c not in explored and c not in to_explore + ir.function + for ir in f.library_calls + if isinstance(ir.function, Function) + and ir.function not in explored + and ir.function not in to_explore ] to_explore += [m for m in f.modifiers if m not in explored and m not in to_explore] @@ -1210,31 +1218,31 @@ def all_state_variables_written(self) -> List[StateVariable]: ) return self._all_state_variables_written - def all_internal_calls(self) -> List["InternalCallType"]: + def all_internal_calls(self) -> List["InternalCall"]: """recursive version of internal_calls""" if self._all_internals_calls is None: self._all_internals_calls = self._explore_functions(lambda x: x.internal_calls) return self._all_internals_calls - def all_low_level_calls(self) -> List["LowLevelCallType"]: + def all_low_level_calls(self) -> List["LowLevelCall"]: """recursive version of low_level calls""" if self._all_low_level_calls is None: self._all_low_level_calls = self._explore_functions(lambda x: x.low_level_calls) return self._all_low_level_calls - def all_high_level_calls(self) -> List["HighLevelCallType"]: + def all_high_level_calls(self) -> List[Tuple["Contract", "HighLevelCall"]]: """recursive version of high_level calls""" if self._all_high_level_calls is None: self._all_high_level_calls = self._explore_functions(lambda x: x.high_level_calls) return self._all_high_level_calls - def all_library_calls(self) -> List["LibraryCallType"]: + def all_library_calls(self) -> List["LibraryCall"]: """recursive version of library calls""" if self._all_library_calls is None: self._all_library_calls = self._explore_functions(lambda x: x.library_calls) return self._all_library_calls - def all_solidity_calls(self) -> List[SolidityFunction]: + def all_solidity_calls(self) -> List["SolidityCall"]: """recursive version of solidity calls""" if self._all_solidity_calls is None: self._all_solidity_calls = self._explore_functions(lambda x: x.solidity_calls) @@ -1653,7 +1661,9 @@ def _analyze_calls(self) -> None: internal_calls = [item for sublist in internal_calls for item in sublist] self._internal_calls = list(set(internal_calls)) - self._solidity_calls = [c for c in internal_calls if isinstance(c, SolidityFunction)] + self._solidity_calls = [ + ir for ir in internal_calls if isinstance(ir.function, SolidityFunction) + ] low_level_calls = [x.low_level_calls for x in self.nodes] low_level_calls = [x for x in low_level_calls if x] diff --git a/slither/core/declarations/solidity_variables.py b/slither/core/declarations/solidity_variables.py index 8094ab7c35..fa4e70f1a3 100644 --- a/slither/core/declarations/solidity_variables.py +++ b/slither/core/declarations/solidity_variables.py @@ -50,6 +50,7 @@ "assert(bool)": [], "require(bool)": [], "require(bool,string)": [], + "require(bool,error)": [], # Solidity 0.8.26 via-ir and Solidity >= 0.8.27 "revert()": [], "revert(string)": [], "revert ": [], @@ -82,7 +83,7 @@ "abi.encodeCall()": ["bytes"], "bytes.concat()": ["bytes"], "string.concat()": ["string"], - # abi.decode returns an a list arbitrary types + # abi.decode returns a list arbitrary types "abi.decode()": [], "type(address)": [], "type()": [], # 0.6.8 changed type(address) to type() diff --git a/slither/core/slither_core.py b/slither/core/slither_core.py index 1206e564bc..f5e36ace31 100644 --- a/slither/core/slither_core.py +++ b/slither/core/slither_core.py @@ -8,7 +8,7 @@ import posixpath import re from collections import defaultdict -from typing import Optional, Dict, List, Set, Union, Tuple +from typing import Optional, Dict, List, Set, Union, Tuple, TypeVar from crytic_compile import CryticCompile from crytic_compile.utils.naming import Filename @@ -88,6 +88,7 @@ def __init__(self) -> None: self._contracts: List[Contract] = [] self._contracts_derived: List[Contract] = [] + self._offset_to_min_offset: Optional[Dict[Filename, Dict[int, Set[int]]]] = None self._offset_to_objects: Optional[Dict[Filename, Dict[int, Set[SourceMapping]]]] = None self._offset_to_references: Optional[Dict[Filename, Dict[int, Set[Source]]]] = None self._offset_to_implementations: Optional[Dict[Filename, Dict[int, Set[Source]]]] = None @@ -195,69 +196,70 @@ def print_functions(self, d: str): for f in c.functions: f.cfg_to_dot(os.path.join(d, f"{c.name}.{f.name}.dot")) - def offset_to_objects(self, filename_str: str, offset: int) -> Set[SourceMapping]: - if self._offset_to_objects is None: - self._compute_offsets_to_ref_impl_decl() - filename: Filename = self.crytic_compile.filename_lookup(filename_str) - return self._offset_to_objects[filename][offset] - def _compute_offsets_from_thing(self, thing: SourceMapping): definition = get_definition(thing, self.crytic_compile) references = get_references(thing) implementations = get_all_implementations(thing, self.contracts) + # Create the offset mapping for offset in range(definition.start, definition.end + 1): - if ( - isinstance(thing, (TopLevel, Contract)) - or ( - isinstance(thing, FunctionContract) - and thing.contract_declarer == thing.contract - ) - or (isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract)) - ): + self._offset_to_min_offset[definition.filename][offset].add(definition.start) - self._offset_to_objects[definition.filename][offset].add(thing) + is_declared_function = ( + isinstance(thing, FunctionContract) and thing.contract_declarer == thing.contract + ) - self._offset_to_definitions[definition.filename][offset].add(definition) - self._offset_to_implementations[definition.filename][offset].update(implementations) - self._offset_to_references[definition.filename][offset] |= set(references) + should_add_to_objects = ( + isinstance(thing, (TopLevel, Contract)) + or is_declared_function + or (isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract)) + ) + + if should_add_to_objects: + self._offset_to_objects[definition.filename][definition.start].add(thing) + + self._offset_to_definitions[definition.filename][definition.start].add(definition) + self._offset_to_implementations[definition.filename][definition.start].update( + implementations + ) + self._offset_to_references[definition.filename][definition.start] |= set(references) + + # For references + should_add_to_objects = ( + isinstance(thing, TopLevel) + or is_declared_function + or (isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract)) + ) for ref in references: for offset in range(ref.start, ref.end + 1): - is_declared_function = ( - isinstance(thing, FunctionContract) - and thing.contract_declarer == thing.contract - ) + self._offset_to_min_offset[definition.filename][offset].add(ref.start) + + if should_add_to_objects: + self._offset_to_objects[definition.filename][ref.start].add(thing) + + if is_declared_function: + # Only show the nearest lexical definition for declared contract-level functions if ( - isinstance(thing, TopLevel) - or is_declared_function - or ( - isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract) - ) + thing.contract.source_mapping.start + < ref.start + < thing.contract.source_mapping.end ): - self._offset_to_objects[definition.filename][offset].add(thing) - - if is_declared_function: - # Only show the nearest lexical definition for declared contract-level functions - if ( - thing.contract.source_mapping.start - < offset - < thing.contract.source_mapping.end - ): - self._offset_to_definitions[ref.filename][offset].add(definition) + self._offset_to_definitions[ref.filename][ref.start].add(definition) - else: - self._offset_to_definitions[ref.filename][offset].add(definition) + else: + self._offset_to_definitions[ref.filename][ref.start].add(definition) - self._offset_to_implementations[ref.filename][offset].update(implementations) - self._offset_to_references[ref.filename][offset] |= set(references) + self._offset_to_implementations[ref.filename][ref.start].update(implementations) + self._offset_to_references[ref.filename][ref.start] |= set(references) def _compute_offsets_to_ref_impl_decl(self): # pylint: disable=too-many-branches self._offset_to_references = defaultdict(lambda: defaultdict(lambda: set())) self._offset_to_definitions = defaultdict(lambda: defaultdict(lambda: set())) self._offset_to_implementations = defaultdict(lambda: defaultdict(lambda: set())) self._offset_to_objects = defaultdict(lambda: defaultdict(lambda: set())) + self._offset_to_min_offset = defaultdict(lambda: defaultdict(lambda: set())) for compilation_unit in self._compilation_units: for contract in compilation_unit.contracts: @@ -308,23 +310,59 @@ def _compute_offsets_to_ref_impl_decl(self): # pylint: disable=too-many-branche for pragma in compilation_unit.pragma_directives: self._compute_offsets_from_thing(pragma) + T = TypeVar("T", Source, SourceMapping) + + def _get_offset( + self, mapping: Dict[Filename, Dict[int, Set[T]]], filename_str: str, offset: int + ) -> Set[T]: + """Get the Source/SourceMapping referenced by the offset. + + For performance reasons, references are only stored once at the lowest offset. + It uses the _offset_to_min_offset mapping to retrieve the correct offsets. + As multiple definitions can be related to the same offset, we retrieve all of them. + + :param mapping: Mapping to search for (objects. references, ...) + :param filename_str: Filename to consider + :param offset: Look-up offset + :raises IndexError: When the start offset is not found + :return: The corresponding set of Source/SourceMapping + """ + filename: Filename = self.crytic_compile.filename_lookup(filename_str) + + start_offsets = self._offset_to_min_offset[filename][offset] + if not start_offsets: + msg = f"Unable to find reference for offset {offset}" + raise IndexError(msg) + + results = set() + for start_offset in start_offsets: + results |= mapping[filename][start_offset] + + return results + def offset_to_references(self, filename_str: str, offset: int) -> Set[Source]: if self._offset_to_references is None: self._compute_offsets_to_ref_impl_decl() - filename: Filename = self.crytic_compile.filename_lookup(filename_str) - return self._offset_to_references[filename][offset] + + return self._get_offset(self._offset_to_references, filename_str, offset) def offset_to_implementations(self, filename_str: str, offset: int) -> Set[Source]: if self._offset_to_implementations is None: self._compute_offsets_to_ref_impl_decl() - filename: Filename = self.crytic_compile.filename_lookup(filename_str) - return self._offset_to_implementations[filename][offset] + + return self._get_offset(self._offset_to_implementations, filename_str, offset) def offset_to_definitions(self, filename_str: str, offset: int) -> Set[Source]: if self._offset_to_definitions is None: self._compute_offsets_to_ref_impl_decl() - filename: Filename = self.crytic_compile.filename_lookup(filename_str) - return self._offset_to_definitions[filename][offset] + + return self._get_offset(self._offset_to_definitions, filename_str, offset) + + def offset_to_objects(self, filename_str: str, offset: int) -> Set[SourceMapping]: + if self._offset_to_objects is None: + self._compute_offsets_to_ref_impl_decl() + + return self._get_offset(self._offset_to_objects, filename_str, offset) # endregion ################################################################################### diff --git a/slither/core/solidity_types/function_type.py b/slither/core/solidity_types/function_type.py index 2d644148e0..8a328e361a 100644 --- a/slither/core/solidity_types/function_type.py +++ b/slither/core/solidity_types/function_type.py @@ -36,7 +36,7 @@ def storage_size(self) -> Tuple[int, bool]: def is_dynamic(self) -> bool: return False - def __str__(self): + def __str__(self) -> str: # Use x.type # x.name may be empty params = ",".join([str(x.type) for x in self._params]) diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index 41841f1e80..355aa55386 100644 --- a/slither/core/source_mapping/source_mapping.py +++ b/slither/core/source_mapping/source_mapping.py @@ -112,12 +112,8 @@ def __eq__(self, other: Any) -> bool: try: return ( self.start == other.start - and self.length == other.length - and self.filename == other.filename + and self.filename.relative == other.filename.relative and self.is_dependency == other.is_dependency - and self.lines == other.lines - and self.starting_column == other.starting_column - and self.ending_column == other.ending_column and self.end == other.end ) except AttributeError: diff --git a/slither/core/variables/state_variable.py b/slither/core/variables/state_variable.py index f2a2d6ee3c..d3e3e60182 100644 --- a/slither/core/variables/state_variable.py +++ b/slither/core/variables/state_variable.py @@ -12,6 +12,7 @@ class StateVariable(ContractLevel, Variable): def __init__(self) -> None: super().__init__() self._node_initialization: Optional["Node"] = None + self._location: Optional[str] = None def is_declared_by(self, contract: "Contract") -> bool: """ @@ -21,6 +22,35 @@ def is_declared_by(self, contract: "Contract") -> bool: """ return self.contract == contract + def set_location(self, loc: str) -> None: + self._location = loc + + @property + def location(self) -> Optional[str]: + """ + Variable Location + Can be default or transient + Returns: + (str) + """ + return self._location + + @property + def is_stored(self) -> bool: + """ + Checks if the state variable is stored, based on it not being constant or immutable or transient. + """ + return ( + not self._is_constant and not self._is_immutable and not self._location == "transient" + ) + + @property + def is_transient(self) -> bool: + """ + Checks if the state variable is transient. A transient variable can not be constant or immutable. + """ + return self._location == "transient" + # endregion ################################################################################### ################################################################################### diff --git a/slither/core/variables/variable.py b/slither/core/variables/variable.py index 63d1a7a838..4af350d31e 100644 --- a/slither/core/variables/variable.py +++ b/slither/core/variables/variable.py @@ -9,6 +9,7 @@ if TYPE_CHECKING: from slither.core.expressions.expression import Expression + from slither.core.declarations import Function # pylint: disable=too-many-instance-attributes class Variable(SourceMapping): @@ -16,7 +17,7 @@ def __init__(self) -> None: super().__init__() self._name: Optional[str] = None self._initial_expression: Optional["Expression"] = None - self._type: Optional[Type] = None + self._type: Optional[Union[List, Type, "Function", str]] = None self._initialized: Optional[bool] = None self._visibility: Optional[str] = None self._is_constant = False @@ -77,7 +78,7 @@ def name(self, name: str) -> None: self._name = name @property - def type(self) -> Optional[Type]: + def type(self) -> Optional[Union[List, Type, "Function", str]]: return self._type @type.setter @@ -93,13 +94,6 @@ def is_constant(self) -> bool: def is_constant(self, is_cst: bool) -> None: self._is_constant = is_cst - @property - def is_stored(self) -> bool: - """ - Checks if a variable is stored, based on it not being constant or immutable. Future updates may adjust for new non-storage keywords. - """ - return not self._is_constant and not self._is_immutable - @property def is_reentrant(self) -> bool: return self._is_reentrant @@ -127,7 +121,7 @@ def visibility(self) -> Optional[str]: def visibility(self, v: str) -> None: self._visibility = v - def set_type(self, t: Optional[Union[List, Type, str]]) -> None: + def set_type(self, t: Optional[Union[List, Type, "Function", str]]) -> None: if isinstance(t, str): self._type = ElementaryType(t) return diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 44a168c2b6..a30d2b3c00 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -97,5 +97,12 @@ from .statements.tautological_compare import TautologicalCompare from .statements.return_bomb import ReturnBomb from .functions.out_of_order_retryable import OutOfOrderRetryable +from .functions.gelato_unprotected_randomness import GelatoUnprotectedRandomness +from .statements.chronicle_unchecked_price import ChronicleUncheckedPrice +from .statements.pyth_unchecked_confidence import PythUncheckedConfidence +from .statements.pyth_unchecked_publishtime import PythUncheckedPublishTime +from .functions.chainlink_feed_registry import ChainlinkFeedRegistry +from .functions.pyth_deprecated_functions import PythDeprecatedFunctions +from .functions.optimism_deprecation import OptimismDeprecation # from .statements.unused_import import UnusedImport diff --git a/slither/detectors/assembly/incorrect_return.py b/slither/detectors/assembly/incorrect_return.py index bd5a6d8449..9052979ace 100644 --- a/slither/detectors/assembly/incorrect_return.py +++ b/slither/detectors/assembly/incorrect_return.py @@ -21,10 +21,8 @@ def _assembly_node(function: Function) -> Optional[SolidityCall]: """ - for ir in function.all_slithir_operations(): - if isinstance(ir, SolidityCall) and ir.function == SolidityFunction( - "return(uint256,uint256)" - ): + for ir in function.all_solidity_calls(): + if ir.function == SolidityFunction("return(uint256,uint256)"): return ir return None @@ -71,23 +69,23 @@ def _detect(self) -> List[Output]: for c in self.contracts: for f in c.functions_and_modifiers_declared: - for node in f.nodes: - if node.sons: - for function_called in node.internal_calls: - if isinstance(function_called, Function): - found = _assembly_node(function_called) - if found: - - info: DETECTOR_INFO = [ - f, - " calls ", - function_called, - " which halt the execution ", - found.node, - "\n", - ] - json = self.generate_result(info) - - results.append(json) + for ir in f.internal_calls: + if ir.node.sons: + function_called = ir.function + if isinstance(function_called, Function): + found = _assembly_node(function_called) + if found: + + info: DETECTOR_INFO = [ + f, + " calls ", + function_called, + " which halt the execution ", + found.node, + "\n", + ] + json = self.generate_result(info) + + results.append(json) return results diff --git a/slither/detectors/assembly/return_instead_of_leave.py b/slither/detectors/assembly/return_instead_of_leave.py index a1ad9c87e9..6037059744 100644 --- a/slither/detectors/assembly/return_instead_of_leave.py +++ b/slither/detectors/assembly/return_instead_of_leave.py @@ -6,7 +6,6 @@ DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import SolidityCall from slither.utils.output import Output @@ -42,15 +41,12 @@ class ReturnInsteadOfLeave(AbstractDetector): def _check_function(self, f: Function) -> List[Output]: results: List[Output] = [] - for node in f.nodes: - for ir in node.irs: - if isinstance(ir, SolidityCall) and ir.function == SolidityFunction( - "return(uint256,uint256)" - ): - info: DETECTOR_INFO = [f, " contains an incorrect call to return: ", node, "\n"] - json = self.generate_result(info) + for ir in f.solidity_calls: + if ir.function == SolidityFunction("return(uint256,uint256)"): + info: DETECTOR_INFO = [f, " contains an incorrect call to return: ", ir.node, "\n"] + json = self.generate_result(info) - results.append(json) + results.append(json) return results def _detect(self) -> List[Output]: diff --git a/slither/detectors/attributes/locked_ether.py b/slither/detectors/attributes/locked_ether.py index 91ec686503..efb376e229 100644 --- a/slither/detectors/attributes/locked_ether.py +++ b/slither/detectors/attributes/locked_ether.py @@ -59,7 +59,7 @@ def do_no_send_ether(contract: Contract) -> bool: explored += to_explore to_explore = [] for function in functions: - calls = [c.name for c in function.internal_calls] + calls = [ir.function.name for ir in function.internal_calls] if "suicide(address)" in calls or "selfdestruct(address)" in calls: return False for node in function.nodes: diff --git a/slither/detectors/compiler_bugs/array_by_reference.py b/slither/detectors/compiler_bugs/array_by_reference.py index 47e2af5819..e4dde43608 100644 --- a/slither/detectors/compiler_bugs/array_by_reference.py +++ b/slither/detectors/compiler_bugs/array_by_reference.py @@ -13,8 +13,6 @@ from slither.core.solidity_types.array_type import ArrayType from slither.core.variables.state_variable import StateVariable from slither.core.variables.local_variable import LocalVariable -from slither.slithir.operations.high_level_call import HighLevelCall -from slither.slithir.operations.internal_call import InternalCall from slither.core.cfg.node import Node from slither.core.declarations.contract import Contract from slither.core.declarations.function_contract import FunctionContract @@ -117,37 +115,26 @@ def detect_calls_passing_ref_to_function( # pylint: disable=too-many-nested-blocks for contract in contracts: for function in contract.functions_and_modifiers_declared: - for node in function.nodes: + for ir in [ir for _, ir in function.high_level_calls] + function.internal_calls: - # If this node has no expression, skip it. - if not node.expression: + # Verify this references a function in our array modifying functions collection. + if ir.function not in array_modifying_funcs: continue - for ir in node.irs: - # Verify this is a high level call. - if not isinstance(ir, (HighLevelCall, InternalCall)): + # Verify one of these parameters is an array in storage. + for (param, arg) in zip(ir.function.parameters, ir.arguments): + # Verify this argument is a variable that is an array type. + if not isinstance(arg, (StateVariable, LocalVariable)): continue - - # Verify this references a function in our array modifying functions collection. - if ir.function not in array_modifying_funcs: + if not isinstance(arg.type, ArrayType): continue - # Verify one of these parameters is an array in storage. - for (param, arg) in zip(ir.function.parameters, ir.arguments): - # Verify this argument is a variable that is an array type. - if not isinstance(arg, (StateVariable, LocalVariable)): - continue - if not isinstance(arg.type, ArrayType): - continue - - # If it is a state variable OR a local variable referencing storage, we add it to the list. - if ( - isinstance(arg, StateVariable) - or (isinstance(arg, LocalVariable) and arg.location == "storage") - ) and ( - isinstance(param.type, ArrayType) and param.location != "storage" - ): - results.append((node, arg, ir.function)) + # If it is a state variable OR a local variable referencing storage, we add it to the list. + if ( + isinstance(arg, StateVariable) + or (isinstance(arg, LocalVariable) and arg.location == "storage") + ) and (isinstance(param.type, ArrayType) and param.location != "storage"): + results.append((ir.node, arg, ir.function)) return results def _detect(self) -> List[Output]: diff --git a/slither/detectors/erc/erc20/arbitrary_send_erc20.py b/slither/detectors/erc/erc20/arbitrary_send_erc20.py index f060054590..4dc1f8db50 100644 --- a/slither/detectors/erc/erc20/arbitrary_send_erc20.py +++ b/slither/detectors/erc/erc20/arbitrary_send_erc20.py @@ -3,7 +3,7 @@ from slither.analyses.data_dependency.data_dependency import is_dependent from slither.core.cfg.node import Node from slither.core.compilation_unit import SlitherCompilationUnit -from slither.core.declarations import Contract, Function, SolidityVariableComposed +from slither.core.declarations import Contract, Function, SolidityVariableComposed, FunctionContract from slither.core.declarations.solidity_variables import SolidityVariable from slither.slithir.operations import HighLevelCall, LibraryCall @@ -31,11 +31,11 @@ def permit_results(self) -> List[Node]: def _detect_arbitrary_from(self, contract: Contract) -> None: for f in contract.functions: all_high_level_calls = [ - f_called[1].solidity_signature - for f_called in f.high_level_calls - if isinstance(f_called[1], Function) + ir.function.solidity_signature + for _, ir in f.high_level_calls + if isinstance(ir.function, Function) ] - all_library_calls = [f_called[1].solidity_signature for f_called in f.library_calls] + all_library_calls = [ir.function.solidity_signature for ir in f.library_calls] if ( "transferFrom(address,address,uint256)" in all_high_level_calls or "safeTransferFrom(address,address,address,uint256)" in all_library_calls @@ -44,51 +44,50 @@ def _detect_arbitrary_from(self, contract: Contract) -> None: "permit(address,address,uint256,uint256,uint8,bytes32,bytes32)" in all_high_level_calls ): - ArbitrarySendErc20._arbitrary_from(f.nodes, self._permit_results) + ArbitrarySendErc20._arbitrary_from(f, self._permit_results) else: - ArbitrarySendErc20._arbitrary_from(f.nodes, self._no_permit_results) + ArbitrarySendErc20._arbitrary_from(f, self._no_permit_results) @staticmethod - def _arbitrary_from(nodes: List[Node], results: List[Node]) -> None: + def _arbitrary_from(function: FunctionContract, results: List[Node]) -> None: """Finds instances of (safe)transferFrom that do not use msg.sender or address(this) as from parameter.""" - for node in nodes: - for ir in node.irs: - if ( - isinstance(ir, HighLevelCall) - and isinstance(ir.function, Function) - and ir.function.solidity_signature == "transferFrom(address,address,uint256)" - and not ( - is_dependent( - ir.arguments[0], - SolidityVariableComposed("msg.sender"), - node, - ) - or is_dependent( - ir.arguments[0], - SolidityVariable("this"), - node, - ) + for _, ir in function.high_level_calls: + if ( + isinstance(ir, LibraryCall) + and ir.function.solidity_signature + == "safeTransferFrom(address,address,address,uint256)" + and not ( + is_dependent( + ir.arguments[1], + SolidityVariableComposed("msg.sender"), + ir.node, ) - ): - results.append(ir.node) - elif ( - isinstance(ir, LibraryCall) - and ir.function.solidity_signature - == "safeTransferFrom(address,address,address,uint256)" - and not ( - is_dependent( - ir.arguments[1], - SolidityVariableComposed("msg.sender"), - node, - ) - or is_dependent( - ir.arguments[1], - SolidityVariable("this"), - node, - ) + or is_dependent( + ir.arguments[1], + SolidityVariable("this"), + ir.node, ) - ): - results.append(ir.node) + ) + ): + results.append(ir.node) + elif ( + isinstance(ir, HighLevelCall) + and isinstance(ir.function, Function) + and ir.function.solidity_signature == "transferFrom(address,address,uint256)" + and not ( + is_dependent( + ir.arguments[0], + SolidityVariableComposed("msg.sender"), + ir.node, + ) + or is_dependent( + ir.arguments[0], + SolidityVariable("this"), + ir.node, + ) + ) + ): + results.append(ir.node) def detect(self) -> None: """Detect transfers that use arbitrary `from` parameter.""" diff --git a/slither/detectors/functions/chainlink_feed_registry.py b/slither/detectors/functions/chainlink_feed_registry.py new file mode 100644 index 0000000000..82ab17424e --- /dev/null +++ b/slither/detectors/functions/chainlink_feed_registry.py @@ -0,0 +1,102 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output + + +class ChainlinkFeedRegistry(AbstractDetector): + + ARGUMENT = "chainlink-feed-registry" + HELP = "Detect when chainlink feed registry is used" + IMPACT = DetectorClassification.LOW + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#chainlink-feed-registry" + + WIKI_TITLE = "Chainlink Feed Registry usage" + WIKI_DESCRIPTION = "Detect when Chainlink Feed Registry is used. At the moment is only available on Ethereum Mainnet." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ +```solidity +import "chainlink/contracts/src/v0.8/interfaces/FeedRegistryInteface.sol" + +contract A { + FeedRegistryInterface public immutable registry; + + constructor(address _registry) { + registry = _registry; + } + + function getPrice(address base, address quote) public return(uint256) { + (, int256 price,,,) = registry.latestRoundData(base, quote); + // Do price validation + return uint256(price); + } +} +``` +If the contract is deployed on a different chain than Ethereum Mainnet the `getPrice` function will revert. +""" + # endregion wiki_exploit_scenario + + WIKI_RECOMMENDATION = "Do not use Chainlink Feed Registry outside of Ethereum Mainnet." + + def _detect(self) -> List[Output]: + # https://github.com/smartcontractkit/chainlink/blob/8ca41fc8f722accfccccb4b1778db2df8fef5437/contracts/src/v0.8/interfaces/FeedRegistryInterface.sol + registry_functions = [ + "decimals", + "description", + "versiom", + "latestRoundData", + "getRoundData", + "latestAnswer", + "latestTimestamp", + "latestRound", + "getAnswer", + "getTimestamp", + "getFeed", + "getPhaseFeed", + "isFeedEnabled", + "getPhase", + "getRoundFeed", + "getPhaseRange", + "getPreviousRoundId", + "getNextRoundId", + "proposeFeed", + "confirmFeed", + "getProposedFeed", + "proposedGetRoundData", + "proposedLatestRoundData", + "getCurrentPhaseId", + ] + results = [] + + for contract in self.compilation_unit.contracts_derived: + nodes = [] + for target, ir in contract.all_high_level_calls: + if ( + target.name == "FeedRegistryInterface" + and ir.function_name in registry_functions + ): + nodes.append(ir.node) + # Sort so output is deterministic + nodes.sort(key=lambda x: (x.node_id, x.function.full_name)) + + if len(nodes) > 0: + info: DETECTOR_INFO = [ + "The Chainlink Feed Registry is used in the ", + contract.name, + " contract. It's only available on Ethereum Mainnet, consider to not use it if the contract needs to be deployed on other chains.\n", + ] + + for node in nodes: + info.extend(["\t - ", node, "\n"]) + + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/functions/dead_code.py b/slither/detectors/functions/dead_code.py index 5cafa16504..3628d10a2c 100644 --- a/slither/detectors/functions/dead_code.py +++ b/slither/detectors/functions/dead_code.py @@ -48,13 +48,15 @@ def _detect(self) -> List[Output]: all_functionss_called = [ f.all_internal_calls() for f in contract.functions_entry_points ] - all_functions_called = [item for sublist in all_functionss_called for item in sublist] + all_functions_called = [ + item.function for sublist in all_functionss_called for item in sublist + ] functions_used |= { f.canonical_name for f in all_functions_called if isinstance(f, Function) } all_libss_called = [f.all_library_calls() for f in contract.functions_entry_points] all_libs_called: List[Tuple[Contract, Function]] = [ - item for sublist in all_libss_called for item in sublist + item.function for sublist in all_libss_called for item in sublist ] functions_used |= { lib[1].canonical_name for lib in all_libs_called if isinstance(lib, tuple) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 5858c2baf2..d9cc2bc361 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -13,8 +13,7 @@ make_solc_versions, ) from slither.formatters.functions.external_function import custom_format -from slither.slithir.operations import InternalCall, InternalDynamicCall -from slither.slithir.operations import SolidityCall +from slither.slithir.operations import InternalDynamicCall from slither.utils.output import Output @@ -55,11 +54,11 @@ def detect_functions_called(contract: Contract) -> List[Function]: for func in contract.all_functions_called: if not isinstance(func, Function): continue - # Loop through all nodes in the function, add all calls to a list. - for node in func.nodes: - for ir in node.irs: - if isinstance(ir, (InternalCall, SolidityCall)): - result.append(ir.function) + + # Loop through all internal and solidity calls in the function, add them to a list. + for ir in func.internal_calls + func.solidity_calls: + result.append(ir.function) + return result @staticmethod @@ -101,6 +100,7 @@ def get_base_most_function(function: FunctionContract) -> FunctionContract: # Somehow we couldn't resolve it, which shouldn't happen, as the provided function should be found if we could # not find some any more basic. + # pylint: disable=broad-exception-raised raise Exception("Could not resolve the base-most function for the provided function.") @staticmethod diff --git a/slither/detectors/functions/gelato_unprotected_randomness.py b/slither/detectors/functions/gelato_unprotected_randomness.py new file mode 100644 index 0000000000..bdc3a6fb09 --- /dev/null +++ b/slither/detectors/functions/gelato_unprotected_randomness.py @@ -0,0 +1,78 @@ +from typing import List + +from slither.slithir.operations.internal_call import InternalCall +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output + + +class GelatoUnprotectedRandomness(AbstractDetector): + """ + Unprotected Gelato VRF requests + """ + + ARGUMENT = "gelato-unprotected-randomness" + HELP = "Call to _requestRandomness within an unprotected function" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#gelato-unprotected-randomness" + + WIKI_TITLE = "Gelato unprotected randomness" + WIKI_DESCRIPTION = "Detect calls to `_requestRandomness` within an unprotected function." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ +```solidity +contract C is GelatoVRFConsumerBase { + function _fulfillRandomness( + uint256 randomness, + uint256, + bytes memory extraData + ) internal override { + // Do something with the random number + } + + function bad() public { + _requestRandomness(abi.encode(msg.sender)); + } +} +``` +The function `bad` is uprotected and requests randomness.""" + # endregion wiki_exploit_scenario + + WIKI_RECOMMENDATION = ( + "Function that request randomness should be allowed only to authorized users." + ) + + def _detect(self) -> List[Output]: + results = [] + + for contract in self.compilation_unit.contracts_derived: + if "GelatoVRFConsumerBase" in [c.name for c in contract.inheritance]: + for function in contract.functions_entry_points: + if not function.is_protected() and ( + nodes_request := [ + ir.node + for ir in function.all_internal_calls() + if isinstance(ir, InternalCall) + and ir.function_name == "_requestRandomness" + ] + ): + # Sort so output is deterministic + nodes_request.sort(key=lambda x: (x.node_id, x.function.full_name)) + + for node in nodes_request: + info: DETECTOR_INFO = [ + function, + " is unprotected and request randomness from Gelato VRF\n\t- ", + node, + "\n", + ] + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/functions/modifier.py b/slither/detectors/functions/modifier.py index 7f14872663..862d2af67e 100644 --- a/slither/detectors/functions/modifier.py +++ b/slither/detectors/functions/modifier.py @@ -17,7 +17,7 @@ def is_revert(node: Node) -> bool: return node.type == NodeType.THROW or any( - c.name in ["revert()", "revert(string"] for c in node.internal_calls + ir.function.name in ["revert()", "revert(string"] for ir in node.internal_calls ) @@ -67,13 +67,16 @@ class ModifierDefaultDetection(AbstractDetector): def _detect(self) -> List[Output]: results = [] + # pylint: disable=too-many-nested-blocks for c in self.contracts: for mod in c.modifiers: if mod.contract_declarer != c: continue # Walk down the tree, only looking at nodes in the outer scope node = mod.entry_point + node_seen = [] while node is not None: + node_seen.append(node) # If any node in the outer scope executes _; or reverts, # we will never return a default value if node.type == NodeType.PLACEHOLDER or is_revert(node): @@ -81,7 +84,13 @@ def _detect(self) -> List[Output]: # Move down, staying on the outer scope in branches if len(node.sons) > 0: - node = _get_false_son(node) if node.contains_if() else node.sons[0] + if node.contains_if(): + node = _get_false_son(node) + else: + if node.sons[0] in node_seen: + node = node.sons[1] + else: + node = node.sons[0] else: node = None else: diff --git a/slither/detectors/functions/optimism_deprecation.py b/slither/detectors/functions/optimism_deprecation.py new file mode 100644 index 0000000000..752e8bb2d9 --- /dev/null +++ b/slither/detectors/functions/optimism_deprecation.py @@ -0,0 +1,92 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.core.cfg.node import Node +from slither.core.variables.variable import Variable +from slither.core.expressions import TypeConversion, Literal +from slither.utils.output import Output + + +class OptimismDeprecation(AbstractDetector): + + ARGUMENT = "optimism-deprecation" + HELP = "Detect when deprecated Optimism predeploy or function is used." + IMPACT = DetectorClassification.LOW + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#optimism-deprecation" + + WIKI_TITLE = "Optimism deprecated predeploy or function" + WIKI_DESCRIPTION = "Detect when deprecated Optimism predeploy or function is used." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ +```solidity +interface GasPriceOracle { + function scalar() external view returns (uint256); +} + +contract Test { + GasPriceOracle constant OPT_GAS = GasPriceOracle(0x420000000000000000000000000000000000000F); + + function a() public { + OPT_GAS.scalar(); + } +} +``` +The call to the `scalar` function of the Optimism GasPriceOracle predeploy always revert. +""" + # endregion wiki_exploit_scenario + + WIKI_RECOMMENDATION = "Do not use the deprecated components." + + def _detect(self) -> List[Output]: + results = [] + + deprecated_predeploys = [ + "0x4200000000000000000000000000000000000000", # LegacyMessagePasser + "0x4200000000000000000000000000000000000001", # L1MessageSender + "0x4200000000000000000000000000000000000002", # DeployerWhitelist + "0x4200000000000000000000000000000000000013", # L1BlockNumber + ] + + for contract in self.compilation_unit.contracts_derived: + use_deprecated: List[Node] = [] + + for _, ir in contract.all_high_level_calls: + # To avoid FPs we assume predeploy contracts are always assigned to a constant and typecasted to an interface + # and we check the target address of a high level call. + if ( + isinstance(ir.destination, Variable) + and isinstance(ir.destination.expression, TypeConversion) + and isinstance(ir.destination.expression.expression, Literal) + ): + if ir.destination.expression.expression.value in deprecated_predeploys: + use_deprecated.append(ir.node) + + if ( + ir.destination.expression.expression.value + == "0x420000000000000000000000000000000000000F" + and ir.function_name in ("overhead", "scalar", "getL1GasUsed") + ): + use_deprecated.append(ir.node) + # Sort so output is deterministic + use_deprecated.sort(key=lambda x: (x.node_id, x.function.full_name)) + if len(use_deprecated) > 0: + info: DETECTOR_INFO = [ + "A deprecated Optimism predeploy or function is used in the ", + contract.name, + " contract.\n", + ] + + for node in use_deprecated: + info.extend(["\t - ", node, "\n"]) + + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/functions/out_of_order_retryable.py b/slither/detectors/functions/out_of_order_retryable.py index db9096f95f..a11e31ef45 100644 --- a/slither/detectors/functions/out_of_order_retryable.py +++ b/slither/detectors/functions/out_of_order_retryable.py @@ -101,9 +101,9 @@ def _detect_multiple_tickets( # include ops from internal function calls internal_ops = [] - for internal_call in node.internal_calls: - if isinstance(internal_call, Function): - internal_ops += internal_call.all_slithir_operations() + for ir in node.internal_calls: + if isinstance(ir.function, Function): + internal_ops += ir.function.all_slithir_operations() # analyze node for retryable tickets for ir in node.irs + internal_ops: diff --git a/slither/detectors/functions/protected_variable.py b/slither/detectors/functions/protected_variable.py index 5796729262..b9260abd61 100644 --- a/slither/detectors/functions/protected_variable.py +++ b/slither/detectors/functions/protected_variable.py @@ -61,7 +61,9 @@ def _analyze_function(self, function: Function, contract: Contract) -> List[Outp if not function_protection: self.logger.error(f"{function_sig} not found") continue - if function_protection not in function.all_internal_calls(): + if function_protection not in [ + ir.function for ir in function.all_internal_calls() + ]: info: DETECTOR_INFO = [ function, " should have ", diff --git a/slither/detectors/functions/pyth_deprecated_functions.py b/slither/detectors/functions/pyth_deprecated_functions.py new file mode 100644 index 0000000000..87cff9181b --- /dev/null +++ b/slither/detectors/functions/pyth_deprecated_functions.py @@ -0,0 +1,73 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output + + +class PythDeprecatedFunctions(AbstractDetector): + """ + Documentation: This detector finds deprecated Pyth function calls + """ + + ARGUMENT = "pyth-deprecated-functions" + HELP = "Detect Pyth deprecated functions" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-deprecated-functions" + WIKI_TITLE = "Pyth deprecated functions" + WIKI_DESCRIPTION = "Detect when a Pyth deprecated function is used" + WIKI_RECOMMENDATION = ( + "Do not use deprecated Pyth functions. Visit https://api-reference.pyth.network/." + ) + + WIKI_EXPLOIT_SCENARIO = """ +```solidity +import "@pythnetwork/pyth-sdk-solidity/IPyth.sol"; +import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol"; + +contract C { + + IPyth pyth; + + constructor(IPyth _pyth) { + pyth = _pyth; + } + + function A(bytes32 priceId) public { + PythStructs.Price memory price = pyth.getPrice(priceId); + ... + } +} +``` +The function `A` uses the deprecated `getPrice` Pyth function. +""" + + def _detect(self): + DEPRECATED_PYTH_FUNCTIONS = [ + "getValidTimePeriod", + "getEmaPrice", + "getPrice", + ] + results: List[Output] = [] + + for contract in self.compilation_unit.contracts_derived: + for target_contract, ir in contract.all_high_level_calls: + if ( + target_contract.name == "IPyth" + and ir.function_name in DEPRECATED_PYTH_FUNCTIONS + ): + info: DETECTOR_INFO = [ + "The following Pyth deprecated function is used\n\t- ", + ir.node, + "\n", + ] + + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/functions/suicidal.py b/slither/detectors/functions/suicidal.py index f0af978ec7..7c7d87f8a6 100644 --- a/slither/detectors/functions/suicidal.py +++ b/slither/detectors/functions/suicidal.py @@ -59,7 +59,7 @@ def detect_suicidal_func(func: FunctionContract) -> bool: if func.visibility not in ["public", "external"]: return False - calls = [c.name for c in func.all_internal_calls()] + calls = [ir.function.name for ir in func.all_internal_calls()] if not ("suicide(address)" in calls or "selfdestruct(address)" in calls): return False diff --git a/slither/detectors/operations/encode_packed.py b/slither/detectors/operations/encode_packed.py index ea7b094df2..b661ddcd72 100644 --- a/slither/detectors/operations/encode_packed.py +++ b/slither/detectors/operations/encode_packed.py @@ -3,14 +3,14 @@ """ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification -from slither.core.declarations.solidity_variables import SolidityFunction -from slither.slithir.operations import SolidityCall +from slither.core.declarations import Contract, SolidityFunction +from slither.core.variables import Variable from slither.analyses.data_dependency.data_dependency import is_tainted from slither.core.solidity_types import ElementaryType from slither.core.solidity_types import ArrayType -def _is_dynamic_type(arg): +def _is_dynamic_type(arg: Variable): """ Args: arg (function argument) @@ -25,7 +25,7 @@ def _is_dynamic_type(arg): return False -def _detect_abi_encodePacked_collision(contract): +def _detect_abi_encodePacked_collision(contract: Contract): """ Args: contract (Contract) @@ -35,22 +35,19 @@ def _detect_abi_encodePacked_collision(contract): ret = [] # pylint: disable=too-many-nested-blocks for f in contract.functions_and_modifiers_declared: - for n in f.nodes: - for ir in n.irs: - if isinstance(ir, SolidityCall) and ir.function == SolidityFunction( - "abi.encodePacked()" - ): - dynamic_type_count = 0 - for arg in ir.arguments: - if is_tainted(arg, contract) and _is_dynamic_type(arg): - dynamic_type_count += 1 - elif dynamic_type_count > 1: - ret.append((f, n)) - dynamic_type_count = 0 - else: - dynamic_type_count = 0 - if dynamic_type_count > 1: - ret.append((f, n)) + for ir in f.solidity_calls: + if ir.function == SolidityFunction("abi.encodePacked()"): + dynamic_type_count = 0 + for arg in ir.arguments: + if is_tainted(arg, contract) and _is_dynamic_type(arg): + dynamic_type_count += 1 + elif dynamic_type_count > 1: + ret.append((f, ir.node)) + dynamic_type_count = 0 + else: + dynamic_type_count = 0 + if dynamic_type_count > 1: + ret.append((f, ir.node)) return ret diff --git a/slither/detectors/operations/low_level_calls.py b/slither/detectors/operations/low_level_calls.py index 463c748757..4925fc4661 100644 --- a/slither/detectors/operations/low_level_calls.py +++ b/slither/detectors/operations/low_level_calls.py @@ -44,10 +44,9 @@ def detect_low_level_calls( ) -> List[Tuple[FunctionContract, List[Node]]]: ret = [] for f in [f for f in contract.functions if contract == f.contract_declarer]: - nodes = f.nodes - assembly_nodes = [n for n in nodes if self._contains_low_level_calls(n)] - if assembly_nodes: - ret.append((f, assembly_nodes)) + low_level_nodes = [ir.node for ir in f.low_level_calls] + if low_level_nodes: + ret.append((f, low_level_nodes)) return ret def _detect(self) -> List[Output]: diff --git a/slither/detectors/reentrancy/reentrancy.py b/slither/detectors/reentrancy/reentrancy.py index 8dd9aecc05..8f39c519d2 100644 --- a/slither/detectors/reentrancy/reentrancy.py +++ b/slither/detectors/reentrancy/reentrancy.py @@ -145,15 +145,16 @@ def analyze_node(self, node: Node, detector: "Reentrancy") -> bool: ) slithir_operations = [] # Add the state variables written in internal calls - for internal_call in node.internal_calls: + for ir in node.internal_calls: # Filter to Function, as internal_call can be a solidity call - if isinstance(internal_call, Function): - for internal_node in internal_call.all_nodes(): + function = ir.function + if isinstance(function, Function): + for internal_node in function.all_nodes(): for read in internal_node.state_variables_read: state_vars_read[read].add(internal_node) for write in internal_node.state_variables_written: state_vars_written[write].add(internal_node) - slithir_operations += internal_call.all_slithir_operations() + slithir_operations += function.all_slithir_operations() contains_call = False @@ -195,7 +196,7 @@ def does_not_bring_new_info(self, new_info: "AbstractState") -> bool: def _filter_if(node: Node) -> bool: """ - Check if the node is a condtional node where + Check if the node is a conditional node where there is an external call checked Heuristic: - The call is a IF node diff --git a/slither/detectors/statements/assert_state_change.py b/slither/detectors/statements/assert_state_change.py index 769d730b82..d70495365f 100644 --- a/slither/detectors/statements/assert_state_change.py +++ b/slither/detectors/statements/assert_state_change.py @@ -30,22 +30,22 @@ def detect_assert_state_change( # Loop for each function and modifier. for function in contract.functions_declared + list(contract.modifiers_declared): - for node in function.nodes: + for ir_call in function.internal_calls: # Detect assert() calls - if any(c.name == "assert(bool)" for c in node.internal_calls) and ( + if ir_call.function.name == "assert(bool)" and ( # Detect direct changes to state - node.state_variables_written + ir_call.node.state_variables_written or # Detect changes to state via function calls any( ir - for ir in node.irs + for ir in ir_call.node.irs if isinstance(ir, InternalCall) and ir.function and ir.function.state_variables_written ) ): - results.append((function, node)) + results.append((function, ir_call.node)) # Return the resulting set of nodes return results diff --git a/slither/detectors/statements/chronicle_unchecked_price.py b/slither/detectors/statements/chronicle_unchecked_price.py new file mode 100644 index 0000000000..47ad2ddc57 --- /dev/null +++ b/slither/detectors/statements/chronicle_unchecked_price.py @@ -0,0 +1,147 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output +from slither.slithir.operations import Binary, Assignment, Unpack, SolidityCall +from slither.core.variables import Variable +from slither.core.declarations.solidity_variables import SolidityFunction +from slither.core.cfg.node import Node + + +class ChronicleUncheckedPrice(AbstractDetector): + """ + Documentation: This detector finds calls to Chronicle oracle where the returned price is not checked + https://docs.chroniclelabs.org/Resources/FAQ/Oracles#how-do-i-check-if-an-oracle-becomes-inactive-gets-deprecated + """ + + ARGUMENT = "chronicle-unchecked-price" + HELP = "Detect when Chronicle price is not checked." + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#chronicle-unchecked-price" + + WIKI_TITLE = "Chronicle unchecked price" + WIKI_DESCRIPTION = "Chronicle oracle is used and the price returned is not checked to be valid. For more information https://docs.chroniclelabs.org/Resources/FAQ/Oracles#how-do-i-check-if-an-oracle-becomes-inactive-gets-deprecated." + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ +```solidity +contract C { + IChronicle chronicle; + + constructor(address a) { + chronicle = IChronicle(a); + } + + function bad() public { + uint256 price = chronicle.read(); + } +``` +The `bad` function gets the price from Chronicle by calling the read function however it does not check if the price is valid.""" + # endregion wiki_exploit_scenario + + WIKI_RECOMMENDATION = "Validate that the price returned by the oracle is valid." + + def _var_is_checked(self, nodes: List[Node], var_to_check: Variable) -> bool: + visited = set() + checked = False + + while nodes: + if checked: + break + next_node = nodes[0] + nodes = nodes[1:] + + for node_ir in next_node.all_slithir_operations(): + if isinstance(node_ir, Binary) and var_to_check in node_ir.read: + checked = True + break + # This case is for tryRead and tryReadWithAge + # if the isValid boolean is checked inside a require(isValid) + if ( + isinstance(node_ir, SolidityCall) + and node_ir.function + in ( + SolidityFunction("require(bool)"), + SolidityFunction("require(bool,string)"), + SolidityFunction("require(bool,error)"), + ) + and var_to_check in node_ir.read + ): + checked = True + break + + if next_node not in visited: + visited.add(next_node) + for son in next_node.sons: + if son not in visited: + nodes.append(son) + return checked + + # pylint: disable=too-many-nested-blocks,too-many-branches + def _detect(self) -> List[Output]: + results: List[Output] = [] + + for contract in self.compilation_unit.contracts_derived: + for target_contract, ir in sorted( + contract.all_high_level_calls, + key=lambda x: (x[1].node.node_id, x[1].node.function.full_name), + ): + if target_contract.name in ("IScribe", "IChronicle") and ir.function_name in ( + "read", + "tryRead", + "readWithAge", + "tryReadWithAge", + "latestAnswer", + "latestRoundData", + ): + found = False + if ir.function_name in ("read", "latestAnswer"): + # We need to iterate the IRs as we are not always sure that the following IR is the assignment + # for example in case of type conversion it isn't + for node_ir in ir.node.irs: + if isinstance(node_ir, Assignment): + possible_unchecked_variable_ir = node_ir.lvalue + found = True + break + elif ir.function_name in ("readWithAge", "tryRead", "tryReadWithAge"): + # We are interested in the first item of the tuple + # readWithAge : value + # tryRead/tryReadWithAge : isValid + for node_ir in ir.node.irs: + if isinstance(node_ir, Unpack) and node_ir.index == 0: + possible_unchecked_variable_ir = node_ir.lvalue + found = True + break + elif ir.function_name == "latestRoundData": + found = False + for node_ir in ir.node.irs: + if isinstance(node_ir, Unpack) and node_ir.index == 1: + possible_unchecked_variable_ir = node_ir.lvalue + found = True + break + + # If we did not find the variable assignment we know it's not checked + checked = ( + self._var_is_checked(ir.node.sons, possible_unchecked_variable_ir) + if found + else False + ) + + if not checked: + info: DETECTOR_INFO = [ + "Chronicle price is not checked to be valid in ", + ir.node.function, + "\n\t- ", + ir.node, + "\n", + ] + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/statements/controlled_delegatecall.py b/slither/detectors/statements/controlled_delegatecall.py index 32e59d6eb7..bf78b3bf98 100644 --- a/slither/detectors/statements/controlled_delegatecall.py +++ b/slither/detectors/statements/controlled_delegatecall.py @@ -8,20 +8,18 @@ DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import LowLevelCall from slither.utils.output import Output def controlled_delegatecall(function: FunctionContract) -> List[Node]: ret = [] - for node in function.nodes: - for ir in node.irs: - if isinstance(ir, LowLevelCall) and ir.function_name in [ - "delegatecall", - "callcode", - ]: - if is_tainted(ir.destination, function.contract): - ret.append(node) + for ir in function.low_level_calls: + if ir.function_name in [ + "delegatecall", + "callcode", + ]: + if is_tainted(ir.destination, function.contract): + ret.append(ir.node) return ret diff --git a/slither/detectors/statements/divide_before_multiply.py b/slither/detectors/statements/divide_before_multiply.py index e33477135d..6734fb239d 100644 --- a/slither/detectors/statements/divide_before_multiply.py +++ b/slither/detectors/statements/divide_before_multiply.py @@ -56,7 +56,7 @@ def is_assert(node: Node) -> bool: # Old Solidity code where using an internal 'assert(bool)' function # While we dont check that this function is correct, we assume it is # To avoid too many FP - if "assert(bool)" in [c.full_name for c in node.internal_calls]: + if "assert(bool)" in [ir.function.full_name for ir in node.internal_calls]: return True return False diff --git a/slither/detectors/statements/pyth_unchecked.py b/slither/detectors/statements/pyth_unchecked.py new file mode 100644 index 0000000000..959aee6a55 --- /dev/null +++ b/slither/detectors/statements/pyth_unchecked.py @@ -0,0 +1,79 @@ +from typing import List + +from slither.detectors.abstract_detector import ( + AbstractDetector, + DETECTOR_INFO, +) +from slither.utils.output import Output +from slither.slithir.operations import Member, Binary, Assignment + + +class PythUnchecked(AbstractDetector): + """ + Documentation: This detector finds deprecated Pyth function calls + """ + + # To be overriden in the derived class + PYTH_FUNCTIONS = [] + PYTH_FIELD = "" + + # pylint: disable=too-many-nested-blocks + def _detect(self) -> List[Output]: + results: List[Output] = [] + + for contract in self.compilation_unit.contracts_derived: + for target_contract, ir in contract.all_high_level_calls: + if target_contract.name == "IPyth" and ir.function_name in self.PYTH_FUNCTIONS: + # We know for sure the second IR in the node is an Assignment operation of the TMP variable. Example: + # Expression: price = pyth.getEmaPriceNoOlderThan(id,age) + # IRs: + # TMP_0(PythStructs.Price) = HIGH_LEVEL_CALL, dest:pyth(IPyth), function:getEmaPriceNoOlderThan, arguments:['id', 'age'] + # price(PythStructs.Price) := TMP_0(PythStructs.Price) + assert isinstance(ir.node.irs[1], Assignment) + return_variable = ir.node.irs[1].lvalue + checked = False + + possible_unchecked_variable_ir = None + nodes = ir.node.sons + visited = set() + while nodes: + if checked: + break + next_node = nodes[0] + nodes = nodes[1:] + + for node_ir in next_node.all_slithir_operations(): + # We are accessing the unchecked_var field of the returned Price struct + if ( + isinstance(node_ir, Member) + and node_ir.variable_left == return_variable + and node_ir.variable_right.name == self.PYTH_FIELD + ): + possible_unchecked_variable_ir = node_ir.lvalue + # We assume that if unchecked_var happens to be inside a binary operation is checked + if ( + isinstance(node_ir, Binary) + and possible_unchecked_variable_ir is not None + and possible_unchecked_variable_ir in node_ir.read + ): + checked = True + break + + if next_node not in visited: + visited.add(next_node) + for son in next_node.sons: + if son not in visited: + nodes.append(son) + + if not checked: + info: DETECTOR_INFO = [ + f"Pyth price {self.PYTH_FIELD} field is not checked in ", + ir.node.function, + "\n\t- ", + ir.node, + "\n", + ] + res = self.generate_result(info) + results.append(res) + + return results diff --git a/slither/detectors/statements/pyth_unchecked_confidence.py b/slither/detectors/statements/pyth_unchecked_confidence.py new file mode 100644 index 0000000000..2e99851a85 --- /dev/null +++ b/slither/detectors/statements/pyth_unchecked_confidence.py @@ -0,0 +1,50 @@ +from slither.detectors.abstract_detector import DetectorClassification +from slither.detectors.statements.pyth_unchecked import PythUnchecked + + +class PythUncheckedConfidence(PythUnchecked): + """ + Documentation: This detector finds when the confidence level of a Pyth price is not checked + """ + + ARGUMENT = "pyth-unchecked-confidence" + HELP = "Detect when the confidence level of a Pyth price is not checked" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-unchecked-confidence" + WIKI_TITLE = "Pyth unchecked confidence level" + WIKI_DESCRIPTION = "Detect when the confidence level of a Pyth price is not checked" + WIKI_RECOMMENDATION = "Check the confidence level of a Pyth price. Visit https://docs.pyth.network/price-feeds/best-practices#confidence-intervals for more information." + + WIKI_EXPLOIT_SCENARIO = """ +```solidity +import "@pythnetwork/pyth-sdk-solidity/IPyth.sol"; +import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol"; + +contract C { + IPyth pyth; + + constructor(IPyth _pyth) { + pyth = _pyth; + } + + function bad(bytes32 id, uint256 age) public { + PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age); + // Use price + } +} +``` +The function `A` uses the price without checking its confidence level. +""" + + PYTH_FUNCTIONS = [ + "getEmaPrice", + "getEmaPriceNoOlderThan", + "getEmaPriceUnsafe", + "getPrice", + "getPriceNoOlderThan", + "getPriceUnsafe", + ] + + PYTH_FIELD = "conf" diff --git a/slither/detectors/statements/pyth_unchecked_publishtime.py b/slither/detectors/statements/pyth_unchecked_publishtime.py new file mode 100644 index 0000000000..e3e2010d67 --- /dev/null +++ b/slither/detectors/statements/pyth_unchecked_publishtime.py @@ -0,0 +1,52 @@ +from slither.detectors.abstract_detector import DetectorClassification +from slither.detectors.statements.pyth_unchecked import PythUnchecked + + +class PythUncheckedPublishTime(PythUnchecked): + """ + Documentation: This detector finds when the publishTime of a Pyth price is not checked + """ + + ARGUMENT = "pyth-unchecked-publishtime" + HELP = "Detect when the publishTime of a Pyth price is not checked" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = ( + "https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-unchecked-publishtime" + ) + WIKI_TITLE = "Pyth unchecked publishTime" + WIKI_DESCRIPTION = "Detect when the publishTime of a Pyth price is not checked" + WIKI_RECOMMENDATION = "Check the publishTime of a Pyth price." + + WIKI_EXPLOIT_SCENARIO = """ +```solidity +import "@pythnetwork/pyth-sdk-solidity/IPyth.sol"; +import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol"; + +contract C { + IPyth pyth; + + constructor(IPyth _pyth) { + pyth = _pyth; + } + + function bad(bytes32 id) public { + PythStructs.Price memory price = pyth.getEmaPriceUnsafe(id); + // Use price + } +} +``` +The function `A` uses the price without checking its `publishTime` coming from the `getEmaPriceUnsafe` function. +""" + + PYTH_FUNCTIONS = [ + "getEmaPrice", + # "getEmaPriceNoOlderThan", + "getEmaPriceUnsafe", + "getPrice", + # "getPriceNoOlderThan", + "getPriceUnsafe", + ] + + PYTH_FIELD = "publishTime" diff --git a/slither/detectors/statements/return_bomb.py b/slither/detectors/statements/return_bomb.py index 8b6cd07a29..6d7052cf40 100644 --- a/slither/detectors/statements/return_bomb.py +++ b/slither/detectors/statements/return_bomb.py @@ -9,7 +9,7 @@ DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import LowLevelCall, HighLevelCall +from slither.slithir.operations import HighLevelCall from slither.analyses.data_dependency.data_dependency import is_tainted from slither.utils.output import Output @@ -71,34 +71,31 @@ def is_dynamic_type(ty: Type) -> bool: def get_nodes_for_function(self, function: Function, contract: Contract) -> List[Node]: nodes = [] - for node in function.nodes: - for ir in node.irs: - if isinstance(ir, (HighLevelCall, LowLevelCall)): - if not is_tainted(ir.destination, contract): # type:ignore - # Only interested if the target address is controlled/tainted - continue - - if isinstance(ir, HighLevelCall) and isinstance(ir.function, Function): - # in normal highlevel calls return bombs are _possible_ - # if the return type is dynamic and the caller tries to copy and decode large data - has_dyn = False - if ir.function.return_type: - has_dyn = any( - self.is_dynamic_type(ty) for ty in ir.function.return_type - ) - - if not has_dyn: - continue - - # If a gas budget was specified then the - # user may not know about the return bomb - if ir.call_gas is None: - # if a gas budget was NOT specified then the caller - # may already suspect the call may spend all gas? - continue - - nodes.append(node) - # TODO: check that there is some state change after the call + + for ir in [ir for _, ir in function.high_level_calls] + function.low_level_calls: + if not is_tainted(ir.destination, contract): # type:ignore + # Only interested if the target address is controlled/tainted + continue + + if isinstance(ir, HighLevelCall) and isinstance(ir.function, Function): + # in normal highlevel calls return bombs are _possible_ + # if the return type is dynamic and the caller tries to copy and decode large data + has_dyn = False + if ir.function.return_type: + has_dyn = any(self.is_dynamic_type(ty) for ty in ir.function.return_type) + + if not has_dyn: + continue + + # If a gas budget was specified then the + # user may not know about the return bomb + if ir.call_gas is None: + # if a gas budget was NOT specified then the caller + # may already suspect the call may spend all gas? + continue + + nodes.append(ir.node) + # TODO: check that there is some state change after the call return nodes diff --git a/slither/detectors/statements/unprotected_upgradeable.py b/slither/detectors/statements/unprotected_upgradeable.py index d25aff187d..aeb785da36 100644 --- a/slither/detectors/statements/unprotected_upgradeable.py +++ b/slither/detectors/statements/unprotected_upgradeable.py @@ -7,23 +7,28 @@ DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import LowLevelCall, SolidityCall from slither.utils.output import Output def _can_be_destroyed(contract: Contract) -> List[Function]: targets = [] for f in contract.functions_entry_points: - for ir in f.all_slithir_operations(): - if ( - isinstance(ir, LowLevelCall) and ir.function_name in ["delegatecall", "codecall"] - ) or ( - isinstance(ir, SolidityCall) - and ir.function - in [SolidityFunction("suicide(address)"), SolidityFunction("selfdestruct(address)")] - ): + found = False + for ir in f.all_low_level_calls(): + if ir.function_name in ["delegatecall", "codecall"]: targets.append(f) + found = True break + + if not found: + for ir in f.all_solidity_calls(): + if ir.function in [ + SolidityFunction("suicide(address)"), + SolidityFunction("selfdestruct(address)"), + ]: + targets.append(f) + break + return targets @@ -35,8 +40,8 @@ def _has_initializing_protection(functions: List[Function]) -> bool: for m in f.modifiers: if m.name == "initializer": return True - for ifc in f.all_internal_calls(): - if ifc.name == "_disableInitializers": + for ir in f.all_internal_calls(): + if ir.function.name == "_disableInitializers": return True # to avoid future FPs in different modifier + function naming implementations, we can also implement a broader check for state var "_initialized" being written to in the constructor diff --git a/slither/detectors/variables/unchanged_state_variables.py b/slither/detectors/variables/unchanged_state_variables.py index 5771d96303..64c4c350f6 100644 --- a/slither/detectors/variables/unchanged_state_variables.py +++ b/slither/detectors/variables/unchanged_state_variables.py @@ -92,7 +92,7 @@ def detect(self) -> None: variables = [] functions = [] - variables.append(c.stored_state_variables) + variables.append(c.storage_variables_ordered) functions.append(c.all_functions_called) valid_candidates: Set[StateVariable] = { diff --git a/slither/detectors/variables/var_read_using_this.py b/slither/detectors/variables/var_read_using_this.py index 537eecf8a3..1e4787e363 100644 --- a/slither/detectors/variables/var_read_using_this.py +++ b/slither/detectors/variables/var_read_using_this.py @@ -7,7 +7,6 @@ DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations.high_level_call import HighLevelCall from slither.utils.output import Output @@ -54,13 +53,11 @@ def _detect(self) -> List[Output]: @staticmethod def _detect_var_read_using_this(func: Function) -> List[Node]: results: List[Node] = [] - for node in func.nodes: - for ir in node.irs: - if isinstance(ir, HighLevelCall): - if ( - ir.destination == SolidityVariable("this") - and ir.is_static_call() - and ir.function.visibility == "public" - ): - results.append(node) + for _, ir in func.high_level_calls: + if ( + ir.destination == SolidityVariable("this") + and ir.is_static_call() + and ir.function.visibility == "public" + ): + results.append(ir.node) return sorted(results, key=lambda x: x.node_id) diff --git a/slither/printers/all_printers.py b/slither/printers/all_printers.py index 3edd5325b6..034578f528 100644 --- a/slither/printers/all_printers.py +++ b/slither/printers/all_printers.py @@ -24,3 +24,4 @@ from .summary.declaration import Declaration from .functions.dominator import Dominator from .summary.martin import Martin +from .summary.cheatcodes import CheatcodePrinter diff --git a/slither/printers/call/call_graph.py b/slither/printers/call/call_graph.py index 38225e6d7a..377f69ad27 100644 --- a/slither/printers/call/call_graph.py +++ b/slither/printers/call/call_graph.py @@ -10,6 +10,7 @@ from slither.core.declarations import Contract, FunctionContract from slither.core.declarations.function import Function +from slither.slithir.operations import HighLevelCall, InternalCall from slither.core.declarations.solidity_variables import SolidityFunction from slither.core.variables.variable import Variable from slither.printers.abstract_printer import AbstractPrinter @@ -49,26 +50,26 @@ def _node(node: str, label: Optional[str] = None) -> str: def _process_internal_call( contract: Contract, function: Function, - internal_call: Union[Function, SolidityFunction], + internal_call: InternalCall, contract_calls: Dict[Contract, Set[str]], solidity_functions: Set[str], solidity_calls: Set[str], ) -> None: - if isinstance(internal_call, (Function)): + if isinstance(internal_call.function, (Function)): contract_calls[contract].add( _edge( _function_node(contract, function), - _function_node(contract, internal_call), + _function_node(contract, internal_call.function), ) ) - elif isinstance(internal_call, (SolidityFunction)): + elif isinstance(internal_call.function, (SolidityFunction)): solidity_functions.add( - _node(_solidity_function_node(internal_call)), + _node(_solidity_function_node(internal_call.function)), ) solidity_calls.add( _edge( _function_node(contract, function), - _solidity_function_node(internal_call), + _solidity_function_node(internal_call.function), ) ) @@ -112,29 +113,29 @@ def _render_solidity_calls(solidity_functions: Set[str], solidity_calls: Set[str def _process_external_call( contract: Contract, function: Function, - external_call: Tuple[Contract, Union[Function, Variable]], + external_call: Tuple[Contract, HighLevelCall], contract_functions: Dict[Contract, Set[str]], external_calls: Set[str], all_contracts: Set[Contract], ) -> None: - external_contract, external_function = external_call + external_contract, ir = external_call if not external_contract in all_contracts: return # add variable as node to respective contract - if isinstance(external_function, (Variable)): + if isinstance(ir.function, (Variable)): contract_functions[external_contract].add( _node( - _function_node(external_contract, external_function), - external_function.name, + _function_node(external_contract, ir.function), + ir.function.name, ) ) external_calls.add( _edge( _function_node(contract, function), - _function_node(external_contract, external_function), + _function_node(external_contract, ir.function), ) ) @@ -256,6 +257,8 @@ def output(self, filename: str) -> Output: } content = "\n".join( ["strict digraph {"] + + ['rankdir="LR"'] + + ["node [shape=box]"] + [_process_functions(list(all_functions_as_dict.values()))] + ["}"] ) @@ -267,7 +270,11 @@ def output(self, filename: str) -> Output: with open(derived_output_filename, "w", encoding="utf8") as f: info += f"Call Graph: {derived_output_filename}\n" content = "\n".join( - ["strict digraph {"] + [_process_functions(derived_contract.functions)] + ["}"] + ["strict digraph {"] + + ['rankdir="LR"'] + + ["node [shape=box]"] + + [_process_functions(derived_contract.functions)] + + ["}"] ) f.write(content) results.append((derived_output_filename, content)) diff --git a/slither/printers/functions/authorization.py b/slither/printers/functions/authorization.py index 32efeaabeb..288392a468 100644 --- a/slither/printers/functions/authorization.py +++ b/slither/printers/functions/authorization.py @@ -19,7 +19,11 @@ class PrinterWrittenVariablesAndAuthorization(AbstractPrinter): @staticmethod def get_msg_sender_checks(function: Function) -> List[str]: all_functions = ( - [f for f in function.all_internal_calls() if isinstance(f, Function)] + [ + ir.function + for ir in function.all_internal_calls() + if isinstance(ir.function, Function) + ] + [function] + [m for m in function.modifiers if isinstance(m, Function)] ) diff --git a/slither/printers/guidance/echidna.py b/slither/printers/guidance/echidna.py index 0c47fa0f98..48e40a90f3 100644 --- a/slither/printers/guidance/echidna.py +++ b/slither/printers/guidance/echidna.py @@ -13,6 +13,7 @@ from slither.core.expressions import NewContract from slither.core.slither_core import SlitherCore from slither.core.solidity_types import TypeAlias +from slither.core.source_mapping.source_mapping import SourceMapping from slither.core.variables.state_variable import StateVariable from slither.core.variables.variable import Variable from slither.printers.abstract_printer import AbstractPrinter @@ -138,17 +139,12 @@ def _extract_assert(contracts: List[Contract]) -> Dict[str, Dict[str, List[Dict] """ ret: Dict[str, Dict[str, List[Dict]]] = {} for contract in contracts: - functions_using_assert = [] # Dict[str, List[Dict]] = defaultdict(list) + functions_using_assert: Dict[str, List[Dict]] = defaultdict(list) for f in contract.functions_entry_points: - for v in f.all_solidity_calls(): - if v == SolidityFunction("assert(bool)"): - functions_using_assert.append(_get_name(f)) - break - # Revert https://github.com/crytic/slither/pull/2105 until format is supported by echidna. - # for node in f.all_nodes(): - # if SolidityFunction("assert(bool)") in node.solidity_calls and node.source_mapping: - # func_name = _get_name(f) - # functions_using_assert[func_name].append(node.source_mapping.to_json()) + for ir in f.all_solidity_calls(): + if ir.function == SolidityFunction("assert(bool)") and ir.node.source_mapping: + func_name = _get_name(f) + functions_using_assert[func_name].append(ir.node.source_mapping.to_json()) if functions_using_assert: ret[contract.name] = functions_using_assert return ret @@ -156,7 +152,7 @@ def _extract_assert(contracts: List[Contract]) -> Dict[str, Dict[str, List[Dict] # Create a named tuple that is serialization in json def json_serializable(cls): - # pylint: disable=unnecessary-comprehension + # pylint: disable=unnecessary-comprehension,unnecessary-dunder-call # TODO: the next line is a quick workaround to prevent pylint from crashing # It can be removed once https://github.com/PyCQA/pylint/pull/3810 is merged my_super = super @@ -179,7 +175,66 @@ class ConstantValue(NamedTuple): # pylint: disable=inherit-non-class,too-few-pu type: str -def _extract_constants_from_irs( # pylint: disable=too-many-branches,too-many-nested-blocks +def _extract_constant_from_read( + ir: Operation, + r: SourceMapping, + all_cst_used: List[ConstantValue], + all_cst_used_in_binary: Dict[str, List[ConstantValue]], + context_explored: Set[Node], +) -> None: + var_read = r.points_to_origin if isinstance(r, ReferenceVariable) else r + # Do not report struct_name in a.struct_name + if isinstance(ir, Member): + return + if isinstance(var_read, Variable) and var_read.is_constant: + # In case of type conversion we use the destination type + if isinstance(ir, TypeConversion): + if isinstance(ir.type, TypeAlias): + value_type = ir.type.type + else: + value_type = ir.type + else: + value_type = var_read.type + try: + value = ConstantFolding(var_read.expression, value_type).result() + all_cst_used.append(ConstantValue(str(value), str(value_type))) + except NotConstant: + pass + if isinstance(var_read, Constant): + all_cst_used.append(ConstantValue(str(var_read.value), str(var_read.type))) + if isinstance(var_read, StateVariable): + if var_read.node_initialization: + if var_read.node_initialization.irs: + if var_read.node_initialization in context_explored: + return + context_explored.add(var_read.node_initialization) + _extract_constants_from_irs( + var_read.node_initialization.irs, + all_cst_used, + all_cst_used_in_binary, + context_explored, + ) + + +def _extract_constant_from_binary( + ir: Binary, + all_cst_used: List[ConstantValue], + all_cst_used_in_binary: Dict[str, List[ConstantValue]], +): + for r in ir.read: + if isinstance(r, Constant): + all_cst_used_in_binary[str(ir.type)].append(ConstantValue(str(r.value), str(r.type))) + if isinstance(ir.variable_left, Constant) or isinstance(ir.variable_right, Constant): + if ir.lvalue: + try: + type_ = ir.lvalue.type + cst = ConstantFolding(ir.expression, type_).result() + all_cst_used.append(ConstantValue(str(cst.value), str(type_))) + except NotConstant: + pass + + +def _extract_constants_from_irs( irs: List[Operation], all_cst_used: List[ConstantValue], all_cst_used_in_binary: Dict[str, List[ConstantValue]], @@ -187,21 +242,7 @@ def _extract_constants_from_irs( # pylint: disable=too-many-branches,too-many-n ) -> None: for ir in irs: if isinstance(ir, Binary): - for r in ir.read: - if isinstance(r, Constant): - all_cst_used_in_binary[str(ir.type)].append( - ConstantValue(str(r.value), str(r.type)) - ) - if isinstance(ir.variable_left, Constant) or isinstance( - ir.variable_right, Constant - ): - if ir.lvalue: - try: - type_ = ir.lvalue.type - cst = ConstantFolding(ir.expression, type_).result() - all_cst_used.append(ConstantValue(str(cst.value), str(type_))) - except NotConstant: - pass + _extract_constant_from_binary(ir, all_cst_used, all_cst_used_in_binary) if isinstance(ir, TypeConversion): if isinstance(ir.variable, Constant): if isinstance(ir.type, TypeAlias): @@ -222,24 +263,9 @@ def _extract_constants_from_irs( # pylint: disable=too-many-branches,too-many-n except ValueError: # index could fail; should never happen in working solidity code pass for r in ir.read: - var_read = r.points_to_origin if isinstance(r, ReferenceVariable) else r - # Do not report struct_name in a.struct_name - if isinstance(ir, Member): - continue - if isinstance(var_read, Constant): - all_cst_used.append(ConstantValue(str(var_read.value), str(var_read.type))) - if isinstance(var_read, StateVariable): - if var_read.node_initialization: - if var_read.node_initialization.irs: - if var_read.node_initialization in context_explored: - continue - context_explored.add(var_read.node_initialization) - _extract_constants_from_irs( - var_read.node_initialization.irs, - all_cst_used, - all_cst_used_in_binary, - context_explored, - ) + _extract_constant_from_read( + ir, r, all_cst_used, all_cst_used_in_binary, context_explored + ) def _extract_constants( diff --git a/slither/printers/summary/cheatcodes.py b/slither/printers/summary/cheatcodes.py new file mode 100644 index 0000000000..857ec2da9f --- /dev/null +++ b/slither/printers/summary/cheatcodes.py @@ -0,0 +1,74 @@ +""" + Cheatcode printer + + This printer prints the usage of cheatcode in the code. +""" +from slither.printers.abstract_printer import AbstractPrinter +from slither.slithir.operations import HighLevelCall +from slither.utils import output + + +class CheatcodePrinter(AbstractPrinter): + + ARGUMENT = "cheatcode" + + HELP = """ + Print the usage of (Foundry) cheatcodes in the code. + For the complete list of Cheatcodes, see https://book.getfoundry.sh/cheatcodes/ + """ + + WIKI = "https://github.com/trailofbits/slither/wiki/Printer-documentation#cheatcode" + + def output(self, filename: str) -> output.Output: + + info: str = "" + + try: + vm = self.slither.get_contract_from_name("Vm").pop() + except IndexError: + return output.Output("No contract named VM found") + + for contract in self.slither.contracts_derived: + # Check that the IS_TEST variable is set. (Only works for Foundry) + is_test_var = contract.variables_as_dict.get("IS_TEST", None) + is_test = False + if is_test_var is not None: + try: + is_test = is_test_var.expression.value == "true" + except AttributeError: + pass + + if not is_test: + continue + + found_contract: bool = False + contract_info: str = "" + for func in contract.functions_declared: + function_info = f"\t{func}\n" + found_function: bool = False + for node in func.nodes: + for op in node.all_slithir_operations(): + if ( + isinstance(op, HighLevelCall) + and op.function.contract == vm + and op.function.visibility == "external" + ): + found_function = True + function_info += ( + f"\t\t{op.function.name} - ({node.source_mapping.to_detailed_str()})\n" + f"\t\t{node.expression}\n\n" + ) + + if found_function: + if found_contract is False: + contract_info = f"{contract} ({contract.source_mapping.filename.short})\n" + found_contract = True + + contract_info += function_info + + if found_contract: + info += contract_info + + self.info(info) + res = output.Output(info) + return res diff --git a/slither/printers/summary/modifier_calls.py b/slither/printers/summary/modifier_calls.py index cd6c4062e3..225376a3cf 100644 --- a/slither/printers/summary/modifier_calls.py +++ b/slither/printers/summary/modifier_calls.py @@ -29,12 +29,12 @@ def output(self, _filename): table = MyPrettyTable(["Function", "Modifiers"]) for function in contract.functions: modifiers = function.modifiers - for call in function.all_internal_calls(): - if isinstance(call, Function): - modifiers += call.modifiers - for (_, call) in function.all_library_calls(): - if isinstance(call, Function): - modifiers += call.modifiers + for ir in function.all_internal_calls(): + if isinstance(ir.function, Function): + modifiers += ir.function.modifiers + for ir in function.all_library_calls(): + if isinstance(ir.function, Function): + modifiers += ir.function.modifiers table.add_row([function.name, sorted([m.name for m in set(modifiers)])]) txt += "\n" + str(table) self.info(txt) diff --git a/slither/printers/summary/require_calls.py b/slither/printers/summary/require_calls.py index 7823de1600..ae79e9ed66 100644 --- a/slither/printers/summary/require_calls.py +++ b/slither/printers/summary/require_calls.py @@ -11,6 +11,7 @@ SolidityFunction("assert(bool)"), SolidityFunction("require(bool)"), SolidityFunction("require(bool,string)"), + SolidityFunction("require(bool,error)"), ] diff --git a/slither/printers/summary/variable_order.py b/slither/printers/summary/variable_order.py index 0d8ce2612c..fb19e39857 100644 --- a/slither/printers/summary/variable_order.py +++ b/slither/printers/summary/variable_order.py @@ -27,10 +27,17 @@ def output(self, _filename: str) -> Output: for contract in self.slither.contracts_derived: txt += f"\n{contract.name}:\n" - table = MyPrettyTable(["Name", "Type", "Slot", "Offset"]) - for variable in contract.stored_state_variables_ordered: + table = MyPrettyTable(["Name", "Type", "Slot", "Offset", "State"]) + for variable in contract.storage_variables_ordered: slot, offset = contract.compilation_unit.storage_layout_of(contract, variable) - table.add_row([variable.canonical_name, str(variable.type), slot, offset]) + table.add_row( + [variable.canonical_name, str(variable.type), slot, offset, "Storage"] + ) + for variable in contract.transient_variables_ordered: + slot, offset = contract.compilation_unit.storage_layout_of(contract, variable) + table.add_row( + [variable.canonical_name, str(variable.type), slot, offset, "Transient"] + ) all_tables.append((contract.name, table)) txt += str(table) + "\n" diff --git a/slither/printers/summary/when_not_paused.py b/slither/printers/summary/when_not_paused.py index aaeeeacec2..fc96268ef3 100644 --- a/slither/printers/summary/when_not_paused.py +++ b/slither/printers/summary/when_not_paused.py @@ -11,8 +11,8 @@ def _use_modifier(function: Function, modifier_name: str = "whenNotPaused") -> bool: - for internal_call in function.all_internal_calls(): - if isinstance(internal_call, SolidityFunction): + for ir in function.all_internal_calls(): + if isinstance(ir, function, SolidityFunction): continue if any(modifier.name == modifier_name for modifier in function.modifiers): return True diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 7d8aa543bf..cc27eb286d 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -36,7 +36,6 @@ ) from slither.core.solidity_types.type import Type from slither.core.solidity_types.type_alias import TypeAliasTopLevel, TypeAlias -from slither.core.variables.function_type_variable import FunctionTypeVariable from slither.core.variables.state_variable import StateVariable from slither.core.variables.variable import Variable from slither.slithir.exceptions import SlithIRError @@ -81,7 +80,7 @@ from slither.slithir.tmp_operations.tmp_new_structure import TmpNewStructure from slither.slithir.variables import Constant, ReferenceVariable, TemporaryVariable from slither.slithir.variables import TupleVariable -from slither.utils.function import get_function_id +from slither.utils.function import get_function_id, get_event_id from slither.utils.type import export_nested_types_from_variable from slither.utils.using_for import USING_FOR from slither.visitors.slithir.expression_to_slithir import ExpressionToSlithIR @@ -279,20 +278,6 @@ def is_temporary(ins: Operation) -> bool: ) -def _make_function_type(func: Function) -> FunctionType: - parameters = [] - returns = [] - for parameter in func.parameters: - v = FunctionTypeVariable() - v.name = parameter.name - parameters.append(v) - for return_var in func.returns: - v = FunctionTypeVariable() - v.name = return_var.name - returns.append(v) - return FunctionType(parameters, returns) - - # endregion ################################################################################### ################################################################################### @@ -395,7 +380,7 @@ def get_declared_param_names( InternalDynamicCall, EventCall, ] -) -> Optional[List[str]]: +) -> Optional[List[List[str]]]: """ Given a call operation, return the list of parameter names, in the order listed in the function declaration. @@ -403,26 +388,31 @@ def get_declared_param_names( ins - The call instruction #### Possible Returns - List[str] - - A list of the parameters in declaration order + List[List[str]] - + A list of list of parameters in declaration order. Note only if it's a Function there can be multiple list None - Workaround: Unable to obtain list of parameters in declaration order """ if isinstance(ins, NewStructure): - return [x.name for x in ins.structure.elems_ordered if not isinstance(x.type, MappingType)] + return [ + [x.name for x in ins.structure.elems_ordered if not isinstance(x.type, MappingType)] + ] if isinstance(ins, (InternalCall, LibraryCall, HighLevelCall)): if isinstance(ins.function, Function): - return [p.name for p in ins.function.parameters] + res = [[p.name for p in ins.function.parameters]] + for f in ins.function.overrides: + res.append([p.name for p in f.parameters]) + return res return None if isinstance(ins, InternalDynamicCall): - return [p.name for p in ins.function_type.params] + return [[p.name for p in ins.function_type.params]] assert isinstance(ins, (EventCall, NewContract)) return None def reorder_arguments( - args: List[Variable], call_names: List[str], decl_names: List[str] + args: List[Variable], call_names: List[str], decl_names: List[List[str]] ) -> List[Variable]: """ Reorder named struct constructor arguments so that they match struct declaration ordering rather @@ -434,16 +424,35 @@ def reorder_arguments( names - Parameter names in call order decl_names - - Parameter names in declaration order + List of list of parameter names in declaration order #### Returns Reordered arguments to constructor call, now in declaration order """ assert len(args) == len(call_names) - assert len(call_names) == len(decl_names) + for names in decl_names: + assert len(call_names) == len(names) args_ret = [] - for n in decl_names: - ind = call_names.index(n) + index_seen = [] # Will have the correct index order + + # We search for declaration of parameters that is fully compatible with the call arguments + # that is why if a arg name is not present in the call name we break and clear index_seen + # Known issue if the overridden function reuse the same parameters' name but in different positions + for names in decl_names: + if len(index_seen) == len(args): + break + + for n in names: + try: + ind = call_names.index(n) + if ind in index_seen: + continue + except ValueError: + index_seen.clear() + break + index_seen.append(ind) + + for ind in index_seen: args_ret.append(args[ind]) return args_ret @@ -793,12 +802,29 @@ def propagate_types(ir: Operation, node: "Node"): # pylint: disable=too-many-lo assignment.set_node(ir.node) assignment.lvalue.set_type(ElementaryType("bytes4")) return assignment - if ir.variable_right == "selector" and isinstance( - ir.variable_left.type, (Function) + if ir.variable_right == "selector" and isinstance(ir.variable_left, (Event)): + # the event selector returns a bytes32, which is different from the error/function selector + # which returns a bytes4 + assignment = Assignment( + ir.lvalue, + Constant( + str(get_event_id(ir.variable_left.full_name)), ElementaryType("bytes32") + ), + ElementaryType("bytes32"), + ) + assignment.set_expression(ir.expression) + assignment.set_node(ir.node) + assignment.lvalue.set_type(ElementaryType("bytes32")) + return assignment + if ir.variable_right == "selector" and ( + isinstance(ir.variable_left.type, (Function)) ): assignment = Assignment( ir.lvalue, - Constant(str(get_function_id(ir.variable_left.type.full_name))), + Constant( + str(get_function_id(ir.variable_left.type.full_name)), + ElementaryType("bytes4"), + ), ElementaryType("bytes4"), ) assignment.set_expression(ir.expression) @@ -826,10 +852,9 @@ def propagate_types(ir: Operation, node: "Node"): # pylint: disable=too-many-lo targeted_function = next( (x for x in ir_func.contract.functions if x.name == str(ir.variable_right)) ) - t = _make_function_type(targeted_function) - ir.lvalue.set_type(t) + ir.lvalue.set_type(targeted_function) elif isinstance(left, (Variable, SolidityVariable)): - t = ir.variable_left.type + t = left.type elif isinstance(left, (Contract, Enum, Structure)): t = UserDefinedType(left) # can be None due to temporary operation @@ -846,10 +871,10 @@ def propagate_types(ir: Operation, node: "Node"): # pylint: disable=too-many-lo ir.lvalue.set_type(elems[elem].type) else: assert isinstance(type_t, Contract) - # Allow type propagtion as a Function + # Allow type propagation as a Function # Only for reference variables # This allows to track the selector keyword - # We dont need to check for function collision, as solc prevents the use of selector + # We don't need to check for function collision, as solc prevents the use of selector # if there are multiple functions with the same name f = next( (f for f in type_t.functions if f.name == ir.variable_right), @@ -858,7 +883,7 @@ def propagate_types(ir: Operation, node: "Node"): # pylint: disable=too-many-lo if f: ir.lvalue.set_type(f) else: - # Allow propgation for variable access through contract's name + # Allow propagation for variable access through contract's name # like Base_contract.my_variable v = next( ( @@ -995,7 +1020,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call, # Support for library call where the parameter is a function # We could merge this with the standard library handling # Except that we will have some troubles with using_for - # As the type of the funciton will not match function() + # As the type of the function will not match function() # Additionally we do not have a correct view on the parameters of the tmpcall # At this level # @@ -1237,7 +1262,7 @@ def convert_to_low_level( ) -> Union[Send, LowLevelCall, Transfer,]: """ Convert to a transfer/send/or low level call - The funciton assume to receive a correct IR + The function assume to receive a correct IR The checks must be done by the caller Must be called after can_be_low_level diff --git a/slither/slithir/operations/assignment.py b/slither/slithir/operations/assignment.py index 1f29ceb7b1..ab6637faa0 100644 --- a/slither/slithir/operations/assignment.py +++ b/slither/slithir/operations/assignment.py @@ -45,10 +45,19 @@ def rvalue(self) -> Union[RVALUE, Function, TupleVariable]: def __str__(self) -> str: lvalue = self.lvalue + + # When rvalues are functions, we want to properly display their return type + # Fix: https://github.com/crytic/slither/issues/2266 + if isinstance(self.rvalue.type, list): + rvalue_type = ",".join(f"{rvalue_type}" for rvalue_type in self.rvalue.type) + else: + rvalue_type = f"{self.rvalue.type}" + assert lvalue if lvalue and isinstance(lvalue, ReferenceVariable): points = lvalue.points_to while isinstance(points, ReferenceVariable): points = points.points_to - return f"{lvalue}({lvalue.type}) (->{points}) := {self.rvalue}({self.rvalue.type})" - return f"{lvalue}({lvalue.type}) := {self.rvalue}({self.rvalue.type})" + return f"{lvalue}({lvalue.type}) (->{points}) := {self.rvalue}({rvalue_type})" + + return f"{lvalue}({lvalue.type}) := {self.rvalue}({rvalue_type})" diff --git a/slither/slithir/operations/member.py b/slither/slithir/operations/member.py index 0942813cfc..55979572c5 100644 --- a/slither/slithir/operations/member.py +++ b/slither/slithir/operations/member.py @@ -1,5 +1,5 @@ from typing import List, Union -from slither.core.declarations import Contract, Function +from slither.core.declarations import Contract, Function, Event from slither.core.declarations.custom_error import CustomError from slither.core.declarations.enum import Enum from slither.core.declarations.solidity_import_placeholder import SolidityImportPlaceHolder @@ -33,14 +33,29 @@ def __init__( # Can be an ElementaryType because of bytes.concat, string.concat assert is_valid_rvalue(variable_left) or isinstance( variable_left, - (Contract, Enum, Function, CustomError, SolidityImportPlaceHolder, ElementaryType), + ( + Contract, + Enum, + Function, + Event, + CustomError, + SolidityImportPlaceHolder, + ElementaryType, + ), ) assert isinstance(variable_right, Constant) assert isinstance(result, ReferenceVariable) super().__init__() self._variable_left: Union[ - RVALUE, Contract, Enum, Function, CustomError, SolidityImportPlaceHolder, ElementaryType + RVALUE, + Contract, + Enum, + Function, + Event, + CustomError, + SolidityImportPlaceHolder, + ElementaryType, ] = variable_left self._variable_right = variable_right self._lvalue = result diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 1ccdc57602..06fc03b7a0 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -350,7 +350,7 @@ def parse_state_variables(self) -> None: if v.visibility != "private" } ) - self._contract.add_variables_ordered( + self._contract.add_state_variables_ordered( [ var for var in father.state_variables_ordered @@ -370,7 +370,7 @@ def parse_state_variables(self) -> None: if var_parser.reference_id is not None: self._contract.state_variables_by_ref_id[var_parser.reference_id] = var self._contract.variables_as_dict[var.name] = var - self._contract.add_variables_ordered([var]) + self._contract.add_state_variables_ordered([var]) def _parse_modifier(self, modifier_data: Dict) -> None: modif = Modifier(self._contract.compilation_unit) diff --git a/slither/solc_parsing/solidity_types/type_parsing.py b/slither/solc_parsing/solidity_types/type_parsing.py index 6ca015127e..cb0b1125db 100644 --- a/slither/solc_parsing/solidity_types/type_parsing.py +++ b/slither/solc_parsing/solidity_types/type_parsing.py @@ -293,7 +293,7 @@ def parse_type( scope = custom_error.contract.file_scope sl = caller_context.compilation_unit - next_context = caller_context.slither_parser + next_context = caller_context structures_direct_access = list(scope.structures.values()) all_structuress = [c.structures for c in scope.contracts.values()] all_structures = [item for sublist in all_structuress for item in sublist] diff --git a/slither/solc_parsing/variables/state_variable.py b/slither/solc_parsing/variables/state_variable.py index a9c0ff730b..227a84c61b 100644 --- a/slither/solc_parsing/variables/state_variable.py +++ b/slither/solc_parsing/variables/state_variable.py @@ -13,3 +13,18 @@ def underlying_variable(self) -> StateVariable: # Todo: Not sure how to overcome this with mypy assert isinstance(self._variable, StateVariable) return self._variable + + def _analyze_variable_attributes(self, attributes: Dict) -> None: + """ + Variable Location + Can be default or transient + """ + if "storageLocation" in attributes: + self.underlying_variable.set_location(attributes["storageLocation"]) + else: + # We don't have to support legacy ast + # as transient location was added in 0.8.28 + # and we know it must be default + self.underlying_variable.set_location("default") + + super()._analyze_variable_attributes(attributes) diff --git a/slither/tools/flattening/flattening.py b/slither/tools/flattening/flattening.py index 9cb2abc3f1..182333d3fc 100644 --- a/slither/tools/flattening/flattening.py +++ b/slither/tools/flattening/flattening.py @@ -294,10 +294,9 @@ def _export_list_used_contracts( # pylint: disable=too-many-branches self._export_list_used_contracts(inherited, exported, list_contract, list_top_level) # Find all the external contracts called - externals = contract.all_library_calls + contract.all_high_level_calls - # externals is a list of (contract, function) + # High level calls already includes library calls # We also filter call to itself to avoid infilite loop - externals = list({e[0] for e in externals if e[0] != contract}) + externals = list({e[0] for e in contract.all_high_level_calls if e[0] != contract}) for inherited in externals: self._export_list_used_contracts(inherited, exported, list_contract, list_top_level) diff --git a/slither/tools/mutator/__main__.py b/slither/tools/mutator/__main__.py index dea11676a6..d97806e286 100644 --- a/slither/tools/mutator/__main__.py +++ b/slither/tools/mutator/__main__.py @@ -77,15 +77,6 @@ def parse_args() -> argparse.Namespace: default=False, ) - # to print just all the mutants - parser.add_argument( - "-vv", - "--very-verbose", - help="log mutants that are caught, uncaught, and fail to compile. And more!", - action="store_true", - default=False, - ) - # select list of mutators to run parser.add_argument( "--mutators-to-run", @@ -159,7 +150,6 @@ def main() -> None: # pylint: disable=too-many-statements,too-many-branches,too timeout: Optional[int] = args.timeout solc_remappings: Optional[str] = args.solc_remaps verbose: Optional[bool] = args.verbose - very_verbose: Optional[bool] = args.very_verbose mutators_to_run: Optional[List[str]] = args.mutators_to_run comprehensive_flag: Optional[bool] = args.comprehensive @@ -167,7 +157,6 @@ def main() -> None: # pylint: disable=too-many-statements,too-many-branches,too if paths_to_ignore: paths_to_ignore_list = paths_to_ignore.strip("][").split(",") - logger.info(blue(f"Ignored paths - {', '.join(paths_to_ignore_list)}")) else: paths_to_ignore_list = [] @@ -178,6 +167,8 @@ def main() -> None: # pylint: disable=too-many-statements,too-many-branches,too # get all the contracts as a list from given codebase sol_file_list: List[str] = get_sol_file_list(Path(args.codebase), paths_to_ignore_list) + logger.info(blue("Preparing to mutate files:\n- " + "\n- ".join(sol_file_list))) + # folder where backup files and uncaught mutants are saved if output_dir is None: output_dir = "./mutation_campaign" @@ -247,9 +238,11 @@ def main() -> None: # pylint: disable=too-many-statements,too-many-branches,too # lines those need not be mutated (taken from RR and CR) dont_mutate_lines = [] - # mutation + # perform mutations on {target_contract} in file {file_name} + # setup placeholder val to signal whether we need to skip if no target_contract is found target_contract = "SLITHER_SKIP_MUTATIONS" if contract_names else "" try: + # loop through all contracts in file_name for compilation_unit_of_main_file in sl.compilation_units: for contract in compilation_unit_of_main_file.contracts: if contract.name in contract_names and contract.name not in mutated_contracts: @@ -287,7 +280,6 @@ def main() -> None: # pylint: disable=too-many-statements,too-many-branches,too target_contract, solc_remappings, verbose, - very_verbose, output_folder, dont_mutate_lines, ) @@ -372,8 +364,6 @@ def main() -> None: # pylint: disable=too-many-statements,too-many-branches,too logger.info(magenta("Zero Tweak mutants analyzed\n")) # Reset mutant counts before moving on to the next file - if very_verbose: - logger.info(blue("Reseting mutant counts to zero")) total_mutant_counts[0] = 0 total_mutant_counts[1] = 0 total_mutant_counts[2] = 0 diff --git a/slither/tools/mutator/mutators/abstract_mutator.py b/slither/tools/mutator/mutators/abstract_mutator.py index cb435f8569..2e99466d92 100644 --- a/slither/tools/mutator/mutators/abstract_mutator.py +++ b/slither/tools/mutator/mutators/abstract_mutator.py @@ -29,7 +29,6 @@ def __init__( # pylint: disable=too-many-arguments contract_instance: Contract, solc_remappings: Union[str, None], verbose: bool, - very_verbose: bool, output_folder: Path, dont_mutate_line: List[int], rate: int = 10, @@ -44,7 +43,6 @@ def __init__( # pylint: disable=too-many-arguments self.timeout = timeout self.solc_remappings = solc_remappings self.verbose = verbose - self.very_verbose = very_verbose self.output_folder = output_folder self.contract = contract_instance self.in_file = self.contract.source_mapping.filename.absolute @@ -98,7 +96,6 @@ def mutate(self) -> Tuple[List[int], List[int], List[int]]: self.timeout, self.solc_remappings, self.verbose, - self.very_verbose, ) # count the uncaught mutants, flag RR/CR mutants to skip further mutations @@ -132,18 +129,4 @@ def mutate(self) -> Tuple[List[int], List[int], List[int]]: else: self.total_mutant_counts[2] += 1 - if self.very_verbose: - if self.NAME == "RR": - logger.info( - f"Found {self.uncaught_mutant_counts[0]} uncaught revert mutants so far (out of {self.total_mutant_counts[0]} that compile)" - ) - elif self.NAME == "CR": - logger.info( - f"Found {self.uncaught_mutant_counts[1]} uncaught comment mutants so far (out of {self.total_mutant_counts[1]} that compile)" - ) - else: - logger.info( - f"Found {self.uncaught_mutant_counts[2]} uncaught tweak mutants so far (out of {self.total_mutant_counts[2]} that compile)" - ) - return self.total_mutant_counts, self.uncaught_mutant_counts, self.dont_mutate_line diff --git a/slither/tools/mutator/utils/testing_generated_mutant.py b/slither/tools/mutator/utils/testing_generated_mutant.py index d62fc3ff0e..d484ff68f8 100644 --- a/slither/tools/mutator/utils/testing_generated_mutant.py +++ b/slither/tools/mutator/utils/testing_generated_mutant.py @@ -91,7 +91,6 @@ def test_patch( # pylint: disable=too-many-arguments timeout: int, mappings: Union[str, None], verbose: bool, - very_verbose: bool, ) -> int: """ function to verify whether each patch is caught by tests @@ -118,7 +117,7 @@ def test_patch( # pylint: disable=too-many-arguments return 0 # uncaught else: - if very_verbose: + if verbose: logger.info( yellow( f"[{generator_name}] Line {patch['line_number']}: '{patch['old_string']}' ==> '{patch['new_string']}' --> COMPILATION FAILURE" diff --git a/slither/tools/possible_paths/possible_paths.py b/slither/tools/possible_paths/possible_paths.py index 6e836e76ab..15218a8720 100644 --- a/slither/tools/possible_paths/possible_paths.py +++ b/slither/tools/possible_paths/possible_paths.py @@ -123,10 +123,14 @@ def __find_target_paths( # Find all function calls in this function (except for low level) called_functions_list = [ - f for (_, f) in function.high_level_calls if isinstance(f, Function) + ir.function + for _, ir in function.high_level_calls + if isinstance(ir.function, Function) + ] + called_functions_list += [ir.function for ir in function.library_calls] + called_functions_list += [ + ir.function for ir in function.internal_calls if isinstance(ir.function, Function) ] - called_functions_list += [f for (_, f) in function.library_calls] - called_functions_list += [f for f in function.internal_calls if isinstance(f, Function)] called_functions = set(called_functions_list) # If any of our target functions are reachable from this function, it's a result. diff --git a/slither/tools/upgradeability/checks/variable_initialization.py b/slither/tools/upgradeability/checks/variable_initialization.py index b86036c87f..047c652dc4 100644 --- a/slither/tools/upgradeability/checks/variable_initialization.py +++ b/slither/tools/upgradeability/checks/variable_initialization.py @@ -43,7 +43,7 @@ class VariableWithInit(AbstractCheck): def _check(self) -> List[Output]: results = [] - for s in self.contract.stored_state_variables_ordered: + for s in self.contract.storage_variables_ordered: if s.initialized: info: CHECK_INFO = [s, " is a state variable with an initial value.\n"] json = self.generate_result(info) diff --git a/slither/tools/upgradeability/checks/variables_order.py b/slither/tools/upgradeability/checks/variables_order.py index 8d525a6dd3..8f5017d74c 100644 --- a/slither/tools/upgradeability/checks/variables_order.py +++ b/slither/tools/upgradeability/checks/variables_order.py @@ -115,29 +115,43 @@ def _contract2(self) -> Contract: def _check(self) -> List[Output]: contract1 = self._contract1() contract2 = self._contract2() - order1 = contract1.stored_state_variables_ordered - order2 = contract2.stored_state_variables_ordered results: List[Output] = [] - for idx, _ in enumerate(order1): - if len(order2) <= idx: - # Handle by MissingVariable - return results - - variable1 = order1[idx] - variable2 = order2[idx] - if (variable1.name != variable2.name) or (variable1.type != variable2.type): - info: CHECK_INFO = [ - "Different variables between ", - contract1, - " and ", - contract2, - "\n", - ] - info += ["\t ", variable1, "\n"] - info += ["\t ", variable2, "\n"] - json = self.generate_result(info) - results.append(json) + + def _check_internal( + contract1: Contract, contract2: Contract, results: List[Output], is_transient: bool + ): + if is_transient: + order1 = contract1.transient_variables_ordered + order2 = contract2.transient_variables_ordered + else: + order1 = contract1.storage_variables_ordered + order2 = contract2.storage_variables_ordered + + for idx, _ in enumerate(order1): + if len(order2) <= idx: + # Handle by MissingVariable + return + + variable1 = order1[idx] + variable2 = order2[idx] + if (variable1.name != variable2.name) or (variable1.type != variable2.type): + info: CHECK_INFO = [ + "Different variables between ", + contract1, + " and ", + contract2, + "\n", + ] + info += ["\t ", variable1, "\n"] + info += ["\t ", variable2, "\n"] + json = self.generate_result(info) + results.append(json) + + # Checking state variables with storage location + _check_internal(contract1, contract2, results, False) + # Checking state variables with transient location + _check_internal(contract1, contract2, results, True) return results @@ -236,22 +250,35 @@ def _contract2(self) -> Contract: def _check(self) -> List[Output]: contract1 = self._contract1() contract2 = self._contract2() - order1 = contract1.stored_state_variables_ordered - order2 = contract2.stored_state_variables_ordered - results = [] + results: List[Output] = [] - if len(order2) <= len(order1): - return [] + def _check_internal( + contract1: Contract, contract2: Contract, results: List[Output], is_transient: bool + ): + if is_transient: + order1 = contract1.transient_variables_ordered + order2 = contract2.transient_variables_ordered + else: + order1 = contract1.storage_variables_ordered + order2 = contract2.storage_variables_ordered - idx = len(order1) + if len(order2) <= len(order1): + return - while idx < len(order2): - variable2 = order2[idx] - info: CHECK_INFO = ["Extra variables in ", contract2, ": ", variable2, "\n"] - json = self.generate_result(info) - results.append(json) - idx = idx + 1 + idx = len(order1) + + while idx < len(order2): + variable2 = order2[idx] + info: CHECK_INFO = ["Extra variables in ", contract2, ": ", variable2, "\n"] + json = self.generate_result(info) + results.append(json) + idx = idx + 1 + + # Checking state variables with storage location + _check_internal(contract1, contract2, results, False) + # Checking state variables with transient location + _check_internal(contract1, contract2, results, True) return results diff --git a/slither/utils/function.py b/slither/utils/function.py index 34e6f221bb..64c098bfdd 100644 --- a/slither/utils/function.py +++ b/slither/utils/function.py @@ -12,3 +12,16 @@ def get_function_id(sig: str) -> int: digest = keccak.new(digest_bits=256) digest.update(sig.encode("utf-8")) return int("0x" + digest.hexdigest()[:8], 16) + + +def get_event_id(sig: str) -> int: + """' + Return the event id of the given signature + Args: + sig (str) + Return: + (int) + """ + digest = keccak.new(digest_bits=256) + digest.update(sig.encode("utf-8")) + return int("0x" + digest.hexdigest(), 16) diff --git a/slither/utils/upgradeability.py b/slither/utils/upgradeability.py index 22461dbcf6..bedf08d4b2 100644 --- a/slither/utils/upgradeability.py +++ b/slither/utils/upgradeability.py @@ -80,8 +80,8 @@ def compare( tainted-contracts: list[TaintedExternalContract] """ - order_vars1 = v1.stored_state_variables_ordered - order_vars2 = v2.stored_state_variables_ordered + order_vars1 = v1.storage_variables_ordered + v1.transient_variables_ordered + order_vars2 = v2.storage_variables_ordered + v2.transient_variables_ordered func_sigs1 = [function.solidity_signature for function in v1.functions] func_sigs2 = [function.solidity_signature for function in v2.functions] @@ -123,7 +123,9 @@ def compare( ): continue modified_calls = [ - func for func in new_modified_functions if func in function.internal_calls + func + for func in new_modified_functions + if func in [ir.function for ir in function.internal_calls] ] tainted_vars = [ var @@ -179,7 +181,8 @@ def tainted_external_contracts(funcs: List[Function]) -> List[TaintedExternalCon tainted_list: list[TaintedExternalContract] = [] for func in funcs: - for contract, target in func.all_high_level_calls(): + for contract, ir in func.all_high_level_calls(): + target = ir.function if contract.is_library: # Not interested in library calls continue @@ -254,7 +257,11 @@ def tainted_inheriting_contracts( new_taint = TaintedExternalContract(c) for f in c.functions_declared: # Search for functions that call an inherited tainted function or access an inherited tainted variable - internal_calls = [c for c in f.all_internal_calls() if isinstance(c, Function)] + internal_calls = [ + ir.function + for ir in f.all_internal_calls() + if isinstance(ir.function, Function) + ] if any( call.canonical_name == t.canonical_name for t in tainted.tainted_functions @@ -299,8 +306,8 @@ def get_missing_vars(v1: Contract, v2: Contract) -> List[StateVariable]: List of StateVariables from v1 missing in v2 """ results = [] - order_vars1 = v1.stored_state_variables_ordered - order_vars2 = v2.stored_state_variables_ordered + order_vars1 = v1.storage_variables_ordered + v1.transient_variables_ordered + order_vars2 = v2.storage_variables_ordered + v2.transient_variables_ordered if len(order_vars2) < len(order_vars1): for variable in order_vars1: if variable.name not in [v.name for v in order_vars2]: diff --git a/slither/visitors/expression/constants_folding.py b/slither/visitors/expression/constants_folding.py index b1fa570c6a..ddadb77a14 100644 --- a/slither/visitors/expression/constants_folding.py +++ b/slither/visitors/expression/constants_folding.py @@ -13,7 +13,9 @@ TupleExpression, TypeConversion, CallExpression, + MemberAccess, ) +from slither.core.expressions.elementary_type_name_expression import ElementaryTypeNameExpression from slither.core.variables import Variable from slither.utils.integer_conversion import convert_string_to_fraction, convert_string_to_int from slither.visitors.expression.expression import ExpressionVisitor @@ -27,7 +29,13 @@ class NotConstant(Exception): KEY = "ConstantFolding" CONSTANT_TYPES_OPERATIONS = Union[ - Literal, BinaryOperation, UnaryOperation, Identifier, TupleExpression, TypeConversion + Literal, + BinaryOperation, + UnaryOperation, + Identifier, + TupleExpression, + TypeConversion, + MemberAccess, ] @@ -69,6 +77,9 @@ def result(self) -> "Literal": # pylint: disable=import-outside-toplevel def _post_identifier(self, expression: Identifier) -> None: from slither.core.declarations.solidity_variables import SolidityFunction + from slither.core.declarations.enum import Enum + from slither.core.solidity_types.type_alias import TypeAlias + from slither.core.declarations.contract import Contract if isinstance(expression.value, Variable): if expression.value.is_constant: @@ -77,7 +88,14 @@ def _post_identifier(self, expression: Identifier) -> None: # Everything outside of literal if isinstance( expr, - (BinaryOperation, UnaryOperation, Identifier, TupleExpression, TypeConversion), + ( + BinaryOperation, + UnaryOperation, + Identifier, + TupleExpression, + TypeConversion, + MemberAccess, + ), ): cf = ConstantFolding(expr, self._type) expr = cf.result() @@ -88,7 +106,12 @@ def _post_identifier(self, expression: Identifier) -> None: elif isinstance(expression.value, SolidityFunction): set_val(expression, expression.value) else: - raise NotConstant + # Enum: We don't want to raise an error for a direct access to an Enum as they can be converted to a constant value + # We can't handle it here because we don't have the field accessed so we do it in _post_member_access + # TypeAlias: Support when a .wrap() is done with a constant + # Contract: Support when a constatn is use from a different contract + if not isinstance(expression.value, (Enum, TypeAlias, Contract)): + raise NotConstant # pylint: disable=too-many-branches,too-many-statements def _post_binary_operation(self, expression: BinaryOperation) -> None: @@ -96,12 +119,28 @@ def _post_binary_operation(self, expression: BinaryOperation) -> None: expression_right = expression.expression_right if not isinstance( expression_left, - (Literal, BinaryOperation, UnaryOperation, Identifier, TupleExpression, TypeConversion), + ( + Literal, + BinaryOperation, + UnaryOperation, + Identifier, + TupleExpression, + TypeConversion, + MemberAccess, + ), ): raise NotConstant if not isinstance( expression_right, - (Literal, BinaryOperation, UnaryOperation, Identifier, TupleExpression, TypeConversion), + ( + Literal, + BinaryOperation, + UnaryOperation, + Identifier, + TupleExpression, + TypeConversion, + MemberAccess, + ), ): raise NotConstant left = get_val(expression_left) @@ -205,6 +244,34 @@ def _post_assignement_operation(self, expression: expressions.AssignmentOperatio raise NotConstant def _post_call_expression(self, expression: expressions.CallExpression) -> None: + from slither.core.declarations.solidity_variables import SolidityFunction + from slither.core.declarations.enum import Enum + from slither.core.solidity_types import TypeAlias + + # pylint: disable=too-many-boolean-expressions + if ( + isinstance(expression.called, Identifier) + and expression.called.value == SolidityFunction("type()") + and len(expression.arguments) == 1 + and ( + isinstance(expression.arguments[0], ElementaryTypeNameExpression) + or isinstance(expression.arguments[0], Identifier) + and isinstance(expression.arguments[0].value, Enum) + ) + ): + # Returning early to support type(ElemType).max/min or type(MyEnum).max/min + return + if ( + isinstance(expression.called.expression, Identifier) + and isinstance(expression.called.expression.value, TypeAlias) + and isinstance(expression.called, MemberAccess) + and expression.called.member_name == "wrap" + and len(expression.arguments) == 1 + ): + # Handle constants in .wrap of user defined type + set_val(expression, get_val(expression.arguments[0])) + return + called = get_val(expression.called) args = [get_val(arg) for arg in expression.arguments] if called.name == "keccak256(bytes)": @@ -220,12 +287,104 @@ def _post_conditional_expression(self, expression: expressions.ConditionalExpres def _post_elementary_type_name_expression( self, expression: expressions.ElementaryTypeNameExpression ) -> None: - raise NotConstant + # We don't have to raise an exception to support type(uint112).max or similar + pass def _post_index_access(self, expression: expressions.IndexAccess) -> None: raise NotConstant + # pylint: disable=too-many-locals def _post_member_access(self, expression: expressions.MemberAccess) -> None: + from slither.core.declarations import ( + SolidityFunction, + Contract, + EnumContract, + EnumTopLevel, + Enum, + ) + from slither.core.solidity_types import UserDefinedType, TypeAlias + + # pylint: disable=too-many-nested-blocks + if isinstance(expression.expression, CallExpression) and expression.member_name in [ + "min", + "max", + ]: + if isinstance(expression.expression.called, Identifier): + if expression.expression.called.value == SolidityFunction("type()"): + assert len(expression.expression.arguments) == 1 + type_expression_found = expression.expression.arguments[0] + type_found: Union[ElementaryType, UserDefinedType] + if isinstance(type_expression_found, ElementaryTypeNameExpression): + type_expression_found_type = type_expression_found.type + assert isinstance(type_expression_found_type, ElementaryType) + type_found = type_expression_found_type + value = ( + type_found.max if expression.member_name == "max" else type_found.min + ) + set_val(expression, value) + return + # type(enum).max/min + # Case when enum is in another contract e.g. type(C.E).max + if isinstance(type_expression_found, MemberAccess): + contract = type_expression_found.expression.value + assert isinstance(contract, Contract) + for enum in contract.enums: + if enum.name == type_expression_found.member_name: + type_found_in_expression = enum + type_found = UserDefinedType(enum) + break + else: + assert isinstance(type_expression_found, Identifier) + type_found_in_expression = type_expression_found.value + assert isinstance(type_found_in_expression, (EnumContract, EnumTopLevel)) + type_found = UserDefinedType(type_found_in_expression) + value = ( + type_found_in_expression.max + if expression.member_name == "max" + else type_found_in_expression.min + ) + set_val(expression, value) + return + elif isinstance(expression.expression, Identifier) and isinstance( + expression.expression.value, Enum + ): + # Handle direct access to enum field + set_val(expression, expression.expression.value.values.index(expression.member_name)) + return + elif isinstance(expression.expression, Identifier) and isinstance( + expression.expression.value, TypeAlias + ): + # User defined type .wrap call handled in _post_call_expression + return + elif ( + isinstance(expression.expression.value, Contract) + and expression.member_name in expression.expression.value.variables_as_dict + and expression.expression.value.variables_as_dict[expression.member_name].is_constant + ): + # Handles when a constant is accessed on another contract + variables = expression.expression.value.variables_as_dict + if isinstance(variables[expression.member_name].expression, MemberAccess): + self._post_member_access(variables[expression.member_name].expression) + set_val(expression, get_val(variables[expression.member_name].expression)) + return + + # If the variable is a Literal we convert its value to int + if isinstance(variables[expression.member_name].expression, Literal): + value = convert_string_to_int( + variables[expression.member_name].expression.converted_value + ) + # If the variable is a UnaryOperation we need convert its value to int + # and replacing possible spaces + elif isinstance(variables[expression.member_name].expression, UnaryOperation): + value = convert_string_to_int( + str(variables[expression.member_name].expression).replace(" ", "") + ) + else: + value = variables[expression.member_name].expression + + set_val(expression, value) + return + raise NotConstant def _post_new_array(self, expression: expressions.NewArray) -> None: @@ -272,6 +431,7 @@ def _post_type_conversion(self, expression: expressions.TypeConversion) -> None: TupleExpression, TypeConversion, CallExpression, + MemberAccess, ), ): raise NotConstant diff --git a/slither/vyper_parsing/declarations/contract.py b/slither/vyper_parsing/declarations/contract.py index 64fab1c549..ddf5161509 100644 --- a/slither/vyper_parsing/declarations/contract.py +++ b/slither/vyper_parsing/declarations/contract.py @@ -470,7 +470,7 @@ def parse_state_variables(self) -> None: assert var.name self._contract.variables_as_dict[var.name] = var - self._contract.add_variables_ordered([var]) + self._contract.add_state_variables_ordered([var]) # Interfaces can refer to constants self._contract.file_scope.variables[var.name] = var diff --git a/tests/e2e/detectors/snapshots/detectors__detector_ChainlinkFeedRegistry_0_8_20_chainlink_feed_registry_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_ChainlinkFeedRegistry_0_8_20_chainlink_feed_registry_sol__0.txt new file mode 100644 index 0000000000..6b7653ed03 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_ChainlinkFeedRegistry_0_8_20_chainlink_feed_registry_sol__0.txt @@ -0,0 +1,3 @@ +The Chainlink Feed Registry is used in the A contract. It's only available on Ethereum Mainnet, consider to not use it if the contract needs to be deployed on other chains. + - (None,price,None,None,None) = registry.latestRoundData(base,quote) (tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol#25) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_ChronicleUncheckedPrice_0_8_20_chronicle_unchecked_price_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_ChronicleUncheckedPrice_0_8_20_chronicle_unchecked_price_sol__0.txt new file mode 100644 index 0000000000..6ddbfa4e58 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_ChronicleUncheckedPrice_0_8_20_chronicle_unchecked_price_sol__0.txt @@ -0,0 +1,18 @@ +Chronicle price is not checked to be valid in C.bad2() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#74-76) + - (price,None) = chronicle.readWithAge() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#75) + +Chronicle price is not checked to be valid in C.bad() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#65-67) + - price = chronicle.read() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#66) + +Chronicle price is not checked to be valid in C.bad5() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#101-103) + - price = scribe.latestAnswer() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#102) + +Chronicle price is not checked to be valid in C.bad4() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#92-94) + - (isValid,price,None) = chronicle.tryReadWithAge() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#93) + +Chronicle price is not checked to be valid in C.bad3() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#83-85) + - (isValid,price) = chronicle.tryRead() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#84) + +Chronicle price is not checked to be valid in C.bad6() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#110-112) + - (None,price,None,None,None) = scribe.latestRoundData() (tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol#111) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_GelatoUnprotectedRandomness_0_8_20_gelato_unprotected_randomness_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_GelatoUnprotectedRandomness_0_8_20_gelato_unprotected_randomness_sol__0.txt new file mode 100644 index 0000000000..aee2ea4ddd --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_GelatoUnprotectedRandomness_0_8_20_gelato_unprotected_randomness_sol__0.txt @@ -0,0 +1,6 @@ +C.bad() (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#42-44) is unprotected and request randomness from Gelato VRF + - id = _requestRandomness(abi.encode(msg.sender)) (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#43) + +C.good2() (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#51-54) is unprotected and request randomness from Gelato VRF + - id = _requestRandomness(abi.encode(msg.sender)) (tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol#53) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_OptimismDeprecation_0_8_20_optimism_deprecation_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_OptimismDeprecation_0_8_20_optimism_deprecation_sol__0.txt new file mode 100644 index 0000000000..f6f4dccba0 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_OptimismDeprecation_0_8_20_optimism_deprecation_sol__0.txt @@ -0,0 +1,4 @@ +A deprecated Optimism predeploy or function is used in the Test contract. + - OPT_GAS.scalar() (tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol#15) + - L1_BLOCK_NUMBER.q() (tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol#19) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_PythDeprecatedFunctions_0_8_20_pyth_deprecated_functions_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_PythDeprecatedFunctions_0_8_20_pyth_deprecated_functions_sol__0.txt new file mode 100644 index 0000000000..4cc23d2138 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_PythDeprecatedFunctions_0_8_20_pyth_deprecated_functions_sol__0.txt @@ -0,0 +1,3 @@ +The following Pyth deprecated function is used + - price = pyth.getPrice(priceId) (tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol#23) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedConfidence_0_8_20_pyth_unchecked_confidence_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedConfidence_0_8_20_pyth_unchecked_confidence_sol__0.txt new file mode 100644 index 0000000000..ae0dc2ae2f --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedConfidence_0_8_20_pyth_unchecked_confidence_sol__0.txt @@ -0,0 +1,3 @@ +Pyth price conf field is not checked in C.bad(bytes32,uint256) (tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol#171-175) + - price = pyth.getEmaPriceNoOlderThan(id,age) (tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol#172) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedPublishTime_0_8_20_pyth_unchecked_publishtime_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedPublishTime_0_8_20_pyth_unchecked_publishtime_sol__0.txt new file mode 100644 index 0000000000..cb331c8d53 --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedPublishTime_0_8_20_pyth_unchecked_publishtime_sol__0.txt @@ -0,0 +1,3 @@ +Pyth price publishTime field is not checked in C.bad(bytes32) (tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol#171-175) + - price = pyth.getEmaPriceUnsafe(id) (tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol#172) + diff --git a/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol b/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol new file mode 100644 index 0000000000..cf5d1ad4d4 --- /dev/null +++ b/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol @@ -0,0 +1,37 @@ +interface FeedRegistryInterface { + function latestRoundData( + address base, + address quote + ) external view returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); +} + +interface MyInterface { + function latestRoundData( + address base, + address quote + ) external view returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); +} + +contract A { + FeedRegistryInterface public immutable registry; + MyInterface public immutable my_interface; + + constructor(FeedRegistryInterface _registry, MyInterface _my_interface) { + registry = _registry; + my_interface = _my_interface; + } + + function getPriceBad(address base, address quote) public returns (uint256) { + (, int256 price,,,) = registry.latestRoundData(base, quote); + // Do price validation + return uint256(price); + } + + function getPriceGood(address base, address quote) public returns (uint256) { + (, int256 price,,,) = my_interface.latestRoundData(base, quote); + // Do price validation + return uint256(price); + } + + +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol-0.8.20.zip b/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol-0.8.20.zip new file mode 100644 index 0000000000..262ede23f1 Binary files /dev/null and b/tests/e2e/detectors/test_data/chainlink-feed-registry/0.8.20/chainlink_feed_registry.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol b/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol new file mode 100644 index 0000000000..e12560fa7d --- /dev/null +++ b/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol @@ -0,0 +1,119 @@ +interface IChronicle { + /// @notice Returns the oracle's current value. + /// @dev Reverts if no value set. + /// @return value The oracle's current value. + function read() external view returns (uint value); + + /// @notice Returns the oracle's current value and its age. + /// @dev Reverts if no value set. + /// @return value The oracle's current value. + /// @return age The value's age. + function readWithAge() external view returns (uint value, uint age); + + /// @notice Returns the oracle's current value. + /// @return isValid True if value exists, false otherwise. + /// @return value The oracle's current value if it exists, zero otherwise. + function tryRead() external view returns (bool isValid, uint value); + + /// @notice Returns the oracle's current value and its age. + /// @return isValid True if value exists, false otherwise. + /// @return value The oracle's current value if it exists, zero otherwise. + /// @return age The value's age if value exists, zero otherwise. + function tryReadWithAge() + external + view + returns (bool isValid, uint value, uint age); +} + +interface IScribe is IChronicle { + /// @notice Returns the oracle's latest value. + /// @dev Provides partial compatibility with Chainlink's + /// IAggregatorV3Interface. + /// @return roundId 1. + /// @return answer The oracle's latest value. + /// @return startedAt 0. + /// @return updatedAt The timestamp of oracle's latest update. + /// @return answeredInRound 1. + function latestRoundData() + external + view + returns ( + uint80 roundId, + int answer, + uint startedAt, + uint updatedAt, + uint80 answeredInRound + ); + + /// @notice Returns the oracle's latest value. + /// @dev Provides partial compatibility with Chainlink's + /// IAggregatorV3Interface. + /// @custom:deprecated See https://docs.chain.link/data-feeds/api-reference/#latestanswer. + /// @return answer The oracle's latest value. + function latestAnswer() external view returns (int); +} + +contract C { + IScribe scribe; + IChronicle chronicle; + + constructor(address a) { + scribe = IScribe(a); + chronicle = IChronicle(a); + } + + function bad() public { + uint256 price = chronicle.read(); + } + + function good() public { + uint256 price = chronicle.read(); + require(price != 0); + } + + function bad2() public { + (uint256 price,) = chronicle.readWithAge(); + } + + function good2() public { + (uint256 price,) = chronicle.readWithAge(); + require(price != 0); + } + + function bad3() public { + (bool isValid, uint256 price) = chronicle.tryRead(); + } + + function good3() public { + (bool isValid, uint256 price) = chronicle.tryRead(); + require(isValid); + } + + function bad4() public { + (bool isValid, uint256 price,) = chronicle.tryReadWithAge(); + } + + function good4() public { + (bool isValid, uint256 price,) = chronicle.tryReadWithAge(); + require(isValid); + } + + function bad5() public { + int256 price = scribe.latestAnswer(); + } + + function good5() public { + int256 price = scribe.latestAnswer(); + require(price != 0); + } + + function bad6() public { + (, int256 price,,,) = scribe.latestRoundData(); + } + + function good6() public { + (, int256 price,,,) = scribe.latestRoundData(); + require(price != 0); + } + +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol-0.8.20.zip b/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol-0.8.20.zip new file mode 100644 index 0000000000..746efabf66 Binary files /dev/null and b/tests/e2e/detectors/test_data/chronicle-unchecked-price/0.8.20/chronicle_unchecked_price.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol new file mode 100644 index 0000000000..108859e9e3 --- /dev/null +++ b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol @@ -0,0 +1,62 @@ +// Mock GelatoVRFConsumerBase for what we need +abstract contract GelatoVRFConsumerBase { + bool[] public requestPending; + mapping(uint256 => bytes32) public requestedHash; + + function _fulfillRandomness( + uint256 randomness, + uint256 requestId, + bytes memory extraData + ) internal virtual; + + function _requestRandomness( + bytes memory extraData + ) internal returns (uint256 requestId) { + requestId = uint256(requestPending.length); + requestPending.push(); + requestPending[requestId] = true; + + bytes memory data = abi.encode(requestId, extraData); + uint256 round = 111; + + bytes memory dataWithRound = abi.encode(round, data); + bytes32 requestHash = keccak256(dataWithRound); + + requestedHash[requestId] = requestHash; + } + +} + +contract C is GelatoVRFConsumerBase { + address owner; + mapping(address => bool) authorized; + + function _fulfillRandomness( + uint256 randomness, + uint256, + bytes memory extraData + ) internal override { + // Do something with the random number + } + + function bad() public { + uint id = _requestRandomness(abi.encode(msg.sender)); + } + + function good() public { + require(msg.sender == owner); + uint id = _requestRandomness(abi.encode(msg.sender)); + } + + // This is currently a FP due to the limitation of function.is_protected + function good2() public { + require(authorized[msg.sender]); + uint id = _requestRandomness(abi.encode(msg.sender)); + } + + function good3() public { + if (msg.sender != owner) { revert(); } + uint id = _requestRandomness(abi.encode(msg.sender)); + } + +} diff --git a/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol-0.8.20.zip b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol-0.8.20.zip new file mode 100644 index 0000000000..013d3ef28d Binary files /dev/null and b/tests/e2e/detectors/test_data/gelato-unprotected-randomness/0.8.20/gelato_unprotected_randomness.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol b/tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol index 96f614880b..4eb35e72b3 100644 --- a/tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol +++ b/tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol @@ -49,4 +49,10 @@ contract Test { _; } while (i < 1); } + + modifier issue2619(bool b) { + while (b ? false : b) {} + _; + } + } diff --git a/tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol-0.4.25.zip b/tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol-0.4.25.zip index 36dd1f4bfe..6f132c957a 100644 Binary files a/tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol-0.4.25.zip and b/tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol-0.4.25.zip differ diff --git a/tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol b/tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol index 96f614880b..4eb35e72b3 100644 --- a/tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol +++ b/tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol @@ -49,4 +49,10 @@ contract Test { _; } while (i < 1); } + + modifier issue2619(bool b) { + while (b ? false : b) {} + _; + } + } diff --git a/tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol-0.5.16.zip b/tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol-0.5.16.zip index cc707c9636..206d606109 100644 Binary files a/tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol-0.5.16.zip and b/tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol-0.5.16.zip differ diff --git a/tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol b/tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol index 96f614880b..4eb35e72b3 100644 --- a/tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol +++ b/tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol @@ -49,4 +49,10 @@ contract Test { _; } while (i < 1); } + + modifier issue2619(bool b) { + while (b ? false : b) {} + _; + } + } diff --git a/tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol-0.6.11.zip b/tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol-0.6.11.zip index 1d5d42bd25..3b0f129d3f 100644 Binary files a/tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol-0.6.11.zip and b/tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol-0.6.11.zip differ diff --git a/tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol b/tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol index 96f614880b..e3bf6f97f3 100644 --- a/tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol +++ b/tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol @@ -49,4 +49,9 @@ contract Test { _; } while (i < 1); } + + modifier issue2619(bool b) { + while (b ? false : b) {} + _; + } } diff --git a/tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol-0.7.6.zip b/tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol-0.7.6.zip index c1c0e0fae4..c223d55d1f 100644 Binary files a/tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol-0.7.6.zip and b/tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol-0.7.6.zip differ diff --git a/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol b/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol new file mode 100644 index 0000000000..7ad55f3ddd --- /dev/null +++ b/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol @@ -0,0 +1,27 @@ +interface GasPriceOracle { + function scalar() external view returns (uint256); + function baseFee() external view returns (uint256); +} + +interface L1BlockNumber { + function q() external view returns (uint256); +} + +contract Test { + GasPriceOracle constant OPT_GAS = GasPriceOracle(0x420000000000000000000000000000000000000F); + L1BlockNumber constant L1_BLOCK_NUMBER = L1BlockNumber(0x4200000000000000000000000000000000000013); + + function bad() public { + OPT_GAS.scalar(); + } + + function bad2() public { + L1_BLOCK_NUMBER.q(); + } + + function good() public { + OPT_GAS.baseFee(); + } + + +} diff --git a/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol-0.8.20.zip b/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol-0.8.20.zip new file mode 100644 index 0000000000..de18d4a0dd Binary files /dev/null and b/tests/e2e/detectors/test_data/optimism-deprecation/0.8.20/optimism_deprecation.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol b/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol new file mode 100644 index 0000000000..dc8130db59 --- /dev/null +++ b/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol @@ -0,0 +1,35 @@ + +// Fake Pyth interface +interface IPyth { + function getPrice(bytes32 id) external returns (uint256 price); + function notDeprecated(bytes32 id) external returns (uint256 price); +} + +interface INotPyth { + function getPrice(bytes32 id) external returns (uint256 price); +} + +contract C { + + IPyth pyth; + INotPyth notPyth; + + constructor(IPyth _pyth, INotPyth _notPyth) { + pyth = _pyth; + notPyth = _notPyth; + } + + function Deprecated(bytes32 priceId) public { + uint256 price = pyth.getPrice(priceId); + } + + function notDeprecated(bytes32 priceId) public { + uint256 price = pyth.notDeprecated(priceId); + } + + function notPythCall(bytes32 priceId) public { + uint256 price = notPyth.getPrice(priceId); + } + + +} diff --git a/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol-0.8.20.zip b/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol-0.8.20.zip new file mode 100644 index 0000000000..258a28c938 Binary files /dev/null and b/tests/e2e/detectors/test_data/pyth-deprecated-functions/0.8.20/pyth_deprecated_functions.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol b/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol new file mode 100644 index 0000000000..58880c382d --- /dev/null +++ b/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol @@ -0,0 +1,193 @@ +contract PythStructs { + // A price with a degree of uncertainty, represented as a price +- a confidence interval. + // + // The confidence interval roughly corresponds to the standard error of a normal distribution. + // Both the price and confidence are stored in a fixed-point numeric representation, + // `x * (10^expo)`, where `expo` is the exponent. + // + // Please refer to the documentation at https://docs.pyth.network/consumers/best-practices for how + // to how this price safely. + struct Price { + // Price + int64 price; + // Confidence interval around the price + uint64 conf; + // Price exponent + int32 expo; + // Unix timestamp describing when the price was published + uint publishTime; + } + + // PriceFeed represents a current aggregate price from pyth publisher feeds. + struct PriceFeed { + // The price ID. + bytes32 id; + // Latest available price + Price price; + // Latest available exponentially-weighted moving average price + Price emaPrice; + } +} + +interface IPyth { + /// @notice Returns the period (in seconds) that a price feed is considered valid since its publish time + function getValidTimePeriod() external view returns (uint validTimePeriod); + + /// @notice Returns the price and confidence interval. + /// @dev Reverts if the price has not been updated within the last `getValidTimePeriod()` seconds. + /// @param id The Pyth Price Feed ID of which to fetch the price and confidence interval. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPrice( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price and confidence interval. + /// @dev Reverts if the EMA price is not available. + /// @param id The Pyth Price Feed ID of which to fetch the EMA price and confidence interval. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPrice( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the price of a price feed without any sanity checks. + /// @dev This function returns the most recent price update in this contract without any recency checks. + /// This function is unsafe as the returned price update may be arbitrarily far in the past. + /// + /// Users of this function should check the `publishTime` in the price to ensure that the returned price is + /// sufficiently recent for their application. If you are considering using this function, it may be + /// safer / easier to use either `getPrice` or `getPriceNoOlderThan`. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPriceUnsafe( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the price that is no older than `age` seconds of the current time. + /// @dev This function is a sanity-checked version of `getPriceUnsafe` which is useful in + /// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently + /// recently. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPriceNoOlderThan( + bytes32 id, + uint age + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price of a price feed without any sanity checks. + /// @dev This function returns the same price as `getEmaPrice` in the case where the price is available. + /// However, if the price is not recent this function returns the latest available price. + /// + /// The returned price can be from arbitrarily far in the past; this function makes no guarantees that + /// the returned price is recent or useful for any particular application. + /// + /// Users of this function should check the `publishTime` in the price to ensure that the returned price is + /// sufficiently recent for their application. If you are considering using this function, it may be + /// safer / easier to use either `getEmaPrice` or `getEmaPriceNoOlderThan`. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPriceUnsafe( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price that is no older than `age` seconds + /// of the current time. + /// @dev This function is a sanity-checked version of `getEmaPriceUnsafe` which is useful in + /// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently + /// recently. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPriceNoOlderThan( + bytes32 id, + uint age + ) external view returns (PythStructs.Price memory price); + + /// @notice Update price feeds with given update messages. + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// Prices will be updated if they are more recent than the current stored prices. + /// The call will succeed even if the update is not the most recent. + /// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid. + /// @param updateData Array of price update data. + function updatePriceFeeds(bytes[] calldata updateData) external payable; + + /// @notice Wrapper around updatePriceFeeds that rejects fast if a price update is not necessary. A price update is + /// necessary if the current on-chain publishTime is older than the given publishTime. It relies solely on the + /// given `publishTimes` for the price feeds and does not read the actual price update publish time within `updateData`. + /// + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// + /// `priceIds` and `publishTimes` are two arrays with the same size that correspond to senders known publishTime + /// of each priceId when calling this method. If all of price feeds within `priceIds` have updated and have + /// a newer or equal publish time than the given publish time, it will reject the transaction to save gas. + /// Otherwise, it calls updatePriceFeeds method to update the prices. + /// + /// @dev Reverts if update is not needed or the transferred fee is not sufficient or the updateData is invalid. + /// @param updateData Array of price update data. + /// @param priceIds Array of price ids. + /// @param publishTimes Array of publishTimes. `publishTimes[i]` corresponds to known `publishTime` of `priceIds[i]` + function updatePriceFeedsIfNecessary( + bytes[] calldata updateData, + bytes32[] calldata priceIds, + uint64[] calldata publishTimes + ) external payable; + + /// @notice Returns the required fee to update an array of price updates. + /// @param updateData Array of price update data. + /// @return feeAmount The required fee in Wei. + function getUpdateFee( + bytes[] calldata updateData + ) external view returns (uint feeAmount); + + /// @notice Parse `updateData` and return price feeds of the given `priceIds` if they are all published + /// within `minPublishTime` and `maxPublishTime`. + /// + /// You can use this method if you want to use a Pyth price at a fixed time and not the most recent price; + /// otherwise, please consider using `updatePriceFeeds`. This method does not store the price updates on-chain. + /// + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// + /// + /// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid or there is + /// no update for any of the given `priceIds` within the given time range. + /// @param updateData Array of price update data. + /// @param priceIds Array of price ids. + /// @param minPublishTime minimum acceptable publishTime for the given `priceIds`. + /// @param maxPublishTime maximum acceptable publishTime for the given `priceIds`. + /// @return priceFeeds Array of the price feeds corresponding to the given `priceIds` (with the same order). + function parsePriceFeedUpdates( + bytes[] calldata updateData, + bytes32[] calldata priceIds, + uint64 minPublishTime, + uint64 maxPublishTime + ) external payable returns (PythStructs.PriceFeed[] memory priceFeeds); +} + + +contract C { + IPyth pyth; + + constructor(IPyth _pyth) { + pyth = _pyth; + } + + function bad(bytes32 id, uint256 age) public { + PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age); + require(price.publishTime > block.timestamp - 120); + // Use price + } + + function good(bytes32 id, uint256 age) public { + PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age); + require(price.conf < 10000); + require(price.publishTime > block.timestamp - 120); + // Use price + } + + function good2(bytes32 id, uint256 age) public { + PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age); + require(price.publishTime > block.timestamp - 120); + if (price.conf >= 10000) { + revert(); + } + // Use price + } + +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol-0.8.20.zip b/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol-0.8.20.zip new file mode 100644 index 0000000000..6e5fa1b9f1 Binary files /dev/null and b/tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol b/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol new file mode 100644 index 0000000000..74ab10fe31 --- /dev/null +++ b/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol @@ -0,0 +1,193 @@ +contract PythStructs { + // A price with a degree of uncertainty, represented as a price +- a confidence interval. + // + // The confidence interval roughly corresponds to the standard error of a normal distribution. + // Both the price and confidence are stored in a fixed-point numeric representation, + // `x * (10^expo)`, where `expo` is the exponent. + // + // Please refer to the documentation at https://docs.pyth.network/consumers/best-practices for how + // to how this price safely. + struct Price { + // Price + int64 price; + // Confidence interval around the price + uint64 conf; + // Price exponent + int32 expo; + // Unix timestamp describing when the price was published + uint publishTime; + } + + // PriceFeed represents a current aggregate price from pyth publisher feeds. + struct PriceFeed { + // The price ID. + bytes32 id; + // Latest available price + Price price; + // Latest available exponentially-weighted moving average price + Price emaPrice; + } +} + +interface IPyth { + /// @notice Returns the period (in seconds) that a price feed is considered valid since its publish time + function getValidTimePeriod() external view returns (uint validTimePeriod); + + /// @notice Returns the price and confidence interval. + /// @dev Reverts if the price has not been updated within the last `getValidTimePeriod()` seconds. + /// @param id The Pyth Price Feed ID of which to fetch the price and confidence interval. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPrice( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price and confidence interval. + /// @dev Reverts if the EMA price is not available. + /// @param id The Pyth Price Feed ID of which to fetch the EMA price and confidence interval. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPrice( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the price of a price feed without any sanity checks. + /// @dev This function returns the most recent price update in this contract without any recency checks. + /// This function is unsafe as the returned price update may be arbitrarily far in the past. + /// + /// Users of this function should check the `publishTime` in the price to ensure that the returned price is + /// sufficiently recent for their application. If you are considering using this function, it may be + /// safer / easier to use either `getPrice` or `getPriceNoOlderThan`. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPriceUnsafe( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the price that is no older than `age` seconds of the current time. + /// @dev This function is a sanity-checked version of `getPriceUnsafe` which is useful in + /// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently + /// recently. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getPriceNoOlderThan( + bytes32 id, + uint age + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price of a price feed without any sanity checks. + /// @dev This function returns the same price as `getEmaPrice` in the case where the price is available. + /// However, if the price is not recent this function returns the latest available price. + /// + /// The returned price can be from arbitrarily far in the past; this function makes no guarantees that + /// the returned price is recent or useful for any particular application. + /// + /// Users of this function should check the `publishTime` in the price to ensure that the returned price is + /// sufficiently recent for their application. If you are considering using this function, it may be + /// safer / easier to use either `getEmaPrice` or `getEmaPriceNoOlderThan`. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPriceUnsafe( + bytes32 id + ) external view returns (PythStructs.Price memory price); + + /// @notice Returns the exponentially-weighted moving average price that is no older than `age` seconds + /// of the current time. + /// @dev This function is a sanity-checked version of `getEmaPriceUnsafe` which is useful in + /// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently + /// recently. + /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely. + function getEmaPriceNoOlderThan( + bytes32 id, + uint age + ) external view returns (PythStructs.Price memory price); + + /// @notice Update price feeds with given update messages. + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// Prices will be updated if they are more recent than the current stored prices. + /// The call will succeed even if the update is not the most recent. + /// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid. + /// @param updateData Array of price update data. + function updatePriceFeeds(bytes[] calldata updateData) external payable; + + /// @notice Wrapper around updatePriceFeeds that rejects fast if a price update is not necessary. A price update is + /// necessary if the current on-chain publishTime is older than the given publishTime. It relies solely on the + /// given `publishTimes` for the price feeds and does not read the actual price update publish time within `updateData`. + /// + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// + /// `priceIds` and `publishTimes` are two arrays with the same size that correspond to senders known publishTime + /// of each priceId when calling this method. If all of price feeds within `priceIds` have updated and have + /// a newer or equal publish time than the given publish time, it will reject the transaction to save gas. + /// Otherwise, it calls updatePriceFeeds method to update the prices. + /// + /// @dev Reverts if update is not needed or the transferred fee is not sufficient or the updateData is invalid. + /// @param updateData Array of price update data. + /// @param priceIds Array of price ids. + /// @param publishTimes Array of publishTimes. `publishTimes[i]` corresponds to known `publishTime` of `priceIds[i]` + function updatePriceFeedsIfNecessary( + bytes[] calldata updateData, + bytes32[] calldata priceIds, + uint64[] calldata publishTimes + ) external payable; + + /// @notice Returns the required fee to update an array of price updates. + /// @param updateData Array of price update data. + /// @return feeAmount The required fee in Wei. + function getUpdateFee( + bytes[] calldata updateData + ) external view returns (uint feeAmount); + + /// @notice Parse `updateData` and return price feeds of the given `priceIds` if they are all published + /// within `minPublishTime` and `maxPublishTime`. + /// + /// You can use this method if you want to use a Pyth price at a fixed time and not the most recent price; + /// otherwise, please consider using `updatePriceFeeds`. This method does not store the price updates on-chain. + /// + /// This method requires the caller to pay a fee in wei; the required fee can be computed by calling + /// `getUpdateFee` with the length of the `updateData` array. + /// + /// + /// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid or there is + /// no update for any of the given `priceIds` within the given time range. + /// @param updateData Array of price update data. + /// @param priceIds Array of price ids. + /// @param minPublishTime minimum acceptable publishTime for the given `priceIds`. + /// @param maxPublishTime maximum acceptable publishTime for the given `priceIds`. + /// @return priceFeeds Array of the price feeds corresponding to the given `priceIds` (with the same order). + function parsePriceFeedUpdates( + bytes[] calldata updateData, + bytes32[] calldata priceIds, + uint64 minPublishTime, + uint64 maxPublishTime + ) external payable returns (PythStructs.PriceFeed[] memory priceFeeds); +} + + +contract C { + IPyth pyth; + + constructor(IPyth _pyth) { + pyth = _pyth; + } + + function bad(bytes32 id) public { + PythStructs.Price memory price = pyth.getEmaPriceUnsafe(id); + require(price.conf < 10000); + // Use price + } + + function good(bytes32 id) public { + PythStructs.Price memory price = pyth.getEmaPriceUnsafe(id); + require(price.publishTime > block.timestamp - 120); + require(price.conf < 10000); + // Use price + } + + function good2(bytes32 id) public { + PythStructs.Price memory price = pyth.getEmaPriceUnsafe(id); + require(price.conf < 10000); + if (price.publishTime <= block.timestamp - 120) { + revert(); + } + // Use price + } + +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol-0.8.20.zip b/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol-0.8.20.zip new file mode 100644 index 0000000000..178b65b388 Binary files /dev/null and b/tests/e2e/detectors/test_data/pyth-unchecked-publishtime/0.8.20/pyth_unchecked_publishtime.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 2c6a5f55a3..d2f191a4d9 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1714,6 +1714,41 @@ def id_test(test_item: Test): "out_of_order_retryable.sol", "0.8.20", ), + Test( + all_detectors.GelatoUnprotectedRandomness, + "gelato_unprotected_randomness.sol", + "0.8.20", + ), + Test( + all_detectors.ChronicleUncheckedPrice, + "chronicle_unchecked_price.sol", + "0.8.20", + ), + Test( + all_detectors.PythUncheckedConfidence, + "pyth_unchecked_confidence.sol", + "0.8.20", + ), + Test( + all_detectors.PythUncheckedPublishTime, + "pyth_unchecked_publishtime.sol", + "0.8.20", + ), + Test( + all_detectors.ChainlinkFeedRegistry, + "chainlink_feed_registry.sol", + "0.8.20", + ), + Test( + all_detectors.PythDeprecatedFunctions, + "pyth_deprecated_functions.sol", + "0.8.20", + ), + Test( + all_detectors.OptimismDeprecation, + "optimism_deprecation.sol", + "0.8.20", + ), # Test( # all_detectors.UnusedImport, # "ConstantContractLevelUsedInContractTest.sol", diff --git a/tests/e2e/printers/test_data/test_printer_cheatcode/README.md b/tests/e2e/printers/test_data/test_printer_cheatcode/README.md new file mode 100644 index 0000000000..f1a26a296c --- /dev/null +++ b/tests/e2e/printers/test_data/test_printer_cheatcode/README.md @@ -0,0 +1,7 @@ +# Counter + +Init using : + +```shell +forge install foundry-rs/forge-std +``` \ No newline at end of file diff --git a/tests/e2e/printers/test_data/test_printer_cheatcode/foundry.toml b/tests/e2e/printers/test_data/test_printer_cheatcode/foundry.toml new file mode 100644 index 0000000000..e6810b2b58 --- /dev/null +++ b/tests/e2e/printers/test_data/test_printer_cheatcode/foundry.toml @@ -0,0 +1,6 @@ +[profile.default] +src = 'src' +out = 'out' +libs = ['lib'] + +# See more config options https://github.com/foundry-rs/foundry/tree/master/config \ No newline at end of file diff --git a/tests/e2e/printers/test_data/test_printer_cheatcode/src/Counter.sol b/tests/e2e/printers/test_data/test_printer_cheatcode/src/Counter.sol new file mode 100644 index 0000000000..aded7997b0 --- /dev/null +++ b/tests/e2e/printers/test_data/test_printer_cheatcode/src/Counter.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +contract Counter { + uint256 public number; + + function setNumber(uint256 newNumber) public { + number = newNumber; + } + + function increment() public { + number++; + } +} diff --git a/tests/e2e/printers/test_data/test_printer_cheatcode/test/Counter.t.sol b/tests/e2e/printers/test_data/test_printer_cheatcode/test/Counter.t.sol new file mode 100644 index 0000000000..529a7ce088 --- /dev/null +++ b/tests/e2e/printers/test_data/test_printer_cheatcode/test/Counter.t.sol @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; +import "../src/Counter.sol"; + +contract CounterTest is Test { + Counter public counter; + + address public alice = address(0x42); + address public bob = address(0x43); + + function difficulty(uint256 value) public { + // empty + } + + function setUp() public { + counter = new Counter(); + counter.setNumber(0); + + vm.deal(alice, 1 ether); + vm.deal(bob, 2 ether); + + difficulty(1); + } + + function testIncrement() public { + vm.prank(alice); + counter.increment(); + assertEq(counter.number(), 1); + + vm.prank(bob); + counter.increment(); + assertEq(counter.number(), 2); + } +} diff --git a/tests/e2e/printers/test_data/test_printer_slithir/bug-2266.sol b/tests/e2e/printers/test_data/test_printer_slithir/bug-2266.sol new file mode 100644 index 0000000000..5c11a29141 --- /dev/null +++ b/tests/e2e/printers/test_data/test_printer_slithir/bug-2266.sol @@ -0,0 +1,13 @@ +pragma solidity ^0.8.0; + +contract A { + function add(uint256 a, uint256 b) public returns (uint256) { + return a + b; + } +} + +contract B is A { + function assignFunction() public { + function(uint256, uint256) returns (uint256) myFunction = super.add; + } +} \ No newline at end of file diff --git a/tests/e2e/printers/test_printers.py b/tests/e2e/printers/test_printers.py index 3dea8b74a4..3ca536b0ac 100644 --- a/tests/e2e/printers/test_printers.py +++ b/tests/e2e/printers/test_printers.py @@ -1,16 +1,23 @@ import re +import shutil from collections import Counter from pathlib import Path +import pytest from crytic_compile import CryticCompile from crytic_compile.platform.solc_standard_json import SolcStandardJson from slither import Slither from slither.printers.inheritance.inheritance_graph import PrinterInheritanceGraph +from slither.printers.summary.cheatcodes import CheatcodePrinter +from slither.printers.summary.slithir import PrinterSlithIR TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" +foundry_available = shutil.which("forge") is not None +project_ready = Path(TEST_DATA_DIR, "test_printer_cheatcode/lib/forge-std").exists() + def test_inheritance_printer(solc_binary_path) -> None: solc_path = solc_binary_path("0.8.0") @@ -34,8 +41,7 @@ def test_inheritance_printer(solc_binary_path) -> None: assert counter["B -> A"] == 2 assert counter["C -> A"] == 1 - - # Lets also test the include/exclude interface behavior + # Let also test the include/exclude interface behavior # Check that the interface is not included assert "MyInterfaceX" not in content @@ -46,3 +52,35 @@ def test_inheritance_printer(solc_binary_path) -> None: # Remove test generated files Path("test_printer.dot").unlink(missing_ok=True) + + +@pytest.mark.skipif( + not foundry_available or not project_ready, reason="requires Foundry and project setup" +) +def test_printer_cheatcode(): + slither = Slither( + Path(TEST_DATA_DIR, "test_printer_cheatcode").as_posix(), foundry_compile_all=True + ) + + printer = CheatcodePrinter(slither=slither, logger=None) + output = printer.output("") + + assert ( + output.data["description"] + == "CounterTest (test/Counter.t.sol)\n\tsetUp\n\t\tdeal - (test/Counter.t.sol#21 (9 - 32)\n\t\tvm.deal(alice,1000000000000000000)\n\n\t\tdeal - (test/Counter.t.sol#22 (9 - 30)\n\t\tvm.deal(bob,2000000000000000000)\n\n\ttestIncrement\n\t\tprank - (test/Counter.t.sol#28 (9 - 24)\n\t\tvm.prank(alice)\n\n\t\tassertEq - (test/Counter.t.sol#30 (9 - 38)\n\t\tassertEq(counter.number(),1)\n\n\t\tprank - (test/Counter.t.sol#32 (9 - 22)\n\t\tvm.prank(bob)\n\n\t\tassertEq - (test/Counter.t.sol#34 (9 - 38)\n\t\tassertEq(counter.number(),2)\n\n" + ) + + +def test_slithir_printer(solc_binary_path) -> None: + solc_path = solc_binary_path("0.8.0") + standard_json = SolcStandardJson() + standard_json.add_source_file( + Path(TEST_DATA_DIR, "test_printer_slithir", "bug-2266.sol").as_posix() + ) + compilation = CryticCompile(standard_json, solc=solc_path) + slither = Slither(compilation) + + printer = PrinterSlithIR(slither, logger=None) + output = printer.output("test_printer_slithir.dot") + + assert "slither.core.solidity_types" not in output.data["description"] diff --git a/tests/e2e/solc_parsing/test_ast_parsing.py b/tests/e2e/solc_parsing/test_ast_parsing.py index ca3872f8c5..6ec7b6fbd2 100644 --- a/tests/e2e/solc_parsing/test_ast_parsing.py +++ b/tests/e2e/solc_parsing/test_ast_parsing.py @@ -475,6 +475,7 @@ def make_version(minor: int, patch_min: int, patch_max: int) -> List[str]: Test("solidity-0.8.24.sol", ["0.8.24"], solc_args="--evm-version cancun"), Test("scope/inherited_function_scope.sol", ["0.8.24"]), Test("using_for_global_user_defined_operator_1.sol", ["0.8.24"]), + Test("require-error.sol", ["0.8.27"]), ] # create the output folder if needed try: diff --git a/tests/e2e/solc_parsing/test_data/compile/require-error.sol-0.8.27-compact.zip b/tests/e2e/solc_parsing/test_data/compile/require-error.sol-0.8.27-compact.zip new file mode 100644 index 0000000000..63aa223b37 Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/require-error.sol-0.8.27-compact.zip differ diff --git a/tests/e2e/solc_parsing/test_data/expected/require-error.sol-0.8.27-compact.json b/tests/e2e/solc_parsing/test_data/expected/require-error.sol-0.8.27-compact.json new file mode 100644 index 0000000000..3c3089c048 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/require-error.sol-0.8.27-compact.json @@ -0,0 +1,5 @@ +{ + "TestToken": { + "transferWithRequireError(address,uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: EXPRESSION 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n2->3;\n3[label=\"Node Type: EXPRESSION 3\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/require-error.sol b/tests/e2e/solc_parsing/test_data/require-error.sol new file mode 100644 index 0000000000..97c8ecac4f --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/require-error.sol @@ -0,0 +1,20 @@ +pragma solidity 0.8.27; + +/// Insufficient balance for transfer. Needed `required` but only +/// `available` available. +/// @param available balance available. +/// @param required requested amount to transfer. +error InsufficientBalance(uint256 available, uint256 required); + +contract TestToken { + mapping(address => uint) balance; + function transferWithRequireError(address to, uint256 amount) public { + require( + balance[msg.sender] >= amount, + InsufficientBalance(balance[msg.sender], amount) + ); + balance[msg.sender] -= amount; + balance[to] += amount; + } + // ... +} diff --git a/tests/unit/slithir/test_argument_reorder.py b/tests/unit/slithir/test_argument_reorder.py index 12f5bd7f28..f08314afef 100644 --- a/tests/unit/slithir/test_argument_reorder.py +++ b/tests/unit/slithir/test_argument_reorder.py @@ -63,3 +63,44 @@ def test_internal_call_reorder(solc_binary_path) -> None: isinstance(internal_calls[0].arguments[2], Constant) and internal_calls[0].arguments[2].value == 5 ) + + +def test_overridden_function_reorder(solc_binary_path) -> None: + solc_path = solc_binary_path("0.8.15") + slither = Slither( + Path(ARG_REORDER_TEST_ROOT, "test_overridden_function.sol").as_posix(), solc=solc_path + ) + + operations = slither.contracts[0].functions[1].slithir_operations + internal_calls = [x for x in operations if isinstance(x, InternalCall)] + assert len(internal_calls) == 1 + + assert ( + isinstance(internal_calls[0].arguments[0], Constant) + and internal_calls[0].arguments[0].value == 23 + ) + assert ( + isinstance(internal_calls[0].arguments[1], Constant) + and internal_calls[0].arguments[1].value == 36 + ) + assert ( + isinstance(internal_calls[0].arguments[2], Constant) + and internal_calls[0].arguments[2].value == 34 + ) + + operations = slither.contracts[1].functions[1].slithir_operations + internal_calls = [x for x in operations if isinstance(x, InternalCall)] + assert len(internal_calls) == 1 + + assert ( + isinstance(internal_calls[0].arguments[0], Constant) + and internal_calls[0].arguments[0].value == 23 + ) + assert ( + isinstance(internal_calls[0].arguments[1], Constant) + and internal_calls[0].arguments[1].value == 36 + ) + assert ( + isinstance(internal_calls[0].arguments[2], Constant) + and internal_calls[0].arguments[2].value == 34 + ) diff --git a/tests/unit/slithir/test_constantfolding.py b/tests/unit/slithir/test_constantfolding.py new file mode 100644 index 0000000000..fcf00035bd --- /dev/null +++ b/tests/unit/slithir/test_constantfolding.py @@ -0,0 +1,22 @@ +from pathlib import Path + +from slither import Slither +from slither.printers.guidance.echidna import _extract_constants, ConstantValue + +TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" + + +def test_enum_max_min(solc_binary_path) -> None: + solc_path = solc_binary_path("0.8.19") + slither = Slither(Path(TEST_DATA_DIR, "constantfolding.sol").as_posix(), solc=solc_path) + + contracts = slither.get_contract_from_name("A") + + constants = _extract_constants(contracts)[0]["A"]["use()"] + + assert set(constants) == { + ConstantValue(value="2", type="uint256"), + ConstantValue(value="10", type="uint256"), + ConstantValue(value="100", type="uint256"), + ConstantValue(value="4294967295", type="uint32"), + } diff --git a/tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol b/tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol new file mode 100644 index 0000000000..4baf0df65a --- /dev/null +++ b/tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol @@ -0,0 +1,8 @@ +contract A { + function a(uint256 e, uint256 q, uint256 w) internal virtual {} + function b() public { a({e: 23, w: 34, q: 36}); } +} + +contract B is A { + function a(uint256 q, uint256 ww, uint256 e) internal override {} +} \ No newline at end of file diff --git a/tests/unit/slithir/test_data/constantfolding.sol b/tests/unit/slithir/test_data/constantfolding.sol new file mode 100644 index 0000000000..aef4a24270 --- /dev/null +++ b/tests/unit/slithir/test_data/constantfolding.sol @@ -0,0 +1,19 @@ +type MyType is uint256; + +contract A{ + + enum E{ + a,b,c + } + + + uint a = 10; + E b = type(E).max; + uint c = type(uint32).max; + MyType d = MyType.wrap(100); + + function use() public returns(uint){ + E e = b; + return a +c + MyType.unwrap(d); + } +} \ No newline at end of file diff --git a/tests/unit/slithir/test_data/selector.sol b/tests/unit/slithir/test_data/selector.sol new file mode 100644 index 0000000000..60ec33cd66 --- /dev/null +++ b/tests/unit/slithir/test_data/selector.sol @@ -0,0 +1,47 @@ +interface I{ + function testFunction(uint a) external ; +} + +contract A{ + function testFunction() public{} +} + +contract Test{ + event TestEvent(); + struct St{ + uint a; + } + error TestError(); + + function testFunction(uint a) public {} + + + function testFunctionStructure(St memory s) public {} + + function returnEvent() public returns (bytes32){ + return TestEvent.selector; + } + + function returnError() public returns (bytes4){ + return TestError.selector; + } + + + function returnFunctionFromContract() public returns (bytes4){ + return I.testFunction.selector; + } + + + function returnFunction() public returns (bytes4){ + return this.testFunction.selector; + } + + function returnFunctionWithStructure() public returns (bytes4){ + return this.testFunctionStructure.selector; + } + + function returnFunctionThroughLocaLVar() public returns(bytes4){ + A a; + return a.testFunction.selector; + } +} \ No newline at end of file diff --git a/tests/unit/slithir/test_selector.py b/tests/unit/slithir/test_selector.py new file mode 100644 index 0000000000..34643b58d8 --- /dev/null +++ b/tests/unit/slithir/test_selector.py @@ -0,0 +1,32 @@ +from pathlib import Path +from slither import Slither +from slither.slithir.operations import Assignment +from slither.slithir.variables import Constant + +TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" + + +func_to_results = { + "returnEvent()": "16700440330922901039223184000601971290390760458944929668086539975128325467771", + "returnError()": "224292994", + "returnFunctionFromContract()": "890000139", + "returnFunction()": "890000139", + "returnFunctionWithStructure()": "1430834845", + "returnFunctionThroughLocaLVar()": "3781905051", +} + + +def test_enum_max_min(solc_binary_path) -> None: + solc_path = solc_binary_path("0.8.19") + slither = Slither(Path(TEST_DATA_DIR, "selector.sol").as_posix(), solc=solc_path) + + contract = slither.get_contract_from_name("Test")[0] + + for func_name, value in func_to_results.items(): + f = contract.get_function_from_signature(func_name) + assignment = f.slithir_operations[0] + assert ( + isinstance(assignment, Assignment) + and isinstance(assignment.rvalue, Constant) + and assignment.rvalue.value == value + ) diff --git a/tests/unit/slithir/vyper/test_ir_generation.py b/tests/unit/slithir/vyper/test_ir_generation.py index 73c9b5e70b..efcf5ce549 100644 --- a/tests/unit/slithir/vyper/test_ir_generation.py +++ b/tests/unit/slithir/vyper/test_ir_generation.py @@ -35,9 +35,9 @@ def bar(): interface = next(iter(x for x in sl.contracts if x.is_interface)) contract = next(iter(x for x in sl.contracts if not x.is_interface)) func = contract.get_function_from_signature("bar()") - (contract, function) = func.high_level_calls[0] + (contract, ir) = func.high_level_calls[0] assert contract == interface - assert function.signature_str == "foo() returns(int128,uint256)" + assert ir.function.signature_str == "foo() returns(int128,uint256)" def test_phi_entry_point_internal_call(slither_from_vyper_source):