diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml index b805c4d9d..c820bea8f 100644 --- a/.github/workflows/black.yml +++ b/.github/workflows/black.yml @@ -9,13 +9,15 @@ defaults: on: pull_request: branches: [master, dev] + paths: + - "**/*.py" schedule: # run CI every day even if no PRs/merges occur - cron: '0 12 * * *' jobs: build: - name: Lint Code Base + name: Black runs-on: ubuntu-latest steps: diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml index 5d9ff9017..a32c9cda6 100644 --- a/.github/workflows/linter.yml +++ b/.github/workflows/linter.yml @@ -9,13 +9,16 @@ defaults: on: pull_request: branches: [master, dev] + paths: + - "**/*.py" + schedule: # run CI every day even if no PRs/merges occur - cron: '0 12 * * *' jobs: build: - name: Lint Code Base + name: Pylint runs-on: ubuntu-latest steps: diff --git a/slither/detectors/statements/divide_before_multiply.py b/slither/detectors/statements/divide_before_multiply.py index 601004d6d..f51115c66 100644 --- a/slither/detectors/statements/divide_before_multiply.py +++ b/slither/detectors/statements/divide_before_multiply.py @@ -78,8 +78,7 @@ def _explore(to_explore: Set[Node], f_results: List[Any], divisions: DefaultDict node_results = [] for ir in node.irs: - # check for Constant, has its not hashable (TODO: make Constant hashable) - if isinstance(ir, Assignment) and not isinstance(ir.rvalue, Constant): + if isinstance(ir, Assignment): if ir.rvalue in divisions: # Avoid dupplicate. We dont use set so we keep the order of the nodes if node not in divisions[ir.rvalue]: diff --git a/slither/detectors/variables/unchanged_state_variables.py b/slither/detectors/variables/unchanged_state_variables.py index 7da6105da..f12cc5784 100644 --- a/slither/detectors/variables/unchanged_state_variables.py +++ b/slither/detectors/variables/unchanged_state_variables.py @@ -118,8 +118,9 @@ class UnchangedStateVariables: self.constant_candidates.append(v) elif ( - v in constructor_variables_written or v in variables_initialized - ) and version.parse(self.compilation_unit.solc_version) >= version.parse( - "0.6.5" + not v.type.is_dynamic + and version.parse(self.compilation_unit.solc_version) + >= version.parse("0.6.5") + and (v in constructor_variables_written or v in variables_initialized) ): self.immutable_candidates.append(v) diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index 7e1f17159..ea433a921 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -36,6 +36,7 @@ from slither.core.expressions.binary_operation import ( from slither.core.solidity_types import ( ArrayType, ElementaryType, + UserDefinedType, ) from slither.solc_parsing.declarations.caller_context import CallerContextExpression from slither.solc_parsing.exceptions import ParsingError, VariableNotFound @@ -122,7 +123,6 @@ def parse_call( if type_conversion: type_call = parse_type(UnknownType(type_return), caller_context) - if caller_context.is_compact_ast: assert len(expression["arguments"]) == 1 expression_to_parse = expression["arguments"][0] @@ -143,6 +143,8 @@ def parse_call( expression = parse_expression(expression_to_parse, caller_context) t = TypeConversion(expression, type_call) t.set_offset(src, caller_context.compilation_unit) + if isinstance(type_call, UserDefinedType): + type_call.type.references.append(t.source_mapping) return t call_gas = None diff --git a/slither/solc_parsing/solidity_types/type_parsing.py b/slither/solc_parsing/solidity_types/type_parsing.py index 7a52ddb0a..e12290722 100644 --- a/slither/solc_parsing/solidity_types/type_parsing.py +++ b/slither/solc_parsing/solidity_types/type_parsing.py @@ -6,7 +6,7 @@ from slither.core.declarations.custom_error_contract import CustomErrorContract from slither.core.declarations.custom_error_top_level import CustomErrorTopLevel from slither.core.declarations.function_contract import FunctionContract from slither.core.expressions.literal import Literal -from slither.core.solidity_types import TypeAlias +from slither.core.solidity_types import TypeAlias, TypeAliasTopLevel, TypeAliasContract from slither.core.solidity_types.array_type import ArrayType from slither.core.solidity_types.elementary_type import ( ElementaryType, @@ -199,6 +199,9 @@ def _add_type_references(type_found: Type, src: str, sl: "SlitherCompilationUnit if isinstance(type_found, UserDefinedType): type_found.type.add_reference_from_raw_source(src, sl) + elif isinstance(type_found, (TypeAliasTopLevel, TypeAliasContract)): + type_found.type.add_reference_from_raw_source(src, sl) + type_found.add_reference_from_raw_source(src, sl) # TODO: since the add of FileScope, we can probably refactor this function and makes it a lot simpler @@ -363,6 +366,7 @@ def parse_type( if name in renaming: name = renaming[name] if name in user_defined_types: + _add_type_references(user_defined_types[name], t["src"], sl) return user_defined_types[name] type_found = _find_from_type_name( name, @@ -383,6 +387,7 @@ def parse_type( if name in renaming: name = renaming[name] if name in user_defined_types: + _add_type_references(user_defined_types[name], t["src"], sl) return user_defined_types[name] type_found = _find_from_type_name( name, diff --git a/slither/solc_parsing/yul/parse_yul.py b/slither/solc_parsing/yul/parse_yul.py index f5bb77dcc..35d5cdd9d 100644 --- a/slither/solc_parsing/yul/parse_yul.py +++ b/slither/solc_parsing/yul/parse_yul.py @@ -792,8 +792,9 @@ def parse_yul_identifier(root: YulScope, _node: YulNode, ast: Dict) -> Optional[ return Identifier(local_variable) if isinstance(parent_func, FunctionContract): - assert parent_func.contract - state_variable = parent_func.contract.get_state_variable_from_name(name) + # Variables must be looked from the contract declarer + assert parent_func.contract_declarer + state_variable = parent_func.contract_declarer.get_state_variable_from_name(name) if state_variable: return Identifier(state_variable) diff --git a/tests/ast-parsing/compile/yul-state-constant-access.sol-0.8.16-compact.zip b/tests/ast-parsing/compile/yul-state-constant-access.sol-0.8.16-compact.zip new file mode 100644 index 000000000..121d6c5ec Binary files /dev/null and b/tests/ast-parsing/compile/yul-state-constant-access.sol-0.8.16-compact.zip differ diff --git a/tests/ast-parsing/expected/yul-state-constant-access.sol-0.8.16-compact.json b/tests/ast-parsing/expected/yul-state-constant-access.sol-0.8.16-compact.json new file mode 100644 index 000000000..6c6bc9377 --- /dev/null +++ b/tests/ast-parsing/expected/yul-state-constant-access.sol-0.8.16-compact.json @@ -0,0 +1,12 @@ +{ + "A": { + "f()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "f2()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: INLINE ASM 2\n\"];\n2->3;\n3[label=\"Node Type: EXPRESSION 3\n\"];\n}\n", + "f3()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n" + }, + "B": { + "f()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "f2()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: INLINE ASM 2\n\"];\n2->3;\n3[label=\"Node Type: EXPRESSION 3\n\"];\n}\n", + "f3()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/ast-parsing/yul-state-constant-access.sol b/tests/ast-parsing/yul-state-constant-access.sol new file mode 100644 index 000000000..543d79744 --- /dev/null +++ b/tests/ast-parsing/yul-state-constant-access.sol @@ -0,0 +1,25 @@ +contract A{ + uint private constant a = 10; + + function f() public returns(uint){ + return a; + } + + function f2() public returns(uint){ + uint ret; + assembly{ + ret := a + } + } + + function f3() public returns(uint){ + uint ret; + unchecked{ + ret = a; + } + } +} + +contract B is A{ + +} \ No newline at end of file diff --git a/tests/detectors/immutable-states/0.4.25/immut_state_variables.sol b/tests/detectors/immutable-states/0.4.25/immut_state_variables.sol index 45f8cc87a..db7f15a44 100644 --- a/tests/detectors/immutable-states/0.4.25/immut_state_variables.sol +++ b/tests/detectors/immutable-states/0.4.25/immut_state_variables.sol @@ -45,9 +45,11 @@ contract MyConc{ uint not_constant_2 = getNumber(); uint not_constant_3 = 10 + block.number; uint not_constant_5; + string cannote_be_immutable; - constructor(uint b) public { + constructor(uint b, string memory c) public { not_constant_5 = b; + cannote_be_immutable = c; } function getNumber() public returns(uint){ diff --git a/tests/detectors/immutable-states/0.5.16/immut_state_variables.sol b/tests/detectors/immutable-states/0.5.16/immut_state_variables.sol index 8fd1ca808..138f007b7 100644 --- a/tests/detectors/immutable-states/0.5.16/immut_state_variables.sol +++ b/tests/detectors/immutable-states/0.5.16/immut_state_variables.sol @@ -46,9 +46,11 @@ contract MyConc{ uint not_constant_2 = getNumber(); uint not_constant_3 = 10 + block.number; uint not_constant_5; + string cannote_be_immutable; - constructor(uint b) public { + constructor(uint b, string memory c) public { not_constant_5 = b; + cannote_be_immutable = c; } function getNumber() public returns(uint){ diff --git a/tests/detectors/immutable-states/0.6.11/immut_state_variables.sol b/tests/detectors/immutable-states/0.6.11/immut_state_variables.sol index 17548f46f..104596290 100644 --- a/tests/detectors/immutable-states/0.6.11/immut_state_variables.sol +++ b/tests/detectors/immutable-states/0.6.11/immut_state_variables.sol @@ -67,9 +67,11 @@ contract Good { uint immutable should_be_immutable_3 = 10 + block.number; B immutable should_be_immutable_4 = new B(); uint immutable should_be_immutable_5; + string cannote_be_immutable; - constructor(uint b) public { + constructor(uint b, string memory c) public { should_be_immutable_5 = b; + cannote_be_immutable = c; } function getNumber() public returns(uint){ diff --git a/tests/detectors/immutable-states/0.7.6/immut_state_variables.sol b/tests/detectors/immutable-states/0.7.6/immut_state_variables.sol index 297dd6294..8b8e6ae95 100644 --- a/tests/detectors/immutable-states/0.7.6/immut_state_variables.sol +++ b/tests/detectors/immutable-states/0.7.6/immut_state_variables.sol @@ -66,9 +66,11 @@ contract Good { uint immutable should_be_immutable_3 = 10 + block.number; B immutable should_be_immutable_4 = new B(); uint immutable should_be_immutable_5; + string cannote_be_immutable; - constructor(uint b) { + constructor(uint b, string memory c) { should_be_immutable_5 = b; + cannote_be_immutable = c; } function getNumber() public returns(uint){ diff --git a/tests/detectors/immutable-states/0.8.0/immut_state_variables.sol b/tests/detectors/immutable-states/0.8.0/immut_state_variables.sol index f405a1587..943b06ec3 100644 --- a/tests/detectors/immutable-states/0.8.0/immut_state_variables.sol +++ b/tests/detectors/immutable-states/0.8.0/immut_state_variables.sol @@ -44,9 +44,11 @@ contract Bad { uint should_be_immutable_2 = getNumber(); uint should_be_immutable_3 = 10 + block.number; uint should_be_immutable_5; + string cannote_be_immutable; - constructor(uint b) { + constructor(uint b, string memory c) { should_be_immutable_5 = b; + cannote_be_immutable = c; } function getNumber() public returns(uint){ diff --git a/tests/detectors/immutable-states/0.8.0/immut_state_variables.sol.0.8.0.CouldBeImmutable.json b/tests/detectors/immutable-states/0.8.0/immut_state_variables.sol.0.8.0.CouldBeImmutable.json index afa2e3bb2..58d26f9bc 100644 --- a/tests/detectors/immutable-states/0.8.0/immut_state_variables.sol.0.8.0.CouldBeImmutable.json +++ b/tests/detectors/immutable-states/0.8.0/immut_state_variables.sol.0.8.0.CouldBeImmutable.json @@ -24,7 +24,7 @@ "name": "Bad", "source_mapping": { "start": 718, - "length": 493, + "length": 577, "filename_relative": "tests/detectors/immutable-states/0.8.0/immut_state_variables.sol", "filename_absolute": "/GENERIC_PATH", "filename_short": "tests/detectors/immutable-states/0.8.0/immut_state_variables.sol", @@ -49,7 +49,9 @@ 53, 54, 55, - 56 + 56, + 57, + 58 ], "starting_column": 1, "ending_column": 2 @@ -90,7 +92,7 @@ "name": "Bad", "source_mapping": { "start": 718, - "length": 493, + "length": 577, "filename_relative": "tests/detectors/immutable-states/0.8.0/immut_state_variables.sol", "filename_absolute": "/GENERIC_PATH", "filename_short": "tests/detectors/immutable-states/0.8.0/immut_state_variables.sol", @@ -115,7 +117,9 @@ 53, 54, 55, - 56 + 56, + 57, + 58 ], "starting_column": 1, "ending_column": 2 @@ -156,7 +160,7 @@ "name": "Bad", "source_mapping": { "start": 718, - "length": 493, + "length": 577, "filename_relative": "tests/detectors/immutable-states/0.8.0/immut_state_variables.sol", "filename_absolute": "/GENERIC_PATH", "filename_short": "tests/detectors/immutable-states/0.8.0/immut_state_variables.sol", @@ -181,7 +185,9 @@ 53, 54, 55, - 56 + 56, + 57, + 58 ], "starting_column": 1, "ending_column": 2 @@ -222,7 +228,7 @@ "name": "Bad", "source_mapping": { "start": 718, - "length": 493, + "length": 577, "filename_relative": "tests/detectors/immutable-states/0.8.0/immut_state_variables.sol", "filename_absolute": "/GENERIC_PATH", "filename_short": "tests/detectors/immutable-states/0.8.0/immut_state_variables.sol", @@ -247,7 +253,9 @@ 53, 54, 55, - 56 + 56, + 57, + 58 ], "starting_column": 1, "ending_column": 2 diff --git a/tests/src_mapping/ReferencesUserDefinedAliases.sol b/tests/src_mapping/ReferencesUserDefinedAliases.sol new file mode 100644 index 000000000..68fac48af --- /dev/null +++ b/tests/src_mapping/ReferencesUserDefinedAliases.sol @@ -0,0 +1,19 @@ +pragma solidity 0.8.16; + +type aliasTopLevel is uint; + +contract C +{ + type aliasContractLevel is uint; +} + +contract Test +{ + aliasTopLevel a; + C.aliasContractLevel b; +} + +function f(aliasTopLevel, C.aliasContractLevel) +{ + +} \ No newline at end of file diff --git a/tests/src_mapping/ReferencesUserDefinedTypesCasting.sol b/tests/src_mapping/ReferencesUserDefinedTypesCasting.sol new file mode 100644 index 000000000..1c23f0d5c --- /dev/null +++ b/tests/src_mapping/ReferencesUserDefinedTypesCasting.sol @@ -0,0 +1,19 @@ +pragma solidity 0.8.16; + +interface A +{ + function a() external; +} + +contract C +{ + function g(address _address) private + { + A(_address).a(); + } +} + +function f(address _address) +{ + A(_address).a(); +} \ No newline at end of file diff --git a/tests/test_ast_parsing.py b/tests/test_ast_parsing.py index 14130b428..c783f827f 100644 --- a/tests/test_ast_parsing.py +++ b/tests/test_ast_parsing.py @@ -443,6 +443,7 @@ ALL_TESTS = [ Test("top-level-struct-0.8.0.sol", ["0.8.0"]), Test("yul-top-level-0.8.0.sol", ["0.8.0"]), Test("complex_imports/import_aliases_issue_1319/test.sol", ["0.5.12"]), + Test("yul-state-constant-access.sol", ["0.8.16"]), ] # create the output folder if needed try: diff --git a/tests/test_source_mapping.py b/tests/test_source_mapping.py index 2ada121fe..c5ff28c67 100644 --- a/tests/test_source_mapping.py +++ b/tests/test_source_mapping.py @@ -81,3 +81,53 @@ def test_source_mapping(): (x.start, x.end) for x in slither.offset_to_implementations("tests/src_mapping/inheritance.sol", 93) } == {(17, 53), (193, 230), (129, 166)} + + +def _sort_references_lines(refs: list) -> list: + return sorted([ref.lines[0] for ref in refs]) + + +def _test_references_user_defined_aliases(): + """ + Tests if references are filled correctly for user defined aliases (declared using "type [...] is [...]" statement). + """ + solc_select.switch_global_version("0.8.16", always_install=True) + slither = Slither("tests/src_mapping/ReferencesUserDefinedAliases.sol") + + alias_top_level = slither.compilation_units[0].user_defined_value_types["aliasTopLevel"] + assert len(alias_top_level.references) == 2 + lines = _sort_references_lines(alias_top_level.references) + assert lines == [12, 16] + + alias_contract_level = ( + slither.compilation_units[0] + .contracts[0] + .file_scope.user_defined_types["C.aliasContractLevel"] + ) + assert len(alias_contract_level.references) == 2 + lines = _sort_references_lines(alias_contract_level.references) + assert lines == [13, 16] + + +def _test_references_user_defined_types_when_casting(): + """ + Tests if references are filled correctly for user defined types in case of casting. + """ + solc_select.switch_global_version("0.8.16", always_install=True) + slither = Slither("tests/src_mapping/ReferencesUserDefinedTypesCasting.sol") + + contracts = slither.compilation_units[0].contracts + a = contracts[0] if contracts[0].is_interface else contracts[1] + assert len(a.references) == 2 + lines = _sort_references_lines(a.references) + assert lines == [12, 18] + + +def test_references(): + """ + Tests if references list is filled correctly in the following cases: + - user defined aliases (declared using "type [...] is [...]" statement) + - user defined types in case of casting (TypeConversion expressions) + """ + _test_references_user_defined_aliases() + _test_references_user_defined_types_when_casting()