diff --git a/.github/ISSUE_TEMPLATE/false_negative.yml b/.github/ISSUE_TEMPLATE/false_negative.yml new file mode 100644 index 000000000..e11b6ca8d --- /dev/null +++ b/.github/ISSUE_TEMPLATE/false_negative.yml @@ -0,0 +1,61 @@ +--- +body: + - + attributes: + value: | + Please check the issues tab to avoid duplicates. + Thanks for helping make Slither the best it can be! + type: markdown + - + attributes: + label: "What bug did Slither miss and which detector did you anticipate would catch it?" + id: what-happened + type: textarea + validations: + required: true + - + attributes: + label: Frequency + description: How often do you run across this false negative? + options: + - Very Frequently + - Occasionally + - Rarely + - Not sure + id: frequency + type: dropdown + validations: + required: true + - + attributes: + description: "It can be a github repo, etherscan link, or code snippet." + label: "Code example to reproduce the issue:" + placeholder: "`contract A {}`\n" + id: reproduce + type: textarea + validations: + required: true + - + attributes: + description: | + What version of slither are you running? + Run `slither --version` + label: "Version:" + id: version + type: textarea + validations: + required: true + - + attributes: + description: | + Please copy and paste the result output. This + will be automatically formatted into code, so no need for backticks. + render: shell + label: "Relevant log output:" + id: logs + type: textarea +description: "Slither missed a bug it should find." +labels: + - false-negative +name: False Negative" +title: "[False Negative]: " diff --git a/.github/ISSUE_TEMPLATE/false_positive.yml b/.github/ISSUE_TEMPLATE/false_positive.yml new file mode 100644 index 000000000..258a70dfb --- /dev/null +++ b/.github/ISSUE_TEMPLATE/false_positive.yml @@ -0,0 +1,61 @@ +--- +body: + - + attributes: + value: | + Please check the issues tab to avoid duplicates. + Thanks for helping make Slither the best it can be! + type: markdown + - + attributes: + label: "Describe the false alarm that Slither raise and how you know it's inaccurate:" + id: what-happened + type: textarea + validations: + required: true + - + attributes: + label: Frequency + description: How often do you run across this false positive? + options: + - Very Frequently + - Occasionally + - Rarely + - Not sure + id: frequency + type: dropdown + validations: + required: true + - + attributes: + description: "It can be a github repo, etherscan link, or code snippet." + label: "Code example to reproduce the issue:" + placeholder: "`contract A {}`\n" + id: reproduce + type: textarea + validations: + required: true + - + attributes: + description: | + What version of slither are you running? + Run `slither --version` + label: "Version:" + id: version + type: textarea + validations: + required: true + - + attributes: + description: | + Please copy and paste the result output. This + will be automatically formatted into code, so no need for backticks. + render: shell + label: "Relevant log output:" + id: logs + type: textarea +description: "Slither warned of an issue that is not legitimate and does not need to be fixed." +labels: + - false-positive +name: "False Positive" +title: "[False-Positive]: " diff --git a/.github/workflows/features.yml b/.github/workflows/features.yml index b11a137d9..2c112e0aa 100644 --- a/.github/workflows/features.yml +++ b/.github/workflows/features.yml @@ -50,6 +50,7 @@ jobs: pytest tests/test_features.py pytest tests/test_constant_folding.py pytest tests/slithir/test_ternary_expressions.py + pytest tests/slithir/test_operation_reads.py pytest tests/test_functions_ids.py pytest tests/test_function.py pytest tests/test_source_mapping.py diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml index 4f029399e..bc9177802 100644 --- a/.github/workflows/linter.yml +++ b/.github/workflows/linter.yml @@ -22,7 +22,7 @@ concurrency: jobs: build: - name: Pylint + name: Superlinter runs-on: ubuntu-latest steps: diff --git a/.github/workflows/read_storage.yml b/.github/workflows/read_storage.yml index 638a5c38c..b9ff687ff 100644 --- a/.github/workflows/read_storage.yml +++ b/.github/workflows/read_storage.yml @@ -40,7 +40,6 @@ jobs: - name: Install python dependencies run: | pip install ".[dev]" - pip install web3 solc-select install 0.8.1 solc-select install 0.8.10 solc-select use 0.8.1 diff --git a/README.md b/README.md index 1f048a1f7..bce20bfb0 100644 --- a/README.md +++ b/README.md @@ -151,26 +151,27 @@ Num | Detector | What it Detects | Impact | Confidence 61 | `assembly` | [Assembly usage](https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage) | Informational | High 62 | `assert-state-change` | [Assert state change](https://github.com/crytic/slither/wiki/Detector-Documentation#assert-state-change) | Informational | High 63 | `boolean-equal` | [Comparison to boolean constant](https://github.com/crytic/slither/wiki/Detector-Documentation#boolean-equality) | Informational | High -64 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High -65 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High -66 | `function-init-state` | [Function initializing state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#function-initializing-state) | Informational | High -67 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High -68 | `missing-inheritance` | [Missing inheritance](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-inheritance) | Informational | High -69 | `naming-convention` | [Conformity to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High -70 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High -71 | `redundant-statements` | [Redundant statements](https://github.com/crytic/slither/wiki/Detector-Documentation#redundant-statements) | Informational | High -72 | `solc-version` | [Incorrect Solidity version](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity) | Informational | High -73 | `unimplemented-functions` | [Unimplemented functions](https://github.com/crytic/slither/wiki/Detector-Documentation#unimplemented-functions) | Informational | High -74 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variable) | Informational | High -75 | `costly-loop` | [Costly operations in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation#costly-operations-inside-a-loop) | Informational | Medium -76 | `dead-code` | [Functions that are not used](https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code) | Informational | Medium -77 | `reentrancy-unlimited-gas` | [Reentrancy vulnerabilities through send and transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-4) | Informational | Medium -78 | `similar-names` | [Variable names are too similar](https://github.com/crytic/slither/wiki/Detector-Documentation#variable-names-too-similar) | Informational | Medium -79 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium -80 | `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 -81 | `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 -82 | `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 -83 | `var-read-using-this` | [Contract reads its own variable using `this`](https://github.com/crytic/slither/wiki/Vulnerabilities-Description#public-variable-read-in-external-context) | Optimization | High +64 | `cyclomatic-complexity` | [Detects functions with high (> 11) cyclomatic complexity](https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity) | Informational | High +65 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High +66 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High +67 | `function-init-state` | [Function initializing state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#function-initializing-state) | Informational | High +68 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High +69 | `missing-inheritance` | [Missing inheritance](https://github.com/crytic/slither/wiki/Detector-Documentation#missing-inheritance) | Informational | High +70 | `naming-convention` | [Conformity to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High +71 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High +72 | `redundant-statements` | [Redundant statements](https://github.com/crytic/slither/wiki/Detector-Documentation#redundant-statements) | Informational | High +73 | `solc-version` | [Incorrect Solidity version](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity) | Informational | High +74 | `unimplemented-functions` | [Unimplemented functions](https://github.com/crytic/slither/wiki/Detector-Documentation#unimplemented-functions) | Informational | High +75 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variable) | Informational | High +76 | `costly-loop` | [Costly operations in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation#costly-operations-inside-a-loop) | Informational | Medium +77 | `dead-code` | [Functions that are not used](https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code) | Informational | Medium +78 | `reentrancy-unlimited-gas` | [Reentrancy vulnerabilities through send and transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-4) | Informational | Medium +79 | `similar-names` | [Variable names are too similar](https://github.com/crytic/slither/wiki/Detector-Documentation#variable-names-too-similar) | Informational | Medium +80 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium +81 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Optimization | High +82 | `external-function` | [Public function that could be declared external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external) | Optimization | High +83 | `immutable-states` | [State variables that could be declared immutable](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-immutable) | Optimization | High +84 | `var-read-using-this` | [Contract reads its own variable using `this`](https://github.com/crytic/slither/wiki/Vulnerabilities-Description#public-variable-read-in-external-context) | Optimization | High For more information, see - The [Detector Documentation](https://github.com/crytic/slither/wiki/Detector-Documentation) for details on each detector @@ -246,17 +247,19 @@ Slither is licensed and distributed under the AGPLv3 license. [Contact us](mailt - [Slither: A Static Analysis Framework For Smart Contracts](https://arxiv.org/abs/1908.09878), Josselin Feist, Gustavo Grieco, Alex Groce - WETSEB '19 ### External publications -Title | Usage | Authors | Venue ---- | --- | --- | --- +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: 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) [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 +[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/) [Demystifying Loops in Smart Contracts](https://www.microsoft.com/en-us/research/uploads/prod/2020/08/loops_solidity__camera_ready-5f3fec3f15c69.pdf) | Leverage data dependency through Slither | Ben Mariano, Yanju Chen, Yu Feng, Shuvendu Lahiri, Isil Dillig | ASE 20 [Trace-Based Dynamic Gas Estimation of Loops in Smart Contracts](https://ieeexplore.ieee.org/stamp/stamp.jsp?arnumber=9268144) | Use Slither’s CFG to detect loops | Chunmiao Li, Shijie Nie, Yang Cao, Yijun Yu, Zhenjiang Hu | IEEE Open J. Comput. Soc. 1 (2020) -[SAILFISH: Vetting Smart Contract State-Inconsistency Bugs in Seconds](https://arxiv.org/pdf/2104.08638.pdf) | Rely on SlithIR to build a *storage dependency graph* | Priyanka Bose, Dipanjan Das, Yanju Chen, Yu Feng, Christopher Kruegel, and Giovanni Vigna | S&P 22 +[SAILFISH: Vetting Smart Contract State-Inconsistency Bugs in Seconds](https://arxiv.org/pdf/2104.08638.pdf) | Rely on SlithIR to build a *storage dependency graph* | Priyanka Bose, Dipanjan Das, Yanju Chen, Yu Feng, Christopher Kruegel, and Giovanni Vigna | S&P 22 | [Sailfish](https://github.com/ucsb-seclab/sailfish) [SolType: Refinement Types for Arithmetic Overflow in Solidity](https://arxiv.org/abs/2110.00677) | Use Slither as frontend to build refinement type system | Bryan Tan, Benjamin Mariano, Shuvendu K. Lahiri, Isil Dillig, Yu Feng | POPL 22 [Do Not Rug on Me: Leveraging Machine Learning Techniques for Automated Scam Detection](https://www.mdpi.com/2227-7390/10/6/949) | Use Slither to extract tokens' features (mintable, pausable, ..) | Mazorra, Bruno, Victor Adan, and Vanesa Daza | Mathematics 10.6 (2022) +[MANDO: Multi-Level Heterogeneous Graph Embeddings for Fine-Grained Detection of Smart Contract Vulnerabilities](https://arxiv.org/abs/2208.13252) | Use Slither to extract the CFG and call graph | Hoang Nguyen, Nhat-Minh Nguyen, Chunyao Xie, Zahra Ahmadi, Daniel Kudendo, Thanh-Nam Doan and Lingxiao Jiang| IEEE 9th International Conference on Data Science and Advanced Analytics (DSAA, 2022) | [ge-sc](https://github.com/MANDO-Project/ge-sc) +[Automated Auditing of Price Gouging TOD Vulnerabilities in Smart Contracts](https://www.cs.toronto.edu/~fanl/papers/price-icbc22.pdf) | Use Slither to extract the CFG and data dependencies| Sidi Mohamed Beillahi, Eric Keilty, Keerthi Nelaturu, Andreas Veneris, and Fan Long | 2022 IEEE International Conference on Blockchain and Cryptocurrency (ICBC) | [Smart-Contract-Repair](https://github.com/Veneris-Group/TOD-Location-Rectification) If you are using Slither on an academic work, consider applying to the [Crytic $10k Research Prize](https://blog.trailofbits.com/2019/11/13/announcing-the-crytic-10k-research-prize/). diff --git a/scripts/ci_test_printers.sh b/scripts/ci_test_printers.sh index e542286fe..f6eaf0fc8 100755 --- a/scripts/ci_test_printers.sh +++ b/scripts/ci_test_printers.sh @@ -5,11 +5,20 @@ cd tests/ast-parsing/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,human-summary,inheritance,inheritance-graph,slithir,slithir-ssa,vars-and-auth,require,variable-order" +ALL_PRINTERS="cfg,constructor-calls,contract-summary,data-dependency,echidna,function-id,function-summary,modifiers,call-graph,human-summary,inheritance,inheritance-graph,slithir,slithir-ssa,vars-and-auth,require,variable-order,declaration" # Only test 0.5.17 to limit test time for file in *0.5.17-compact.zip; do - if ! slither "$file" --print "$ALL_PRINTERS" > /dev/null 2>&1 ; then + if ! slither "$file" --print "$ALL_PRINTERS" ; then + echo "Printer failed" + echo "$file" + exit 1 + fi +done + +# Only test 0.8.12 to limit test time +for file in *0.8.12-compact.zip; do + if ! slither "$file" --print "declaration" ; then echo "Printer failed" echo "$file" exit 1 diff --git a/setup.py b/setup.py index 3d2fa2a35..f9d100afd 100644 --- a/setup.py +++ b/setup.py @@ -17,6 +17,7 @@ setup( "pycryptodome>=3.4.6", # "crytic-compile>=0.3.0", "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile", + "web3>=6.0.0", ], extras_require={ "dev": [ @@ -27,10 +28,9 @@ setup( "pytest-xdist", "deepdiff", "numpy", - "solc-select>=v1.0.0b1", "openai", "pdoc", - ] + ], }, license="AGPL-3.0", long_description=long_description, diff --git a/slither/__main__.py b/slither/__main__.py index ddc11936c..ca61a8269 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -497,7 +497,7 @@ def parse_args( group_misc.add_argument( "--filter-paths", - help="Comma-separated list of paths for which results will be excluded", + help="Regex filter to exclude detector results matching file path e.g. (mocks/|test/)", action="store", dest="filter_paths", default=defaults_flag_in_config["filter_paths"], diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 2c82f9b58..fd8f761c6 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -88,6 +88,7 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods self._kind: Optional[str] = None self._is_interface: bool = False self._is_library: bool = False + self._is_fully_implemented: bool = False self._signatures: Optional[List[str]] = None self._signatures_declared: Optional[List[str]] = None @@ -113,6 +114,8 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods Dict["StateVariable", Set[Union["StateVariable", "Function"]]] ] = None + self._comments: Optional[str] = None + ################################################################################### ################################################################################### # region General's properties @@ -168,6 +171,39 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods def is_library(self, is_library: bool) -> None: self._is_library = is_library + @property + def comments(self) -> Optional[str]: + """ + Return the comments associated with the contract. + + When using comments, avoid strict text matching, as the solc behavior might change. + For example, for old solc version, the first space after the * is not kept, i.e: + + * @title Test Contract + * @dev Test comment + + Returns + - " @title Test Contract\n @dev Test comment" for newest versions + - "@title Test Contract\n@dev Test comment" for older versions + + + Returns: + the comment as a string + """ + return self._comments + + @comments.setter + def comments(self, comments: str): + self._comments = comments + + @property + def is_fully_implemented(self) -> bool: + return self._is_fully_implemented + + @is_fully_implemented.setter + def is_fully_implemented(self, is_fully_implemented: bool): + self._is_fully_implemented = is_fully_implemented + # endregion ################################################################################### ################################################################################### diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index 4c8742b22..fceab7855 100644 --- a/slither/core/source_mapping/source_mapping.py +++ b/slither/core/source_mapping/source_mapping.py @@ -18,7 +18,7 @@ if TYPE_CHECKING: # pylint: disable=too-many-instance-attributes class Source: - def __init__(self) -> None: + def __init__(self, compilation_unit: "SlitherCompilationUnit") -> None: self.start: int = 0 self.length: int = 0 self.filename: Filename = Filename("", "", "", "") @@ -27,7 +27,7 @@ class Source: self.starting_column: int = 0 self.ending_column: int = 0 self.end: int = 0 - self.compilation_unit: Optional["SlitherCompilationUnit"] = None + self.compilation_unit = compilation_unit def to_json(self) -> Dict: return { @@ -51,17 +51,13 @@ class Source: filename_relative: str = self.filename.relative if self.filename.relative else "" return f"{markdown_root}{filename_relative}{lines}" - def to_detailled_str(self) -> str: + def to_detailed_str(self) -> str: lines = self._get_lines_str() filename_short: str = self.filename.short if self.filename.short else "" return f"{filename_short}{lines} ({self.starting_column} - {self.ending_column})" def _get_lines_str(self, line_descr: str = "") -> str: - # If the compilation unit was not initialized, it means that the set_offset was never called - # on the corresponding object, which should not happen - assert self.compilation_unit is not None - line_prefix = self.compilation_unit.core.line_prefix lines = self.lines @@ -129,6 +125,7 @@ def _compute_line( Not done in an efficient way """ + start_line, starting_column = compilation_unit.core.crytic_compile.get_line_from_offset( filename, start ) @@ -151,7 +148,7 @@ def _convert_source_mapping( position = re.findall("([0-9]*):([0-9]*):([-]?[0-9]*)", offset) if len(position) != 1: - return Source() + return Source(compilation_unit) s, l, f = position[0] s = int(s) @@ -159,7 +156,7 @@ def _convert_source_mapping( f = int(f) if f not in sourceUnits: - new_source = Source() + new_source = Source(compilation_unit) new_source.start = s new_source.length = l return new_source @@ -173,7 +170,7 @@ def _convert_source_mapping( (lines, starting_column, ending_column) = _compute_line(compilation_unit, filename, s, l) - new_source = Source() + new_source = Source(compilation_unit) new_source.start = s new_source.length = l new_source.filename = filename @@ -182,28 +179,22 @@ def _convert_source_mapping( new_source.starting_column = starting_column new_source.ending_column = ending_column new_source.end = new_source.start + l + return new_source class SourceMapping(Context, metaclass=ABCMeta): def __init__(self) -> None: super().__init__() - # self._source_mapping: Optional[Dict] = None - self.source_mapping: Source = Source() + self.source_mapping: Optional[Source] = None self.references: List[Source] = [] def set_offset( self, offset: Union["Source", str], compilation_unit: "SlitherCompilationUnit" ) -> None: + assert compilation_unit if isinstance(offset, Source): - self.source_mapping.start = offset.start - self.source_mapping.length = offset.length - self.source_mapping.filename = offset.filename - self.source_mapping.is_dependency = offset.is_dependency - self.source_mapping.lines = offset.lines - self.source_mapping.starting_column = offset.starting_column - self.source_mapping.ending_column = offset.ending_column - self.source_mapping.end = offset.end + self.source_mapping = offset else: self.source_mapping = _convert_source_mapping(offset, compilation_unit) self.source_mapping.compilation_unit = compilation_unit diff --git a/slither/detectors/attributes/incorrect_solc.py b/slither/detectors/attributes/incorrect_solc.py index 393fbbfe4..eaf40bf21 100644 --- a/slither/detectors/attributes/incorrect_solc.py +++ b/slither/detectors/attributes/incorrect_solc.py @@ -47,10 +47,7 @@ We also recommend avoiding complex `pragma` statement.""" # region wiki_recommendation WIKI_RECOMMENDATION = """ Deploy with any of the following Solidity versions: -- 0.5.16 - 0.5.17 -- 0.6.11 - 0.6.12 -- 0.7.5 - 0.7.6 -- 0.8.16 +- 0.8.18 The recommendations take into account: - Risks related to recent releases @@ -66,13 +63,14 @@ Consider using the latest version of Solidity for testing.""" OLD_VERSION_TXT = "allows old versions" LESS_THAN_TXT = "uses lesser than" - TOO_RECENT_VERSION_TXT = "necessitates a version too recent to be trusted. Consider deploying with 0.6.12/0.7.6/0.8.16" BUGGY_VERSION_TXT = ( "is known to contain severe issues (https://solidity.readthedocs.io/en/latest/bugs.html)" ) # Indicates the allowed versions. Must be formatted in increasing order. - ALLOWED_VERSIONS = ["0.5.16", "0.5.17", "0.6.11", "0.6.12", "0.7.5", "0.7.6", "0.8.16"] + ALLOWED_VERSIONS = ["0.8.18"] + + TOO_RECENT_VERSION_TXT = f"necessitates a version too recent to be trusted. Consider deploying with {'/'.join(ALLOWED_VERSIONS)}." # Indicates the versions that should not be used. BUGGY_VERSIONS = [ diff --git a/slither/detectors/reentrancy/reentrancy_eth.py b/slither/detectors/reentrancy/reentrancy_eth.py index 73622cf54..ccb668837 100644 --- a/slither/detectors/reentrancy/reentrancy_eth.py +++ b/slither/detectors/reentrancy/reentrancy_eth.py @@ -38,7 +38,7 @@ Do not report reentrancies that don't involve Ether (see `reentrancy-no-eth`)""" ```solidity function withdrawBalance(){ // send userBalance[msg.sender] Ether to msg.sender - // if mgs.sender is a contract, it will call its fallback function + // if msg.sender is a contract, it will call its fallback function if( ! (msg.sender.call.value(userBalance[msg.sender])() ) ){ throw; } diff --git a/slither/printers/summary/declaration.py b/slither/printers/summary/declaration.py index 9266d5580..c7c4798d5 100644 --- a/slither/printers/summary/declaration.py +++ b/slither/printers/summary/declaration.py @@ -21,37 +21,37 @@ class Declaration(AbstractPrinter): txt += "\n# Contracts\n" for contract in compilation_unit.contracts: txt += f"# {contract.name}\n" - txt += f"\t- Declaration: {get_definition(contract, compilation_unit.core.crytic_compile).to_detailled_str()}\n" - txt += f"\t- Implementation: {get_implementation(contract).to_detailled_str()}\n" + txt += f"\t- Declaration: {get_definition(contract, compilation_unit.core.crytic_compile).to_detailed_str()}\n" + txt += f"\t- Implementation: {get_implementation(contract).to_detailed_str()}\n" txt += ( - f"\t- References: {[x.to_detailled_str() for x in get_references(contract)]}\n" + f"\t- References: {[x.to_detailed_str() for x in get_references(contract)]}\n" ) txt += "\n\t## Function\n" for func in contract.functions: txt += f"\t\t- {func.canonical_name}\n" - txt += f"\t\t\t- Declaration: {get_definition(func, compilation_unit.core.crytic_compile).to_detailled_str()}\n" - txt += ( - f"\t\t\t- Implementation: {get_implementation(func).to_detailled_str()}\n" - ) - txt += f"\t\t\t- References: {[x.to_detailled_str() for x in get_references(func)]}\n" + txt += f"\t\t\t- Declaration: {get_definition(func, compilation_unit.core.crytic_compile).to_detailed_str()}\n" + txt += f"\t\t\t- Implementation: {get_implementation(func).to_detailed_str()}\n" + txt += f"\t\t\t- References: {[x.to_detailed_str() for x in get_references(func)]}\n" txt += "\n\t## State variables\n" for var in contract.state_variables: txt += f"\t\t- {var.name}\n" - txt += f"\t\t\t- Declaration: {get_definition(var, compilation_unit.core.crytic_compile).to_detailled_str()}\n" - txt += f"\t\t\t- Implementation: {get_implementation(var).to_detailled_str()}\n" - txt += f"\t\t\t- References: {[x.to_detailled_str() for x in get_references(var)]}\n" + txt += f"\t\t\t- Declaration: {get_definition(var, compilation_unit.core.crytic_compile).to_detailed_str()}\n" + txt += f"\t\t\t- Implementation: {get_implementation(var).to_detailed_str()}\n" + txt += f"\t\t\t- References: {[x.to_detailed_str() for x in get_references(var)]}\n" txt += "\n\t## Structures\n" for st in contract.structures: txt += f"\t\t- {st.name}\n" - txt += f"\t\t\t- Declaration: {get_definition(st, compilation_unit.core.crytic_compile).to_detailled_str()}\n" - txt += f"\t\t\t- Implementation: {get_implementation(st).to_detailled_str()}\n" - txt += f"\t\t\t- References: {[x.to_detailled_str() for x in get_references(st)]}\n" + txt += f"\t\t\t- Declaration: {get_definition(st, compilation_unit.core.crytic_compile).txt}\n" + txt += f"\t\t\t- Implementation: {get_implementation(st).to_detailed_str()}\n" + txt += ( + f"\t\t\t- References: {[x.to_detailed_str() for x in get_references(st)]}\n" + ) self.info(txt) res = self.generate_output(txt) diff --git a/slither/printers/summary/evm.py b/slither/printers/summary/evm.py index 660d91204..0e5ce58d7 100644 --- a/slither/printers/summary/evm.py +++ b/slither/printers/summary/evm.py @@ -21,8 +21,12 @@ def _extract_evm_info(slither): CFG = load_evm_cfg_builder() for contract in slither.contracts_derived: - contract_bytecode_runtime = contract.scope.bytecode_runtime(contract.name) - contract_srcmap_runtime = contract.scope.srcmap_runtime(contract.name) + contract_bytecode_runtime = contract.file_scope.bytecode_runtime( + contract.compilation_unit.crytic_compile_compilation_unit, contract.name + ) + contract_srcmap_runtime = contract.file_scope.srcmap_runtime( + contract.compilation_unit.crytic_compile_compilation_unit, contract.name + ) cfg = CFG(contract_bytecode_runtime) evm_info["cfg", contract.name] = cfg evm_info["mapping", contract.name] = generate_source_to_evm_ins_mapping( @@ -32,8 +36,12 @@ def _extract_evm_info(slither): contract.source_mapping.filename.absolute, ) - contract_bytecode_init = contract.scope.bytecode_init(contract.name) - contract_srcmap_init = contract.scope.srcmap_init(contract.name) + contract_bytecode_init = contract.file_scope.bytecode_init( + contract.compilation_unit.crytic_compile_compilation_unit, contract.name + ) + contract_srcmap_init = contract.file_scope.srcmap_init( + contract.compilation_unit.crytic_compile_compilation_unit, contract.name + ) cfg_init = CFG(contract_bytecode_init) evm_info["cfg_init", contract.name] = cfg_init diff --git a/slither/printers/summary/slithir.py b/slither/printers/summary/slithir.py index 6f64d7624..be9ebc8f5 100644 --- a/slither/printers/summary/slithir.py +++ b/slither/printers/summary/slithir.py @@ -46,7 +46,7 @@ class PrinterSlithIR(AbstractPrinter): txt += f"\tModifier {modifier.canonical_name}\n" txt += _print_function(modifier) if compilation_unit.functions_top_level: - txt += "Top level functions" + txt += "Top level functions\n" for function in compilation_unit.functions_top_level: txt += f"\tFunction {function.canonical_name}\n" txt += _print_function(function) diff --git a/slither/slither.py b/slither/slither.py index 3e44944b3..85f852e1d 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -188,6 +188,16 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes instance = detector_class(compilation_unit, self, logger_detector) self._detectors.append(instance) + def unregister_detector(self, detector_class: Type[AbstractDetector]) -> None: + """ + :param detector_class: Class inheriting from `AbstractDetector`. + """ + + for obj in self._detectors: + if isinstance(obj, detector_class): + self._detectors.remove(obj) + return + def register_printer(self, printer_class: Type[AbstractPrinter]) -> None: """ :param printer_class: Class inheriting from `AbstractPrinter`. @@ -197,6 +207,16 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes instance = printer_class(self, logger_printer) self._printers.append(instance) + def unregister_printer(self, printer_class: Type[AbstractPrinter]) -> None: + """ + :param printer_class: Class inheriting from `AbstractPrinter`. + """ + + for obj in self._printers: + if isinstance(obj, printer_class): + self._printers.remove(obj) + return + def run_detectors(self) -> List[Dict]: """ :return: List of registered detectors results. diff --git a/slither/slithir/operations/high_level_call.py b/slither/slithir/operations/high_level_call.py index d707e11b3..5d654fc80 100644 --- a/slither/slithir/operations/high_level_call.py +++ b/slither/slithir/operations/high_level_call.py @@ -79,7 +79,7 @@ class HighLevelCall(Call, OperationWithLValue): def read(self) -> List[SourceMapping]: all_read = [self.destination, self.call_gas, self.call_value] + self._unroll(self.arguments) # remove None - return [x for x in all_read if x] + [self.destination] + return [x for x in all_read if x] @property def destination(self) -> Union[Variable, SolidityVariable, Contract]: diff --git a/slither/slithir/operations/new_contract.py b/slither/slithir/operations/new_contract.py index 8d3c949df..10fa91efd 100644 --- a/slither/slithir/operations/new_contract.py +++ b/slither/slithir/operations/new_contract.py @@ -54,7 +54,9 @@ class NewContract(Call, OperationWithLValue): # pylint: disable=too-many-instan @property def read(self) -> List[Any]: - return self._unroll(self.arguments) + all_read = [self.call_salt, self.call_value] + self._unroll(self.arguments) + # remove None + return [x for x in all_read if x] @property def contract_created(self) -> Contract: diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index b9dbe9a9f..f3202d00c 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -170,6 +170,7 @@ class ContractSolc(CallerContextExpression): elif attributes["contractKind"] == "library": self._contract.is_library = True self._contract.contract_kind = attributes["contractKind"] + self._contract.is_fully_implemented = attributes["fullyImplemented"] self._linearized_base_contracts = attributes["linearizedBaseContracts"] # self._contract.fullyImplemented = attributes["fullyImplemented"] @@ -782,12 +783,35 @@ class ContractSolc(CallerContextExpression): self._customErrorParsed = [] def _handle_comment(self, attributes: Dict) -> None: + """ + Save the contract comment in self.comments + And handle custom slither comments + + Args: + attributes: + + Returns: + + """ + # Old solc versions store the comment in attributes["documentation"] + # More recent ones store it in attributes["documentation"]["text"] if ( "documentation" in attributes and attributes["documentation"] is not None - and "text" in attributes["documentation"] + and ( + "text" in attributes["documentation"] + or isinstance(attributes["documentation"], str) + ) ): - candidates = attributes["documentation"]["text"].replace("\n", ",").split(",") + text = ( + attributes["documentation"] + if isinstance(attributes["documentation"], str) + else attributes["documentation"]["text"] + ) + self._contract.comments = text + + # Look for custom comments + candidates = text.replace("\n", ",").split(",") for candidate in candidates: if "@custom:security isDelegatecallProxy" in candidate: diff --git a/slither/tools/read_storage/read_storage.py b/slither/tools/read_storage/read_storage.py index bb662c4d5..387aa619a 100644 --- a/slither/tools/read_storage/read_storage.py +++ b/slither/tools/read_storage/read_storage.py @@ -1,30 +1,22 @@ import logging -import sys from math import floor -from typing import Callable, Optional, Tuple, Union, List, Dict, Any - -try: - from web3 import Web3 - from eth_typing.evm import ChecksumAddress - from eth_abi import decode_single, encode_abi - from eth_utils import keccak - from .utils import ( - get_offset_value, - get_storage_data, - coerce_type, - ) -except ImportError: - print("ERROR: in order to use slither-read-storage, you need to install web3") - print("$ pip3 install web3 --user\n") - sys.exit(-1) +from typing import Any, Callable, Dict, List, Optional, Tuple, Union import dataclasses -from slither.utils.myprettytable import MyPrettyTable -from slither.core.solidity_types.type import Type -from slither.core.solidity_types import ArrayType, ElementaryType, UserDefinedType, MappingType + +from eth_abi import decode, encode +from eth_typing.evm import ChecksumAddress +from eth_utils import keccak +from web3 import Web3 + from slither.core.declarations import Contract, Structure +from slither.core.solidity_types import ArrayType, ElementaryType, MappingType, UserDefinedType +from slither.core.solidity_types.type import Type from slither.core.variables.state_variable import StateVariable from slither.core.variables.structure_variable import StructureVariable +from slither.utils.myprettytable import MyPrettyTable + +from .utils import coerce_type, get_offset_value, get_storage_data logging.basicConfig() logger = logging.getLogger("Slither-read-storage") @@ -92,7 +84,7 @@ class SlitherReadStorage: if not self.storage_address: raise ValueError if not self._checksum_address: - self._checksum_address = self.web3.toChecksumAddress(self.storage_address) + self._checksum_address = self.web3.to_checksum_address(self.storage_address) return self._checksum_address @property @@ -449,7 +441,7 @@ class SlitherReadStorage: if "int" in key_type: # without this eth_utils encoding fails key = int(key) key = coerce_type(key_type, key) - slot = keccak(encode_abi([key_type, "uint256"], [key, decode_single("uint256", slot)])) + slot = keccak(encode([key_type, "uint256"], [key, decode("uint256", slot)])) if isinstance(target_variable_type.type_to, UserDefinedType) and isinstance( target_variable_type.type_to.type, Structure @@ -471,7 +463,7 @@ class SlitherReadStorage: deep_key = int(deep_key) # If deep map, will be keccak256(abi.encode(key1, keccak256(abi.encode(key0, uint(slot))))) - slot = keccak(encode_abi([key_type, "bytes32"], [deep_key, slot])) + slot = keccak(encode([key_type, "bytes32"], [deep_key, slot])) # mapping(elem => mapping(elem => elem)) target_variable_type_type_to_type_to = target_variable_type.type_to.type_to diff --git a/slither/tools/read_storage/utils/__init__.py b/slither/tools/read_storage/utils/__init__.py index 2fb43c8b8..9a624a4c7 100644 --- a/slither/tools/read_storage/utils/__init__.py +++ b/slither/tools/read_storage/utils/__init__.py @@ -1,5 +1 @@ -from .utils import ( - get_offset_value, - get_storage_data, - coerce_type, -) +from .utils import coerce_type, get_offset_value, get_storage_data diff --git a/slither/utils/source_mapping.py b/slither/utils/source_mapping.py index 26ad7c0d2..b117cd5f7 100644 --- a/slither/utils/source_mapping.py +++ b/slither/utils/source_mapping.py @@ -35,7 +35,7 @@ def get_definition(target: SourceMapping, crytic_compile: CryticCompile) -> Sour target.source_mapping.filename, target.source_mapping.start + start_offset + len(pattern) ) - s = Source() + s = Source(target.source_mapping.compilation_unit) s.start = target.source_mapping.start + start_offset s.length = len(pattern) s.filename = target.source_mapping.filename @@ -44,8 +44,7 @@ def get_definition(target: SourceMapping, crytic_compile: CryticCompile) -> Sour s.starting_column = starting_column s.ending_column = ending_column s.end = s.start + s.length - s.compilation_unit = target.compilation_unit - + s.txt = txt return s diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index 8263a6e33..90905be4e 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -226,7 +226,13 @@ class ExpressionToSlithIR(ExpressionVisitor): operation.set_expression(expression) self._result.append(operation) set_val(expression, left) - elif expression.type and expression.expression_return_type: + elif isinstance(left.type, ArrayType): + # Special case for init of array, when the right has only one element + operation = InitArray([right], left) + operation.set_expression(expression) + self._result.append(operation) + set_val(expression, left) + else: operation = convert_assignment( left, right, expression.type, expression.expression_return_type ) diff --git a/tests/custom_comments/contract_comment.sol b/tests/custom_comments/contract_comment.sol new file mode 100644 index 000000000..8f0fb5233 --- /dev/null +++ b/tests/custom_comments/contract_comment.sol @@ -0,0 +1,7 @@ +/** + * @title Test Contract + * @dev Test comment + */ +contract A{ + +} \ No newline at end of file diff --git a/tests/detectors/solc-version/0.5.16/dynamic_1.sol.0.5.16.IncorrectSolc.json b/tests/detectors/solc-version/0.5.16/dynamic_1.sol.0.5.16.IncorrectSolc.json index 28381e3ca..599392dc8 100644 --- a/tests/detectors/solc-version/0.5.16/dynamic_1.sol.0.5.16.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.5.16/dynamic_1.sol.0.5.16.IncorrectSolc.json @@ -1,5 +1,15 @@ [ [ + { + "elements": [], + "description": "solc-0.5.16 is not recommended for deployment\n", + "markdown": "solc-0.5.16 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "94ddf430efb860e471a768a108c851848fa998e8a2c489c6fb23ed71d3ef4b09", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" + }, { "elements": [ { diff --git a/tests/detectors/solc-version/0.5.16/dynamic_2.sol.0.5.16.IncorrectSolc.json b/tests/detectors/solc-version/0.5.16/dynamic_2.sol.0.5.16.IncorrectSolc.json index 19ab5889f..8eacb5663 100644 --- a/tests/detectors/solc-version/0.5.16/dynamic_2.sol.0.5.16.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.5.16/dynamic_2.sol.0.5.16.IncorrectSolc.json @@ -38,6 +38,16 @@ "check": "solc-version", "impact": "Informational", "confidence": "High" + }, + { + "elements": [], + "description": "solc-0.5.16 is not recommended for deployment\n", + "markdown": "solc-0.5.16 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "94ddf430efb860e471a768a108c851848fa998e8a2c489c6fb23ed71d3ef4b09", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" } ] ] \ No newline at end of file diff --git a/tests/detectors/solc-version/0.5.16/static.sol.0.5.16.IncorrectSolc.json b/tests/detectors/solc-version/0.5.16/static.sol.0.5.16.IncorrectSolc.json index 5825bcacc..df4bac19d 100644 --- a/tests/detectors/solc-version/0.5.16/static.sol.0.5.16.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.5.16/static.sol.0.5.16.IncorrectSolc.json @@ -1,3 +1,49 @@ [ - [] + [ + { + "elements": [ + { + "type": "pragma", + "name": "0.5.16", + "source_mapping": { + "start": 0, + "length": 23, + "filename_relative": "tests/detectors/solc-version/0.5.16/static.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/solc-version/0.5.16/static.sol", + "is_dependency": false, + "lines": [ + 1 + ], + "starting_column": 1, + "ending_column": 24 + }, + "type_specific_fields": { + "directive": [ + "solidity", + "0.5", + ".16" + ] + } + } + ], + "description": "Pragma version0.5.16 (tests/detectors/solc-version/0.5.16/static.sol#1) allows old versions\n", + "markdown": "Pragma version[0.5.16](tests/detectors/solc-version/0.5.16/static.sol#L1) allows old versions\n", + "first_markdown_element": "tests/detectors/solc-version/0.5.16/static.sol#L1", + "id": "2407d991de90e57d2f6b6bdbc61bb939845a5c0bb2d82910ed4c49abff2ab6e3", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" + }, + { + "elements": [], + "description": "solc-0.5.16 is not recommended for deployment\n", + "markdown": "solc-0.5.16 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "94ddf430efb860e471a768a108c851848fa998e8a2c489c6fb23ed71d3ef4b09", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" + } + ] ] \ No newline at end of file diff --git a/tests/detectors/solc-version/0.6.11/dynamic_1.sol.0.6.11.IncorrectSolc.json b/tests/detectors/solc-version/0.6.11/dynamic_1.sol.0.6.11.IncorrectSolc.json index b182c02c7..a71fa6406 100644 --- a/tests/detectors/solc-version/0.6.11/dynamic_1.sol.0.6.11.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.6.11/dynamic_1.sol.0.6.11.IncorrectSolc.json @@ -35,6 +35,16 @@ "check": "solc-version", "impact": "Informational", "confidence": "High" + }, + { + "elements": [], + "description": "solc-0.6.11 is not recommended for deployment\n", + "markdown": "solc-0.6.11 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "bafd522d637977886f038e619ad47c1987efedc6c4c24515e6e27b23585535bd", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" } ] ] \ No newline at end of file diff --git a/tests/detectors/solc-version/0.6.11/dynamic_2.sol.0.6.11.IncorrectSolc.json b/tests/detectors/solc-version/0.6.11/dynamic_2.sol.0.6.11.IncorrectSolc.json index 4f844e25e..d6cac106b 100644 --- a/tests/detectors/solc-version/0.6.11/dynamic_2.sol.0.6.11.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.6.11/dynamic_2.sol.0.6.11.IncorrectSolc.json @@ -38,6 +38,16 @@ "check": "solc-version", "impact": "Informational", "confidence": "High" + }, + { + "elements": [], + "description": "solc-0.6.11 is not recommended for deployment\n", + "markdown": "solc-0.6.11 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "bafd522d637977886f038e619ad47c1987efedc6c4c24515e6e27b23585535bd", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" } ] ] \ No newline at end of file diff --git a/tests/detectors/solc-version/0.6.11/static.sol.0.6.11.IncorrectSolc.json b/tests/detectors/solc-version/0.6.11/static.sol.0.6.11.IncorrectSolc.json index 5825bcacc..61ea21e16 100644 --- a/tests/detectors/solc-version/0.6.11/static.sol.0.6.11.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.6.11/static.sol.0.6.11.IncorrectSolc.json @@ -1,3 +1,49 @@ [ - [] + [ + { + "elements": [ + { + "type": "pragma", + "name": "0.6.11", + "source_mapping": { + "start": 0, + "length": 23, + "filename_relative": "tests/detectors/solc-version/0.6.11/static.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/solc-version/0.6.11/static.sol", + "is_dependency": false, + "lines": [ + 1 + ], + "starting_column": 1, + "ending_column": 24 + }, + "type_specific_fields": { + "directive": [ + "solidity", + "0.6", + ".11" + ] + } + } + ], + "description": "Pragma version0.6.11 (tests/detectors/solc-version/0.6.11/static.sol#1) allows old versions\n", + "markdown": "Pragma version[0.6.11](tests/detectors/solc-version/0.6.11/static.sol#L1) allows old versions\n", + "first_markdown_element": "tests/detectors/solc-version/0.6.11/static.sol#L1", + "id": "ad7b24eed22ac098a57ae02ade0ccffb4cb094e851effe93cad1d0a65b489816", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" + }, + { + "elements": [], + "description": "solc-0.6.11 is not recommended for deployment\n", + "markdown": "solc-0.6.11 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "bafd522d637977886f038e619ad47c1987efedc6c4c24515e6e27b23585535bd", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" + } + ] ] \ No newline at end of file diff --git a/tests/detectors/solc-version/0.7.6/dynamic_1.sol.0.7.6.IncorrectSolc.json b/tests/detectors/solc-version/0.7.6/dynamic_1.sol.0.7.6.IncorrectSolc.json index aafb7b5d1..763302445 100644 --- a/tests/detectors/solc-version/0.7.6/dynamic_1.sol.0.7.6.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.7.6/dynamic_1.sol.0.7.6.IncorrectSolc.json @@ -1,5 +1,15 @@ [ [ + { + "elements": [], + "description": "solc-0.7.6 is not recommended for deployment\n", + "markdown": "solc-0.7.6 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "ddb8ee36d9dd69b14eab702506268f8f9ef3283777d042e197277e29407b386e", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" + }, { "elements": [ { diff --git a/tests/detectors/solc-version/0.7.6/dynamic_2.sol.0.7.6.IncorrectSolc.json b/tests/detectors/solc-version/0.7.6/dynamic_2.sol.0.7.6.IncorrectSolc.json index 4aea32763..516f3142b 100644 --- a/tests/detectors/solc-version/0.7.6/dynamic_2.sol.0.7.6.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.7.6/dynamic_2.sol.0.7.6.IncorrectSolc.json @@ -38,6 +38,16 @@ "check": "solc-version", "impact": "Informational", "confidence": "High" + }, + { + "elements": [], + "description": "solc-0.7.6 is not recommended for deployment\n", + "markdown": "solc-0.7.6 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "ddb8ee36d9dd69b14eab702506268f8f9ef3283777d042e197277e29407b386e", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" } ] ] \ No newline at end of file diff --git a/tests/detectors/solc-version/0.7.6/static.sol.0.7.6.IncorrectSolc.json b/tests/detectors/solc-version/0.7.6/static.sol.0.7.6.IncorrectSolc.json index 5825bcacc..426786fff 100644 --- a/tests/detectors/solc-version/0.7.6/static.sol.0.7.6.IncorrectSolc.json +++ b/tests/detectors/solc-version/0.7.6/static.sol.0.7.6.IncorrectSolc.json @@ -1,3 +1,49 @@ [ - [] + [ + { + "elements": [], + "description": "solc-0.7.6 is not recommended for deployment\n", + "markdown": "solc-0.7.6 is not recommended for deployment\n", + "first_markdown_element": "", + "id": "ddb8ee36d9dd69b14eab702506268f8f9ef3283777d042e197277e29407b386e", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" + }, + { + "elements": [ + { + "type": "pragma", + "name": "0.7.6", + "source_mapping": { + "start": 0, + "length": 22, + "filename_relative": "tests/detectors/solc-version/0.7.6/static.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/solc-version/0.7.6/static.sol", + "is_dependency": false, + "lines": [ + 1 + ], + "starting_column": 1, + "ending_column": 23 + }, + "type_specific_fields": { + "directive": [ + "solidity", + "0.7", + ".6" + ] + } + } + ], + "description": "Pragma version0.7.6 (tests/detectors/solc-version/0.7.6/static.sol#1) allows old versions\n", + "markdown": "Pragma version[0.7.6](tests/detectors/solc-version/0.7.6/static.sol#L1) allows old versions\n", + "first_markdown_element": "tests/detectors/solc-version/0.7.6/static.sol#L1", + "id": "e7a5c0ee3d0af7a59907908f88499f79435e302745284c6279167a1cc209b4ed", + "check": "solc-version", + "impact": "Informational", + "confidence": "High" + } + ] ] \ No newline at end of file diff --git a/tests/function_features/abstract.sol b/tests/function_features/abstract.sol new file mode 100644 index 000000000..b29f683c4 --- /dev/null +++ b/tests/function_features/abstract.sol @@ -0,0 +1,5 @@ +pragma solidity ^0.8.0; + +abstract contract ExplicitAbstract{ + function f() virtual public; +} diff --git a/tests/function_features/implicit_abstract.sol b/tests/function_features/implicit_abstract.sol new file mode 100644 index 000000000..c46ccd821 --- /dev/null +++ b/tests/function_features/implicit_abstract.sol @@ -0,0 +1,5 @@ +pragma solidity ^0.5.0; + +contract ImplicitAbstract{ + function f() public; +} \ No newline at end of file diff --git a/tests/slithir/operation_reads.sol b/tests/slithir/operation_reads.sol new file mode 100644 index 000000000..22adc2288 --- /dev/null +++ b/tests/slithir/operation_reads.sol @@ -0,0 +1,17 @@ + +contract Placeholder { + constructor() payable {} +} + +contract NewContract { + bytes32 internal constant state_variable_read = bytes32(0); + + function readAllStateVariables() external { + new Placeholder{salt: state_variable_read} (); + } + + function readAllLocalVariables() external { + bytes32 local_variable_read = bytes32(0); + new Placeholder{salt: local_variable_read} (); + } +} \ No newline at end of file diff --git a/tests/slithir/test_operation_reads.py b/tests/slithir/test_operation_reads.py new file mode 100644 index 000000000..aa183333f --- /dev/null +++ b/tests/slithir/test_operation_reads.py @@ -0,0 +1,49 @@ +from collections import namedtuple +from slither import Slither +from slither.slithir.operations import Operation, NewContract + + +def check_num_local_vars_read(function, slithir_op: Operation, num_reads_expected: int): + for node in function.nodes: + for operation in node.irs: + if isinstance(operation, slithir_op): + assert len(operation.read) == num_reads_expected + assert len(node.local_variables_read) == num_reads_expected + + +def check_num_states_vars_read(function, slithir_op: Operation, num_reads_expected: int): + for node in function.nodes: + for operation in node.irs: + if isinstance(operation, slithir_op): + assert len(operation.read) == num_reads_expected + assert len(node.state_variables_read) == num_reads_expected + + +OperationTest = namedtuple("OperationTest", "contract_name slithir_op") + +OPERATION_TEST = [OperationTest("NewContract", NewContract)] + + +def test_operation_reads() -> None: + """ + Every slithir operation has its own contract and reads all local and state variables in readAllLocalVariables and readAllStateVariables, respectively. + """ + slither = Slither("./tests/slithir/operation_reads.sol") + + for op_test in OPERATION_TEST: + print(op_test) + available = slither.get_contract_from_name(op_test.contract_name) + assert len(available) == 1 + target = available[0] + + num_state_variables = len(target.state_variables_ordered) + state_function = target.get_function_from_signature("readAllStateVariables()") + check_num_states_vars_read(state_function, op_test.slithir_op, num_state_variables) + + local_function = target.get_function_from_signature("readAllLocalVariables()") + num_local_vars = len(local_function.local_variables) + check_num_local_vars_read(local_function, op_test.slithir_op, num_local_vars) + + +if __name__ == "__main__": + test_operation_reads() diff --git a/tests/slithir/test_ternary_expressions.py b/tests/slithir/test_ternary_expressions.py index 17cac6b2f..a1f00eb4f 100644 --- a/tests/slithir/test_ternary_expressions.py +++ b/tests/slithir/test_ternary_expressions.py @@ -1,3 +1,4 @@ +from solc_select import solc_select from slither import Slither from slither.core.cfg.node import NodeType from slither.slithir.operations import Assignment @@ -6,6 +7,7 @@ from slither.core.expressions import AssignmentOperation, TupleExpression # pylint: disable=too-many-nested-blocks def test_ternary_conversions() -> None: """This tests that true and false sons define the same number of variables that the father node declares""" + solc_select.switch_global_version("0.8.0", always_install=True) slither = Slither("./tests/slithir/ternary_expressions.sol") for contract in slither.contracts: for function in contract.functions: diff --git a/tests/test_features.py b/tests/test_features.py index 2716d8457..20393df38 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -32,7 +32,7 @@ def test_node() -> None: def test_collision() -> None: - + solc_select.switch_global_version("0.8.0", always_install=True) standard_json = SolcStandardJson() standard_json.add_source_file("./tests/collisions/a.sol") standard_json.add_source_file("./tests/collisions/b.sol") @@ -44,6 +44,7 @@ def test_collision() -> None: def test_cycle() -> None: + solc_select.switch_global_version("0.8.0", always_install=True) slither = Slither("./tests/test_cyclic_import/a.sol") _run_all_detectors(slither) @@ -75,6 +76,36 @@ def test_upgradeable_comments() -> None: assert v1.upgradeable_version == "version_1" +def test_contract_comments() -> None: + comments = " @title Test Contract\n @dev Test comment" + + solc_select.switch_global_version("0.8.10", always_install=True) + slither = Slither("./tests/custom_comments/contract_comment.sol") + compilation_unit = slither.compilation_units[0] + contract = compilation_unit.get_contract_from_name("A")[0] + + assert contract.comments == comments + + # Old solc versions have a different parsing of comments + # the initial space (after *) is also not kept on every line + comments = "@title Test Contract\n@dev Test comment" + solc_select.switch_global_version("0.5.16", always_install=True) + slither = Slither("./tests/custom_comments/contract_comment.sol") + compilation_unit = slither.compilation_units[0] + contract = compilation_unit.get_contract_from_name("A")[0] + + assert contract.comments == comments + + # Test with legacy AST + comments = "@title Test Contract\n@dev Test comment" + solc_select.switch_global_version("0.5.16", always_install=True) + slither = Slither("./tests/custom_comments/contract_comment.sol", solc_force_legacy_json=True) + compilation_unit = slither.compilation_units[0] + contract = compilation_unit.get_contract_from_name("A")[0] + + assert contract.comments == comments + + def test_using_for_top_level_same_name() -> None: solc_select.switch_global_version("0.8.15", always_install=True) slither = Slither("./tests/ast-parsing/using-for-3-0.8.0.sol") @@ -171,3 +202,18 @@ def test_using_for_global_collision() -> None: compilation = CryticCompile(standard_json) sl = Slither(compilation) _run_all_detectors(sl) + + +def test_abstract_contract() -> None: + solc_select.switch_global_version("0.8.0", always_install=True) + slither = Slither("./tests/function_features/abstract.sol") + assert not slither.contracts[0].is_fully_implemented + + solc_select.switch_global_version("0.5.0", always_install=True) + slither = Slither("./tests/function_features/implicit_abstract.sol") + assert not slither.contracts[0].is_fully_implemented + + slither = Slither( + "./tests/function_features/implicit_abstract.sol", solc_force_legacy_json=True + ) + assert not slither.contracts[0].is_fully_implemented diff --git a/tests/test_read_storage.py b/tests/test_read_storage.py index 67a89dae8..7aec6ff40 100644 --- a/tests/test_read_storage.py +++ b/tests/test_read_storage.py @@ -1,7 +1,6 @@ -import re -import os -import sys import json +import os +import re import shutil import subprocess from time import sleep @@ -9,17 +8,12 @@ from typing import Generator import pytest from deepdiff import DeepDiff +from web3 import Web3 +from web3.contract import Contract + from slither import Slither from slither.tools.read_storage import SlitherReadStorage -try: - from web3 import Web3 - from web3.contract import Contract -except ImportError: - print("ERROR: in order to use slither-read-storage, you need to install web3") - print("$ pip3 install web3 --user\n") - sys.exit(-1) - SLITHER_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) STORAGE_TEST_ROOT = os.path.join(SLITHER_ROOT, "tests", "storage-layout") @@ -98,7 +92,7 @@ def deploy_contract(w3, ganache, contract_bin, contract_abi) -> Contract: # pylint: disable=too-many-locals @pytest.mark.usefixtures("web3", "ganache") def test_read_storage(web3, ganache) -> None: - assert web3.isConnected() + assert web3.is_connected() bin_path = os.path.join(STORAGE_TEST_ROOT, "StorageLayout.bin") abi_path = os.path.join(STORAGE_TEST_ROOT, "StorageLayout.abi") bytecode = get_source_file(bin_path) diff --git a/tests/test_ssa_generation.py b/tests/test_ssa_generation.py index c7bc8d5cc..94620285e 100644 --- a/tests/test_ssa_generation.py +++ b/tests/test_ssa_generation.py @@ -28,6 +28,7 @@ from slither.slithir.operations import ( BinaryType, InternalCall, Index, + InitArray, ) from slither.slithir.utils.ssa import is_used_later from slither.slithir.variables import ( @@ -817,20 +818,20 @@ def test_memory_array(): uint val1; uint val2; } - + function test_array() internal { A[] memory a= new A[](4); // Create REF_0 -> a_1[2] accept_array_entry(a[2]); - + // Create REF_1 -> a_1[3] accept_array_entry(a[3]); - + A memory alocal; accept_array_entry(alocal); - + } - + // val_1 = ϕ(val_0, REF_0, REF_1, alocal_1) // val_0 is an unknown external value function accept_array_entry(A memory val) public returns (uint) { @@ -839,9 +840,9 @@ def test_memory_array(): // Create REF_2 -> val_1.val1 return b(val.val1); } - + function b(uint arg) public returns (uint){ - // arg_1 = ϕ(arg_0, zero_1, REF_2) + // arg_1 = ϕ(arg_0, zero_1, REF_2) return arg + 1; } }""" @@ -883,12 +884,12 @@ def test_storage_array(): uint val1; uint val2; } - + // NOTE(hbrodin): a is never written, should only become a_0. Same for astorage (astorage_0). Phi-nodes at entry - // should only add new versions of a state variable if it is actually written. + // should only add new versions of a state variable if it is actually written. A[] a; A astorage; - + function test_array() internal { accept_array_entry(a[2]); accept_array_entry(a[3]); @@ -898,7 +899,7 @@ def test_storage_array(): function accept_array_entry(A storage val) internal returns (uint) { // val is either a[2], a[3] or astorage_0. Ideally this could be identified. uint five = 5; - + // NOTE(hbrodin): If the following line is enabled, there would ideally be a phi-node representing writes // to either a or astorage. //val.val2 = 4; @@ -1080,3 +1081,20 @@ def test_issue_473(): # return is for second phi assert len(return_value.values) == 1 assert second_phi.lvalue in return_value.values + + +def test_issue_1748(): + source = """ + contract Contract { + uint[] arr; + function foo(uint i) public { + arr = [1]; + } + } + """ + with slither_from_source(source) as slither: + c = slither.get_contract_from_name("Contract")[0] + f = c.functions[0] + operations = f.slithir_operations + assign_op = operations[0] + assert isinstance(assign_op, InitArray)