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 2168dd18f..22eed98fa 100644 --- a/README.md +++ b/README.md @@ -246,7 +246,7 @@ 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 | Code +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) 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..0d26167b3 100644 --- a/setup.py +++ b/setup.py @@ -27,10 +27,10 @@ setup( "pytest-xdist", "deepdiff", "numpy", - "solc-select>=v1.0.0b1", "openai", "pdoc", - ] + "web3>=6.0.0", + ], }, license="AGPL-3.0", long_description=long_description, diff --git a/slither/__main__.py b/slither/__main__.py index 528a93e8f..5d0dda9e0 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -76,9 +76,6 @@ def process_single( ast = "--ast-compact-json" if args.legacy_ast: ast = "--ast-json" - if args.checklist: - args.show_ignored_findings = True - slither = Slither(target, ast_format=ast, **vars(args)) return _process(slither, detector_classes, printer_classes) @@ -517,7 +514,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"], @@ -760,7 +757,7 @@ def main_impl( # If we are outputting JSON, capture all standard output. If we are outputting to stdout, we block typical stdout # output. - if outputting_json or output_to_sarif: + if outputting_json or outputting_sarif: StandardOutputCapture.enable(outputting_json_stdout or outputting_sarif_stdout) printer_classes = choose_printers(args, all_printer_classes) @@ -871,7 +868,9 @@ def main_impl( # Output our results to markdown if we wish to compile a checklist. if args.checklist: - output_results_to_markdown(results_detectors, args.checklist_limit) + output_results_to_markdown( + results_detectors, args.checklist_limit, args.show_ignored_findings + ) # Don't print the number of result for printers if number_contracts == 0: diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 2d2d10b04..95b05aa6b 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -110,6 +110,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 @@ -165,6 +167,31 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods def is_library(self, is_library: bool): 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 + # endregion ################################################################################### ################################################################################### diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index a0fcf354a..e58b04c21 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/shadowing/local.py b/slither/detectors/shadowing/local.py index ad65b62d9..07abe5248 100644 --- a/slither/detectors/shadowing/local.py +++ b/slither/detectors/shadowing/local.py @@ -57,6 +57,7 @@ contract Bug { OVERSHADOWED_MODIFIER = "modifier" OVERSHADOWED_STATE_VARIABLE = "state variable" OVERSHADOWED_EVENT = "event" + OVERSHADOWED_RETURN_VARIABLE = "return variable" # pylint: disable=too-many-branches def detect_shadowing_definitions( @@ -111,6 +112,15 @@ contract Bug { overshadowed.append( (self.OVERSHADOWED_STATE_VARIABLE, scope_state_variable) ) + # Check named return variables + for named_return in function.returns: + # Shadowed local delcarations in the same function will have "_scope_" in their name. + # See `FunctionSolc._add_local_variable` + if ( + "_scope_" in variable.name + and variable.name.split("_scope_")[0] == named_return.name + ): + overshadowed.append((self.OVERSHADOWED_RETURN_VARIABLE, named_return)) # If we have found any overshadowed objects, we'll want to add it to our result list. if overshadowed: diff --git a/slither/detectors/variables/predeclaration_usage_local.py b/slither/detectors/variables/predeclaration_usage_local.py index 2ba539a91..177035ef4 100644 --- a/slither/detectors/variables/predeclaration_usage_local.py +++ b/slither/detectors/variables/predeclaration_usage_local.py @@ -7,7 +7,11 @@ from slither.core.cfg.node import Node from slither.core.declarations import Function from slither.core.declarations.contract import Contract from slither.core.variables.local_variable import LocalVariable -from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + ALL_SOLC_VERSIONS_04, +) from slither.utils.output import Output @@ -54,6 +58,8 @@ Additionally, the for-loop uses the variable `max`, which is declared in a previ WIKI_RECOMMENDATION = "Move all variable declarations prior to any usage of the variable, and ensure that reaching a variable declaration does not depend on some conditional if it is used unconditionally." + VULNERABLE_SOLC_VERSIONS = ALL_SOLC_VERSIONS_04 + def detect_predeclared_local_usage( self, node: Node, diff --git a/slither/printers/guidance/echidna.py b/slither/printers/guidance/echidna.py index 0df47e965..b160dd0e6 100644 --- a/slither/printers/guidance/echidna.py +++ b/slither/printers/guidance/echidna.py @@ -12,6 +12,7 @@ from slither.core.declarations.solidity_variables import ( ) from slither.core.expressions import NewContract from slither.core.slither_core import SlitherCore +from slither.core.solidity_types import TypeAlias from slither.core.variables.state_variable import StateVariable from slither.core.variables.variable import Variable from slither.printers.abstract_printer import AbstractPrinter @@ -30,8 +31,8 @@ from slither.slithir.operations import ( ) from slither.slithir.operations.binary import Binary from slither.slithir.variables import Constant -from slither.visitors.expression.constants_folding import ConstantFolding from slither.utils.output import Output +from slither.visitors.expression.constants_folding import ConstantFolding def _get_name(f: Union[Function, Variable]) -> str: @@ -184,7 +185,11 @@ def _extract_constants_from_irs( # pylint: disable=too-many-branches,too-many-n all_cst_used.append(ConstantValue(str(cst.value), str(type_))) if isinstance(ir, TypeConversion): if isinstance(ir.variable, Constant): - all_cst_used.append(ConstantValue(str(ir.variable.value), str(ir.type))) + if isinstance(ir.type, TypeAlias): + value_type = ir.type.type + else: + value_type = ir.type + all_cst_used.append(ConstantValue(str(ir.variable.value), str(value_type))) continue if ( isinstance(ir, Member) diff --git a/slither/printers/summary/declaration.py b/slither/printers/summary/declaration.py index 5888a1f00..529aba5f0 100644 --- a/slither/printers/summary/declaration.py +++ b/slither/printers/summary/declaration.py @@ -20,37 +20,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/convert.py b/slither/slithir/convert.py index 87a6b075b..9657e2317 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -525,9 +525,7 @@ def _convert_type_contract(ir: Member) -> Assignment: raise SlithIRError(f"type({contract.name}).{ir.variable_right} is unknown") -def propagate_types( - ir: slither.slithir.operations.operation.Operation, node: "Node" -): # pylint: disable=too-many-locals +def propagate_types(ir: Operation, node: "Node"): # pylint: disable=too-many-locals # propagate the type node_function = node.function using_for = ( @@ -577,9 +575,9 @@ def propagate_types( if (isinstance(t, ElementaryType) and t.name == "address") or ( isinstance(t, TypeAlias) and t.underlying_type.name == "address" ): - # Cannot be a top level function with this. - assert isinstance(node_function, FunctionContract) if ir.destination.name == "this": + # Cannot be a top level function with this. + assert isinstance(node_function, FunctionContract) # the target contract is the contract itself return convert_type_of_high_and_internal_level_call( ir, node_function.contract diff --git a/slither/slithir/operations/high_level_call.py b/slither/slithir/operations/high_level_call.py index 93fb73bd4..a12917519 100644 --- a/slither/slithir/operations/high_level_call.py +++ b/slither/slithir/operations/high_level_call.py @@ -76,7 +76,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) -> SourceMapping: diff --git a/slither/slithir/operations/new_contract.py b/slither/slithir/operations/new_contract.py index 879d12df6..08ddbd960 100644 --- a/slither/slithir/operations/new_contract.py +++ b/slither/slithir/operations/new_contract.py @@ -52,7 +52,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 475c3fab2..e63dbe68f 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -780,12 +780,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/solc_parsing/declarations/using_for_top_level.py b/slither/solc_parsing/declarations/using_for_top_level.py index b16fadc40..707ad83ac 100644 --- a/slither/solc_parsing/declarations/using_for_top_level.py +++ b/slither/solc_parsing/declarations/using_for_top_level.py @@ -49,7 +49,7 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- type_name = parse_type(self._type_name, self) self._using_for.using_for[type_name] = [] - if self._library_name is not None: + if self._library_name: library_name = parse_type(self._library_name, self) self._using_for.using_for[type_name].append(library_name) self._propagate_global(type_name) @@ -90,8 +90,13 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- def _analyze_top_level_function( self, function_name: str, type_name: Union[TypeAliasTopLevel, UserDefinedType] ) -> None: - for tl_function in self.compilation_unit.functions_top_level: - if tl_function.name == function_name: + for tl_function in self._using_for.file_scope.functions: + # The library function is bound to the first parameter's type + if ( + tl_function.name == function_name + and tl_function.parameters + and type_name == tl_function.parameters[0].type + ): self._using_for.using_for[type_name].append(tl_function) self._propagate_global(type_name) break @@ -108,7 +113,12 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- break if c.name == library_name: for cf in c.functions: - if cf.name == function_name: + # The library function is bound to the first parameter's type + if ( + cf.name == function_name + and cf.parameters + and type_name == cf.parameters[0].type + ): self._using_for.using_for[type_name].append(cf) self._propagate_global(type_name) found = True 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/tools/read_storage/utils/utils.py b/slither/tools/read_storage/utils/utils.py index 3e51e2181..befd3d0e7 100644 --- a/slither/tools/read_storage/utils/utils.py +++ b/slither/tools/read_storage/utils/utils.py @@ -1,7 +1,7 @@ from typing import Union from eth_typing.evm import ChecksumAddress -from eth_utils import to_int, to_text, to_checksum_address +from eth_utils import to_checksum_address, to_int, to_text def get_offset_value(hex_bytes: bytes, offset: int, size: int) -> bytes: diff --git a/slither/utils/command_line.py b/slither/utils/command_line.py index 174c2a4b6..911609bf6 100644 --- a/slither/utils/command_line.py +++ b/slither/utils/command_line.py @@ -169,7 +169,9 @@ def convert_result_to_markdown(txt: str) -> str: return "".join(ret) -def output_results_to_markdown(all_results: List[Dict], checklistlimit: str) -> None: +def output_results_to_markdown( + all_results: List[Dict], checklistlimit: str, show_ignored_findings: bool +) -> None: checks = defaultdict(list) info: Dict = defaultdict(dict) for results_ in all_results: @@ -179,6 +181,11 @@ def output_results_to_markdown(all_results: List[Dict], checklistlimit: str) -> "confidence": results_["confidence"], } + if not show_ignored_findings: + print( + "**THIS CHECKLIST IS NOT COMPLETE**. Use `--show-ignored-findings` to show all the results." + ) + print("Summary") for check_ in checks: print( 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/utils/type.py b/slither/utils/type.py index 5c7fcee34..1674999aa 100644 --- a/slither/utils/type.py +++ b/slither/utils/type.py @@ -1,7 +1,13 @@ import math from typing import List, Union, Set -from slither.core.solidity_types import ArrayType, MappingType, ElementaryType, UserDefinedType +from slither.core.solidity_types import ( + ArrayType, + MappingType, + ElementaryType, + UserDefinedType, + TypeAlias, +) from slither.core.solidity_types.type import Type from slither.core.variables.variable import Variable @@ -89,6 +95,9 @@ def convert_type_for_solidity_signature(t: Type, seen: Set[Type]) -> Union[Type, ] return types + if isinstance(t, TypeAlias): + return t.type + return t 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/shadowing-local/0.4.25/shadowing_local_variable.sol b/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol index b67833f05..acd93bb46 100644 --- a/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol +++ b/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol @@ -24,3 +24,13 @@ contract FurtherExtendedContract is ExtendedContract { function shadowingParent(uint x) public pure { int y; uint z; uint w; uint v; } } + +contract LocalReturnVariables { + uint state; + function shadowedState() external view returns(uint state) { + return state; + } + function good() external view returns(uint val1) { + return val1; + } +} \ No newline at end of file diff --git a/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol.0.4.25.LocalShadowing.json b/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol.0.4.25.LocalShadowing.json index ce0f062a6..e12b5f26d 100644 --- a/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol.0.4.25.LocalShadowing.json +++ b/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol.0.4.25.LocalShadowing.json @@ -204,6 +204,129 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 533, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30 + ], + "starting_column": 52, + "ending_column": 62 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedState", + "source_mapping": { + "start": 486, + "length": 88, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30, + 31, + 32 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 434, + "length": 225, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedState()" + } + } + } + }, + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 470, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 29 + ], + "starting_column": 5, + "ending_column": 15 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 434, + "length": 225, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37 + ], + "starting_column": 1, + "ending_column": 0 + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedState().state (tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#30) shadows:\n\t- LocalReturnVariables.state (tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#29) (state variable)\n", + "markdown": "[LocalReturnVariables.shadowedState().state](tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#L30) shadows:\n\t- [LocalReturnVariables.state](tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#L29) (state variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#L30", + "id": "1b0030affabcff703e57e4f388b86dbda0f412e51ba8d15248bcae9e4748a012", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { diff --git a/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol b/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol index ec17297e6..385f6194c 100644 --- a/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol +++ b/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol @@ -24,3 +24,17 @@ contract FurtherExtendedContract is ExtendedContract { function shadowingParent(uint x) public pure { int y; uint z; uint w; uint v; } } + +contract LocalReturnVariables { + uint state; + function shadowedState() external view returns(uint state) { + return state; + } + function shadowedReturn() external view returns(uint local) { + uint local = 1; + return local; + } + function good() external view returns(uint val1) { + return val1; + } +} \ No newline at end of file diff --git a/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol.0.5.16.LocalShadowing.json b/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol.0.5.16.LocalShadowing.json index 1385b0e98..d4d54ad9d 100644 --- a/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol.0.5.16.LocalShadowing.json +++ b/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol.0.5.16.LocalShadowing.json @@ -204,6 +204,137 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 536, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30 + ], + "starting_column": 52, + "ending_column": 62 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedState", + "source_mapping": { + "start": 489, + "length": 88, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30, + 31, + 32 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 437, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedState()" + } + } + } + }, + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 473, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 29 + ], + "starting_column": 5, + "ending_column": 15 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 437, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedState().state (tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#30) shadows:\n\t- LocalReturnVariables.state (tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#29) (state variable)\n", + "markdown": "[LocalReturnVariables.shadowedState().state](tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L30) shadows:\n\t- [LocalReturnVariables.state](tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L29) (state variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L30", + "id": "1b0030affabcff703e57e4f388b86dbda0f412e51ba8d15248bcae9e4748a012", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { @@ -567,6 +698,161 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "local_scope_0", + "source_mapping": { + "start": 653, + "length": 14, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 34 + ], + "starting_column": 9, + "ending_column": 23 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 583, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 437, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedReturn()" + } + } + } + }, + { + "type": "variable", + "name": "local", + "source_mapping": { + "start": 631, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33 + ], + "starting_column": 53, + "ending_column": 63 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 583, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 437, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedReturn()" + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedReturn().local_scope_0 (tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#34) shadows:\n\t- LocalReturnVariables.shadowedReturn().local (tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#33) (return variable)\n", + "markdown": "[LocalReturnVariables.shadowedReturn().local_scope_0](tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L34) shadows:\n\t- [LocalReturnVariables.shadowedReturn().local](tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L33) (return variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L34", + "id": "cd63bdf3f6420e4e109d20ec44b52fcbcbde1c5b6a0701fc6994b35960ab1e85", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { diff --git a/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol b/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol index 450dc851b..c44fe934e 100644 --- a/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol +++ b/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol @@ -24,3 +24,17 @@ contract FurtherExtendedContract is ExtendedContract { function shadowingParent(uint __x) public pure { int y; uint z; uint w; uint v; } } + +contract LocalReturnVariables { + uint state; + function shadowedState() external view returns(uint state) { + return state; + } + function shadowedReturn() external view returns(uint local) { + uint local = 1; + return local; + } + function good() external view returns(uint val1) { + return val1; + } +} \ No newline at end of file diff --git a/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol.0.6.11.LocalShadowing.json b/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol.0.6.11.LocalShadowing.json index 0c495f07c..2dc747505 100644 --- a/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol.0.6.11.LocalShadowing.json +++ b/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol.0.6.11.LocalShadowing.json @@ -1,5 +1,136 @@ [ [ + { + "elements": [ + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 541, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30 + ], + "starting_column": 52, + "ending_column": 62 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedState", + "source_mapping": { + "start": 494, + "length": 88, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30, + 31, + 32 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedState()" + } + } + } + }, + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 478, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 29 + ], + "starting_column": 5, + "ending_column": 15 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedState().state (tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#30) shadows:\n\t- LocalReturnVariables.state (tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#29) (state variable)\n", + "markdown": "[LocalReturnVariables.shadowedState().state](tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L30) shadows:\n\t- [LocalReturnVariables.state](tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L29) (state variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L30", + "id": "1b0030affabcff703e57e4f388b86dbda0f412e51ba8d15248bcae9e4748a012", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { @@ -486,6 +617,161 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "local_scope_0", + "source_mapping": { + "start": 658, + "length": 14, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 34 + ], + "starting_column": 9, + "ending_column": 23 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 588, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedReturn()" + } + } + } + }, + { + "type": "variable", + "name": "local", + "source_mapping": { + "start": 636, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33 + ], + "starting_column": 53, + "ending_column": 63 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 588, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedReturn()" + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedReturn().local_scope_0 (tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#34) shadows:\n\t- LocalReturnVariables.shadowedReturn().local (tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#33) (return variable)\n", + "markdown": "[LocalReturnVariables.shadowedReturn().local_scope_0](tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L34) shadows:\n\t- [LocalReturnVariables.shadowedReturn().local](tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L33) (return variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L34", + "id": "cd63bdf3f6420e4e109d20ec44b52fcbcbde1c5b6a0701fc6994b35960ab1e85", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { diff --git a/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol b/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol index 450dc851b..e43477e54 100644 --- a/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol +++ b/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol @@ -24,3 +24,17 @@ contract FurtherExtendedContract is ExtendedContract { function shadowingParent(uint __x) public pure { int y; uint z; uint w; uint v; } } + +contract LocalReturnVariables { + uint state; + function shadowedState() external view returns(uint state) { + return state; + } + function shadowedReturn() external view returns(uint local) { + uint local = 1; + return local; + } + function good() external view returns(uint val1) { + return val1; + } +} diff --git a/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol.0.7.6.LocalShadowing.json b/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol.0.7.6.LocalShadowing.json index 9ea9783e1..e013dc0eb 100644 --- a/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol.0.7.6.LocalShadowing.json +++ b/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol.0.7.6.LocalShadowing.json @@ -1,5 +1,134 @@ [ [ + { + "elements": [ + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 541, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30 + ], + "starting_column": 52, + "ending_column": 62 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedState", + "source_mapping": { + "start": 494, + "length": 88, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30, + 31, + 32 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "shadowedState()" + } + } + } + }, + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 478, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 29 + ], + "starting_column": 5, + "ending_column": 15 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40 + ], + "starting_column": 1, + "ending_column": 2 + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedState().state (tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#30) shadows:\n\t- LocalReturnVariables.state (tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#29) (state variable)\n", + "markdown": "[LocalReturnVariables.shadowedState().state](tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L30) shadows:\n\t- [LocalReturnVariables.state](tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L29) (state variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L30", + "id": "1b0030affabcff703e57e4f388b86dbda0f412e51ba8d15248bcae9e4748a012", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { @@ -486,6 +615,159 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "local_scope_0", + "source_mapping": { + "start": 658, + "length": 14, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 34 + ], + "starting_column": 9, + "ending_column": 23 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 588, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "shadowedReturn()" + } + } + } + }, + { + "type": "variable", + "name": "local", + "source_mapping": { + "start": 636, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33 + ], + "starting_column": 53, + "ending_column": 63 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 588, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "shadowedReturn()" + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedReturn().local_scope_0 (tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#34) shadows:\n\t- LocalReturnVariables.shadowedReturn().local (tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#33) (return variable)\n", + "markdown": "[LocalReturnVariables.shadowedReturn().local_scope_0](tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L34) shadows:\n\t- [LocalReturnVariables.shadowedReturn().local](tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L33) (return variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L34", + "id": "cd63bdf3f6420e4e109d20ec44b52fcbcbde1c5b6a0701fc6994b35960ab1e85", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { 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/test_features.py b/tests/test_features.py index d29a5eb6a..68a21a884 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -1,4 +1,5 @@ import inspect +from pathlib import Path from crytic_compile import CryticCompile from crytic_compile.platform.solc_standard_json import SolcStandardJson @@ -8,7 +9,7 @@ from slither import Slither from slither.core.variables.state_variable import StateVariable from slither.detectors import all_detectors from slither.detectors.abstract_detector import AbstractDetector -from slither.slithir.operations import LibraryCall, InternalCall +from slither.slithir.operations import InternalCall, LibraryCall from slither.utils.arithmetic import unchecked_arithemtic_usage @@ -31,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") @@ -43,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) @@ -74,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") @@ -160,3 +192,13 @@ def test_arithmetic_usage() -> None: assert { f.source_mapping.content_hash for f in unchecked_arithemtic_usage(slither.contracts[0]) } == {"2b4bc73cf59d486dd9043e840b5028b679354dd9", "e4ecd4d0fda7e762d29aceb8425f2c5d4d0bf962"} + + +def test_using_for_global_collision() -> None: + solc_select.switch_global_version("0.8.18", always_install=True) + standard_json = SolcStandardJson() + for source_file in Path("./tests/using-for-global-collision").rglob("*.sol"): + standard_json.add_source_file(Path(source_file).as_posix()) + compilation = CryticCompile(standard_json) + sl = Slither(compilation) + _run_all_detectors(sl) 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/using-for-global-collision/src/MyTypeA.sol b/tests/using-for-global-collision/src/MyTypeA.sol new file mode 100644 index 000000000..dbb00faf2 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeA.sol @@ -0,0 +1,2 @@ +import "./MyTypeA/Type.sol"; +import "./MyTypeA/Math.sol"; \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeA/Casting.sol b/tests/using-for-global-collision/src/MyTypeA/Casting.sol new file mode 100644 index 000000000..c166436fe --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeA/Casting.sol @@ -0,0 +1,4 @@ +import "./Type.sol"; +function unwrap(MyTypeA a) pure returns (int256) { + return MyTypeA.unwrap(a); +} \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeA/Math.sol b/tests/using-for-global-collision/src/MyTypeA/Math.sol new file mode 100644 index 000000000..21f7c7925 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeA/Math.sol @@ -0,0 +1,5 @@ +import "./Type.sol"; + +function mul(MyTypeA a, MyTypeA b) pure returns (MyTypeA) { + return MyTypeA.wrap(MyTypeA.unwrap(a) * MyTypeA.unwrap(b)); +} diff --git a/tests/using-for-global-collision/src/MyTypeA/Type.sol b/tests/using-for-global-collision/src/MyTypeA/Type.sol new file mode 100644 index 000000000..0973c7869 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeA/Type.sol @@ -0,0 +1,6 @@ +import "./Casting.sol" as C; +import "./Math.sol" as M; + +type MyTypeA is int256; + +using {M.mul, C.unwrap} for MyTypeA global; \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeB.sol b/tests/using-for-global-collision/src/MyTypeB.sol new file mode 100644 index 000000000..3ddde2ac6 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeB.sol @@ -0,0 +1,2 @@ +import "./MyTypeB/Type.sol"; +import "./MyTypeB/Math.sol"; \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeB/Casting.sol b/tests/using-for-global-collision/src/MyTypeB/Casting.sol new file mode 100644 index 000000000..c400a9112 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeB/Casting.sol @@ -0,0 +1,4 @@ +import "./Type.sol"; +function unwrap(MyTypeB a) pure returns (uint256) { + return MyTypeB.unwrap(a); +} \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeB/Math.sol b/tests/using-for-global-collision/src/MyTypeB/Math.sol new file mode 100644 index 000000000..24ee1a582 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeB/Math.sol @@ -0,0 +1,6 @@ +import "./Type.sol"; + +function mul(MyTypeB a, MyTypeB b) pure returns (MyTypeB) { + return MyTypeB.wrap(MyTypeB.unwrap(a) * MyTypeB.unwrap(b)); +} + diff --git a/tests/using-for-global-collision/src/MyTypeB/Type.sol b/tests/using-for-global-collision/src/MyTypeB/Type.sol new file mode 100644 index 000000000..a66b65f5d --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeB/Type.sol @@ -0,0 +1,6 @@ +import "./Casting.sol" as C; +import "./Math.sol" as M; + +type MyTypeB is uint256; + +using {M.mul, C.unwrap} for MyTypeB global; \ No newline at end of file diff --git a/tests/using-for-global-collision/src/Test.sol b/tests/using-for-global-collision/src/Test.sol new file mode 100644 index 000000000..013570048 --- /dev/null +++ b/tests/using-for-global-collision/src/Test.sol @@ -0,0 +1,7 @@ +import "./MyTypeB.sol"; + +contract UsingForGlobalTopLevelCollision { + function mulAndUnwrap(MyTypeB x, MyTypeB y) external pure returns (uint256 z) { + z = x.mul(y).unwrap(); + } +} \ No newline at end of file