diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 000000000..dd3015cbc --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,8 @@ +--- +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + target-branch: "dev" + schedule: + interval: "weekly" diff --git a/.github/workflows/pip-audit.yml b/.github/workflows/pip-audit.yml index 20367787d..4fbc1a3fd 100644 --- a/.github/workflows/pip-audit.yml +++ b/.github/workflows/pip-audit.yml @@ -34,6 +34,6 @@ jobs: python -m pip install . - name: Run pip-audit - uses: pypa/gh-action-pip-audit@v1.0.7 + uses: pypa/gh-action-pip-audit@v1.0.8 with: virtual-environment: /tmp/pip-audit-env diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml new file mode 100644 index 000000000..529752f80 --- /dev/null +++ b/.github/workflows/publish.yml @@ -0,0 +1,54 @@ +name: Publish to PyPI + +on: + release: + types: [published] + +jobs: + build-release: + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.x' + + - name: Build distributions + run: | + python -m pip install --upgrade pip + python -m pip install build + python -m build + - name: Upload distributions + uses: actions/upload-artifact@v3 + with: + name: slither-dists + path: dist/ + + publish: + runs-on: ubuntu-latest + environment: release + permissions: + id-token: write # For trusted publishing + codesigning. + contents: write # For attaching signing artifacts to the release. + needs: + - build-release + steps: + - name: fetch dists + uses: actions/download-artifact@v3 + with: + name: slither-dists + path: dist/ + + - name: publish + uses: pypa/gh-action-pypi-publish@v1.8.6 + + - name: sign + uses: sigstore/gh-action-sigstore-python@v1.2.3 + with: + inputs: ./dist/*.tar.gz ./dist/*.whl + release-signing-artifacts: true + bundle-only: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1d1a9497f..5cf02136b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ For each new detector, at least one regression tests must be present. 1. Create a folder in `tests/e2e/detectors/test_data` with the detector's argument name. 2. Create a test contract in `tests/e2e/detectors/test_data//`. -3. Update `ALL_TEST` in `tests/e2e/detectors/test_detectors.py` +3. Update `ALL_TESTS` in `tests/e2e/detectors/test_detectors.py`. 4. Run `python tests/e2e/detectors/test_detectors.py --compile` to create a zip file of the compilation artifacts. 5. `pytest tests/e2e/detectors/test_detectors.py --insta update-new`. This will generate a snapshot of the detector output in `tests/e2e/detectors/snapshots/`. If updating an existing detector, run `pytest tests/e2e/detectors/test_detectors.py --insta review` and accept or reject the updates. 6. Run `pytest tests/e2e/detectors/test_detectors.py` to ensure everything worked. Then, add and commit the files to git. @@ -97,8 +97,9 @@ For each new detector, at least one regression tests must be present. 1. Create a test in `tests/e2e/solc_parsing/` 2. Run `python tests/e2e/solc_parsing/test_ast_parsing.py --compile`. This will compile the artifact in `tests/e2e/solc_parsing/compile`. Add the compiled artifact to git. -3. Run `python tests/e2e/solc_parsing/test_ast_parsing.py --generate`. This will generate the json artifacts in `tests/e2e/solc_parsing/expected_json`. Add the generated files to git. -4. Run `pytest tests/e2e/solc_parsing/test_ast_parsing.py` and check that everything worked. +3. Update `ALL_TESTS` in `tests/e2e/solc_parsing/test_ast_parsing.py`. +4. Run `python tests/e2e/solc_parsing/test_ast_parsing.py --generate`. This will generate the json artifacts in `tests/e2e/solc_parsing/expected_json`. Add the generated files to git. +5. Run `pytest tests/e2e/solc_parsing/test_ast_parsing.py` and check that everything worked. > ##### Helpful commands for parsing tests > @@ -106,7 +107,7 @@ For each new detector, at least one regression tests must be present. > - To run tests for a specific test case, run `pytest tests/e2e/solc_parsing/test_ast_parsing.py -k user_defined_value_type` (the filename is the argument). > - To run tests for a specific version, run `pytest tests/e2e/solc_parsing/test_ast_parsing.py -k 0.8.12`. > - To run tests for a specific compiler json format, run `pytest tests/e2e/solc_parsing/test_ast_parsing.py -k legacy` (can be legacy or compact). -> - The IDs of tests can be inspected using ``pytest tests/e2e/solc_parsing/test_ast_parsing.py --collect-only`. +> - The IDs of tests can be inspected using `pytest tests/e2e/solc_parsing/test_ast_parsing.py --collect-only`. ### Synchronization with crytic-compile diff --git a/scripts/ci_test_printers.sh b/scripts/ci_test_printers.sh index 61994b337..d13932b81 100755 --- a/scripts/ci_test_printers.sh +++ b/scripts/ci_test_printers.sh @@ -1,11 +1,11 @@ #!/usr/bin/env bash -### Test printer +### Test printer cd tests/e2e/solc_parsing/test_data/compile/ || exit # Do not test the evm printer,as it needs a refactoring -ALL_PRINTERS="cfg,constructor-calls,contract-summary,data-dependency,echidna,function-id,function-summary,modifiers,call-graph,human-summary,inheritance,inheritance-graph,slithir,slithir-ssa,vars-and-auth,require,variable-order,declaration" +ALL_PRINTERS="cfg,constructor-calls,contract-summary,data-dependency,echidna,function-id,function-summary,modifiers,call-graph,human-summary,inheritance,inheritance-graph,loc,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 diff --git a/setup.py b/setup.py index 70d4f71fd..798d43936 100644 --- a/setup.py +++ b/setup.py @@ -8,15 +8,15 @@ setup( description="Slither is a Solidity static analysis framework written in Python 3.", url="https://github.com/crytic/slither", author="Trail of Bits", - version="0.9.3", + version="0.9.4", packages=find_packages(), python_requires=">=3.8", install_requires=[ "packaging", "prettytable>=3.3.0", "pycryptodome>=3.4.6", - # "crytic-compile>=0.3.1,<0.4.0", - "crytic-compile@git+https://github.com/crytic/crytic-compile.git@dev#egg=crytic-compile", + "crytic-compile>=0.3.2,<0.4.0", + # "crytic-compile@git+https://github.com/crytic/crytic-compile.git@dev#egg=crytic-compile", "web3>=6.0.0", "eth-abi>=4.0.0", "eth-typing>=3.0.0", diff --git a/slither/__main__.py b/slither/__main__.py index ca61a8269..d9201a90d 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -35,6 +35,7 @@ from slither.utils.output import ( from slither.utils.output_capture import StandardOutputCapture from slither.utils.colors import red, set_colorization_enabled from slither.utils.command_line import ( + FailOnLevel, output_detectors, output_results_to_markdown, output_detectors_json, @@ -206,22 +207,22 @@ def choose_detectors( detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT) return detectors_to_run - if args.exclude_optimization and not args.fail_pedantic: + if args.exclude_optimization: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.OPTIMIZATION ] - if args.exclude_informational and not args.fail_pedantic: + if args.exclude_informational: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.INFORMATIONAL ] - if args.exclude_low and not args.fail_low: + if args.exclude_low: detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.LOW] - if args.exclude_medium and not args.fail_medium: + if args.exclude_medium: detectors_to_run = [ d for d in detectors_to_run if d.IMPACT != DetectorClassification.MEDIUM ] - if args.exclude_high and not args.fail_high: + if args.exclude_high: detectors_to_run = [d for d in detectors_to_run if d.IMPACT != DetectorClassification.HIGH] if args.detectors_to_exclude: detectors_to_run = [ @@ -386,41 +387,44 @@ def parse_args( default=defaults_flag_in_config["exclude_high"], ) - group_detector.add_argument( + fail_on_group = group_detector.add_mutually_exclusive_group() + fail_on_group.add_argument( "--fail-pedantic", - help="Return the number of findings in the exit code", - action="store_true", - default=defaults_flag_in_config["fail_pedantic"], + help="Fail if any findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.PEDANTIC, ) - - group_detector.add_argument( - "--no-fail-pedantic", - help="Do not return the number of findings in the exit code. Opposite of --fail-pedantic", - dest="fail_pedantic", - action="store_false", - required=False, - ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-low", - help="Fail if low or greater impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_low"], + help="Fail if any low or greater impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.LOW, ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-medium", - help="Fail if medium or greater impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_medium"], + help="Fail if any medium or greater impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.MEDIUM, ) - - group_detector.add_argument( + fail_on_group.add_argument( "--fail-high", - help="Fail if high impact finding is detected", - action="store_true", - default=defaults_flag_in_config["fail_high"], + help="Fail if any high impact findings are detected", + action="store_const", + dest="fail_on", + const=FailOnLevel.HIGH, + ) + fail_on_group.add_argument( + "--fail-none", + "--no-fail-pedantic", + help="Do not return the number of findings in the exit code", + action="store_const", + dest="fail_on", + const=FailOnLevel.NONE, ) + fail_on_group.set_defaults(fail_on=FailOnLevel.PEDANTIC) group_detector.add_argument( "--show-ignored-findings", @@ -896,17 +900,18 @@ def main_impl( stats = pstats.Stats(cp).sort_stats("cumtime") stats.print_stats() - if args.fail_high: + fail_on = FailOnLevel(args.fail_on) + if fail_on == FailOnLevel.HIGH: fail_on_detection = any(result["impact"] == "High" for result in results_detectors) - elif args.fail_medium: + elif fail_on == FailOnLevel.MEDIUM: fail_on_detection = any( result["impact"] in ["Medium", "High"] for result in results_detectors ) - elif args.fail_low: + elif fail_on == FailOnLevel.LOW: fail_on_detection = any( result["impact"] in ["Low", "Medium", "High"] for result in results_detectors ) - elif args.fail_pedantic: + elif fail_on == FailOnLevel.PEDANTIC: fail_on_detection = bool(results_detectors) else: fail_on_detection = False diff --git a/slither/analyses/evm/convert.py b/slither/analyses/evm/convert.py index bff308cbc..5e8c4a128 100644 --- a/slither/analyses/evm/convert.py +++ b/slither/analyses/evm/convert.py @@ -186,7 +186,7 @@ def generate_source_to_evm_ins_mapping(evm_instructions, srcmap_runtime, slither if mapping_item[i] == "": mapping_item[i] = int(prev_mapping[i]) - offset, _length, file_id, _ = mapping_item + offset, _length, file_id, *_ = mapping_item prev_mapping = mapping_item if file_id == "-1": diff --git a/slither/core/cfg/scope.py b/slither/core/cfg/scope.py index d3ac4e836..3f9959851 100644 --- a/slither/core/cfg/scope.py +++ b/slither/core/cfg/scope.py @@ -7,8 +7,10 @@ if TYPE_CHECKING: # pylint: disable=too-few-public-methods class Scope: - def __init__(self, is_checked: bool, is_yul: bool, scope: Union["Scope", "Function"]) -> None: + def __init__( + self, is_checked: bool, is_yul: bool, parent_scope: Union["Scope", "Function"] + ) -> None: self.nodes: List["Node"] = [] self.is_checked = is_checked self.is_yul = is_yul - self.father = scope + self.father = parent_scope diff --git a/slither/core/declarations/event.py b/slither/core/declarations/event.py index 9d42ac224..1b58ff63b 100644 --- a/slither/core/declarations/event.py +++ b/slither/core/declarations/event.py @@ -45,7 +45,7 @@ class Event(ContractLevel, SourceMapping): Returns: str: contract.func_name(type1,type2) """ - return self.contract.name + self.full_name + return self.contract.name + "." + self.full_name @property def elems(self) -> List["EventVariable"]: diff --git a/slither/core/declarations/solidity_variables.py b/slither/core/declarations/solidity_variables.py index 552cf9e7f..f6a0f0839 100644 --- a/slither/core/declarations/solidity_variables.py +++ b/slither/core/declarations/solidity_variables.py @@ -21,10 +21,11 @@ SOLIDITY_VARIABLES_COMPOSED = { "block.basefee": "uint", "block.coinbase": "address", "block.difficulty": "uint256", + "block.prevrandao": "uint256", "block.gaslimit": "uint256", "block.number": "uint256", "block.timestamp": "uint256", - "block.blockhash": "uint256", # alias for blockhash. It's a call + "block.blockhash": "bytes32", # alias for blockhash. It's a call "block.chainid": "uint256", "msg.data": "bytes", "msg.gas": "uint256", @@ -60,6 +61,7 @@ SOLIDITY_FUNCTIONS: Dict[str, List[str]] = { "log2(bytes32,bytes32,bytes32)": [], "log3(bytes32,bytes32,bytes32,bytes32)": [], "blockhash(uint256)": ["bytes32"], + "prevrandao()": ["uint256"], # the following need a special handling # as they are recognized as a SolidityVariableComposed # and converted to a SolidityFunction by SlithIR diff --git a/slither/core/expressions/new_array.py b/slither/core/expressions/new_array.py index 162b48f1e..b1d407793 100644 --- a/slither/core/expressions/new_array.py +++ b/slither/core/expressions/new_array.py @@ -1,32 +1,23 @@ -from typing import Union, TYPE_CHECKING - +from typing import TYPE_CHECKING from slither.core.expressions.expression import Expression -from slither.core.solidity_types.type import Type if TYPE_CHECKING: - from slither.core.solidity_types.elementary_type import ElementaryType - from slither.core.solidity_types.type_alias import TypeAliasTopLevel + from slither.core.solidity_types.array_type import ArrayType class NewArray(Expression): - - # note: dont conserve the size of the array if provided - def __init__( - self, depth: int, array_type: Union["TypeAliasTopLevel", "ElementaryType"] - ) -> None: + def __init__(self, array_type: "ArrayType") -> None: super().__init__() - assert isinstance(array_type, Type) - self._depth: int = depth - self._array_type: Type = array_type + # pylint: disable=import-outside-toplevel + from slither.core.solidity_types.array_type import ArrayType - @property - def array_type(self) -> Type: - return self._array_type + assert isinstance(array_type, ArrayType) + self._array_type = array_type @property - def depth(self) -> int: - return self._depth + def array_type(self) -> "ArrayType": + return self._array_type def __str__(self): - return "new " + str(self._array_type) + "[]" * self._depth + return "new " + str(self._array_type) diff --git a/slither/core/solidity_types/type_alias.py b/slither/core/solidity_types/type_alias.py index 9387f511a..ead9b5394 100644 --- a/slither/core/solidity_types/type_alias.py +++ b/slither/core/solidity_types/type_alias.py @@ -1,10 +1,11 @@ -from typing import TYPE_CHECKING, Tuple +from typing import TYPE_CHECKING, Tuple, Dict from slither.core.declarations.top_level import TopLevel from slither.core.declarations.contract_level import ContractLevel from slither.core.solidity_types import Type, ElementaryType if TYPE_CHECKING: + from slither.core.declarations.function_top_level import FunctionTopLevel from slither.core.declarations import Contract from slither.core.scope.scope import FileScope @@ -43,6 +44,8 @@ class TypeAliasTopLevel(TypeAlias, TopLevel): def __init__(self, underlying_type: ElementaryType, name: str, scope: "FileScope") -> None: super().__init__(underlying_type, name) self.file_scope: "FileScope" = scope + # operators redefined + self.operators: Dict[str, "FunctionTopLevel"] = {} def __str__(self) -> str: return self.name diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 1a4121083..57f44bc56 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -89,4 +89,6 @@ from .functions.protected_variable import ProtectedVariables from .functions.permit_domain_signature_collision import DomainSeparatorCollision from .functions.codex import Codex from .functions.cyclomatic_complexity import CyclomaticComplexity +from .operations.cache_array_length import CacheArrayLength +from .statements.incorrect_using_for import IncorrectUsingFor from .operations.encode_packed import EncodePackedCollision diff --git a/slither/detectors/attributes/locked_ether.py b/slither/detectors/attributes/locked_ether.py index a6f882922..91ec68650 100644 --- a/slither/detectors/attributes/locked_ether.py +++ b/slither/detectors/attributes/locked_ether.py @@ -3,7 +3,7 @@ """ from typing import List -from slither.core.declarations.contract import Contract +from slither.core.declarations import Contract, SolidityFunction from slither.detectors.abstract_detector import ( AbstractDetector, DetectorClassification, @@ -17,7 +17,9 @@ from slither.slithir.operations import ( NewContract, LibraryCall, InternalCall, + SolidityCall, ) +from slither.slithir.variables import Constant from slither.utils.output import Output @@ -68,8 +70,28 @@ Every Ether sent to `Locked` will be lost.""" ): if ir.call_value and ir.call_value != 0: return False - if isinstance(ir, (LowLevelCall)): - if ir.function_name in ["delegatecall", "callcode"]: + if isinstance(ir, (LowLevelCall)) and ir.function_name in [ + "delegatecall", + "callcode", + ]: + return False + if isinstance(ir, SolidityCall): + call_can_send_ether = ir.function in [ + SolidityFunction( + "delegatecall(uint256,uint256,uint256,uint256,uint256,uint256)" + ), + SolidityFunction( + "callcode(uint256,uint256,uint256,uint256,uint256,uint256,uint256)" + ), + SolidityFunction( + "call(uint256,uint256,uint256,uint256,uint256,uint256,uint256)" + ), + ] + nonzero_call_value = call_can_send_ether and ( + not isinstance(ir.arguments[2], Constant) + or ir.arguments[2].value != 0 + ) + if nonzero_call_value: return False # If a new internal call or librarycall # Add it to the list to explore diff --git a/slither/detectors/compiler_bugs/array_by_reference.py b/slither/detectors/compiler_bugs/array_by_reference.py index 04dfe085a..47e2af581 100644 --- a/slither/detectors/compiler_bugs/array_by_reference.py +++ b/slither/detectors/compiler_bugs/array_by_reference.py @@ -133,7 +133,7 @@ As a result, Bob's usage of the contract is incorrect.""" continue # Verify one of these parameters is an array in storage. - for arg in ir.arguments: + for (param, arg) in zip(ir.function.parameters, ir.arguments): # Verify this argument is a variable that is an array type. if not isinstance(arg, (StateVariable, LocalVariable)): continue @@ -141,8 +141,11 @@ As a result, Bob's usage of the contract is incorrect.""" continue # If it is a state variable OR a local variable referencing storage, we add it to the list. - if isinstance(arg, StateVariable) or ( - isinstance(arg, LocalVariable) and arg.location == "storage" + if ( + isinstance(arg, StateVariable) + or (isinstance(arg, LocalVariable) and arg.location == "storage") + ) and ( + isinstance(param.type, ArrayType) and param.location != "storage" ): results.append((node, arg, ir.function)) return results @@ -165,9 +168,9 @@ As a result, Bob's usage of the contract is incorrect.""" calling_node.function, " passes array ", affected_argument, - "by reference to ", + " by reference to ", invoked_function, - "which only takes arrays by value\n", + " which only takes arrays by value\n", ] res = self.generate_result(info) diff --git a/slither/detectors/operations/cache_array_length.py b/slither/detectors/operations/cache_array_length.py new file mode 100644 index 000000000..da73d3fd5 --- /dev/null +++ b/slither/detectors/operations/cache_array_length.py @@ -0,0 +1,221 @@ +from typing import List, Set + +from slither.core.cfg.node import Node, NodeType +from slither.core.declarations import Function +from slither.core.expressions import BinaryOperation, Identifier, MemberAccess, UnaryOperation +from slither.core.solidity_types import ArrayType +from slither.core.source_mapping.source_mapping import SourceMapping +from slither.core.variables import StateVariable +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.slithir.operations import Length, Delete, HighLevelCall + + +class CacheArrayLength(AbstractDetector): + """ + Detects `for` loops that use `length` member of some storage array in their loop condition and don't modify it. + """ + + ARGUMENT = "cache-array-length" + HELP = ( + "Detects `for` loops that use `length` member of some storage array in their loop condition and don't " + "modify it. " + ) + IMPACT = DetectorClassification.OPTIMIZATION + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#cache-array-length" + + WIKI_TITLE = "Cache array length" + WIKI_DESCRIPTION = ( + "Detects `for` loops that use `length` member of some storage array in their loop condition " + "and don't modify it. " + ) + WIKI_EXPLOIT_SCENARIO = """ +```solidity +contract C +{ + uint[] array; + + function f() public + { + for (uint i = 0; i < array.length; i++) + { + // code that does not modify length of `array` + } + } +} +``` +Since the `for` loop in `f` doesn't modify `array.length`, it is more gas efficient to cache it in some local variable and use that variable instead, like in the following example: + +```solidity +contract C +{ + uint[] array; + + function f() public + { + uint array_length = array.length; + for (uint i = 0; i < array_length; i++) + { + // code that does not modify length of `array` + } + } +} +``` + """ + WIKI_RECOMMENDATION = ( + "Cache the lengths of storage arrays if they are used and not modified in `for` loops." + ) + + @staticmethod + def _is_identifier_member_access_comparison(exp: BinaryOperation) -> bool: + """ + Checks whether a BinaryOperation `exp` is an operation on Identifier and MemberAccess. + """ + return ( + isinstance(exp.expression_left, Identifier) + and isinstance(exp.expression_right, MemberAccess) + ) or ( + isinstance(exp.expression_left, MemberAccess) + and isinstance(exp.expression_right, Identifier) + ) + + @staticmethod + def _extract_array_from_length_member_access(exp: MemberAccess) -> StateVariable: + """ + Given a member access `exp`, it returns state array which `length` member is accessed through `exp`. + If array is not a state array or its `length` member is not referenced, it returns `None`. + """ + if exp.member_name != "length": + return None + if not isinstance(exp.expression, Identifier): + return None + if not isinstance(exp.expression.value, StateVariable): + return None + if not isinstance(exp.expression.value.type, ArrayType): + return None + return exp.expression.value + + @staticmethod + def _is_loop_referencing_array_length( + node: Node, visited: Set[Node], array: StateVariable, depth: int + ) -> True: + """ + For a given loop, checks if it references `array.length` at some point. + Will also return True if `array.length` is referenced but not changed. + This may potentially generate false negatives in the detector, but it was done this way because: + - situations when array `length` is referenced but not modified in loop are rare + - checking if `array.length` is indeed modified would require much more work + """ + visited.add(node) + if node.type == NodeType.STARTLOOP: + depth += 1 + if node.type == NodeType.ENDLOOP: + depth -= 1 + if depth == 0: + return False + + # Array length may change in the following situations: + # - when `push` is called + # - when `pop` is called + # - when `delete` is called on the entire array + # - when external function call is made (instructions from internal function calls are already in + # `node.all_slithir_operations()`, so we don't need to handle internal calls separately) + if node.type == NodeType.EXPRESSION: + for op in node.all_slithir_operations(): + if isinstance(op, Length) and op.value == array: + # op accesses array.length, not necessarily modifying it + return True + if isinstance(op, Delete): + # take into account only delete entire array, since delete array[i] doesn't change `array.length` + if ( + isinstance(op.expression, UnaryOperation) + and isinstance(op.expression.expression, Identifier) + and op.expression.expression.value == array + ): + return True + if isinstance(op, HighLevelCall) and not op.function.view and not op.function.pure: + return True + + for son in node.sons: + if son not in visited: + if CacheArrayLength._is_loop_referencing_array_length(son, visited, array, depth): + return True + return False + + @staticmethod + def _handle_loops(nodes: List[Node], non_optimal_array_len_usages: List[SourceMapping]) -> None: + """ + For each loop, checks if it has a comparison with `length` array member and, if it has, checks whether that + array size could potentially change in that loop. + If it cannot, the loop condition is added to `non_optimal_array_len_usages`. + There may be some false negatives here - see docs for `_is_loop_referencing_array_length` for more information. + """ + for node in nodes: + if node.type == NodeType.STARTLOOP: + if_node = node.sons[0] + if if_node.type != NodeType.IFLOOP: + continue + if not isinstance(if_node.expression, BinaryOperation): + continue + exp: BinaryOperation = if_node.expression + if not CacheArrayLength._is_identifier_member_access_comparison(exp): + continue + array: StateVariable + if isinstance(exp.expression_right, MemberAccess): + array = CacheArrayLength._extract_array_from_length_member_access( + exp.expression_right + ) + else: # isinstance(exp.expression_left, MemberAccess) == True + array = CacheArrayLength._extract_array_from_length_member_access( + exp.expression_left + ) + if array is None: + continue + + visited: Set[Node] = set() + if not CacheArrayLength._is_loop_referencing_array_length( + if_node, visited, array, 1 + ): + non_optimal_array_len_usages.append(if_node) + + @staticmethod + def _get_non_optimal_array_len_usages_for_function(f: Function) -> List[SourceMapping]: + """ + Finds non-optimal usages of array length in loop conditions in a given function. + """ + non_optimal_array_len_usages: List[SourceMapping] = [] + CacheArrayLength._handle_loops(f.nodes, non_optimal_array_len_usages) + + return non_optimal_array_len_usages + + @staticmethod + def _get_non_optimal_array_len_usages(functions: List[Function]) -> List[SourceMapping]: + """ + Finds non-optimal usages of array length in loop conditions in given functions. + """ + non_optimal_array_len_usages: List[SourceMapping] = [] + + for f in functions: + non_optimal_array_len_usages += ( + CacheArrayLength._get_non_optimal_array_len_usages_for_function(f) + ) + + return non_optimal_array_len_usages + + def _detect(self): + results = [] + + non_optimal_array_len_usages = CacheArrayLength._get_non_optimal_array_len_usages( + self.compilation_unit.functions + ) + for usage in non_optimal_array_len_usages: + info = [ + "Loop condition at ", + usage, + " should use cached array length instead of referencing `length` member " + "of the storage array.\n ", + ] + res = self.generate_result(info) + results.append(res) + return results diff --git a/slither/detectors/statements/incorrect_using_for.py b/slither/detectors/statements/incorrect_using_for.py new file mode 100644 index 000000000..e2e87e7e0 --- /dev/null +++ b/slither/detectors/statements/incorrect_using_for.py @@ -0,0 +1,223 @@ +from typing import List + +from slither.core.declarations import Contract, Structure, Enum +from slither.core.declarations.using_for_top_level import UsingForTopLevel +from slither.core.solidity_types import ( + UserDefinedType, + Type, + ElementaryType, + TypeAlias, + MappingType, + ArrayType, +) +from slither.core.solidity_types.elementary_type import Uint, Int, Byte +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output + + +def _is_correctly_used(type_: Type, library: Contract) -> bool: + """ + Checks if a `using library for type_` statement is used correctly (that is, does library contain any function + with type_ as the first argument). + """ + for f in library.functions: + if len(f.parameters) == 0: + continue + if f.parameters[0].type and not _implicitly_convertible_to(type_, f.parameters[0].type): + continue + return True + return False + + +def _implicitly_convertible_to(type1: Type, type2: Type) -> bool: + """ + Returns True if type1 may be implicitly converted to type2. + """ + if isinstance(type1, TypeAlias) or isinstance(type2, TypeAlias): + if isinstance(type1, TypeAlias) and isinstance(type2, TypeAlias): + return type1.type == type2.type + return False + + if isinstance(type1, UserDefinedType) and isinstance(type2, UserDefinedType): + if isinstance(type1.type, Contract) and isinstance(type2.type, Contract): + return _implicitly_convertible_to_for_contracts(type1.type, type2.type) + + if isinstance(type1.type, Structure) and isinstance(type2.type, Structure): + return type1.type.canonical_name == type2.type.canonical_name + + if isinstance(type1.type, Enum) and isinstance(type2.type, Enum): + return type1.type.canonical_name == type2.type.canonical_name + + if isinstance(type1, ElementaryType) and isinstance(type2, ElementaryType): + return _implicitly_convertible_to_for_elementary_types(type1, type2) + + if isinstance(type1, MappingType) and isinstance(type2, MappingType): + return _implicitly_convertible_to_for_mappings(type1, type2) + + if isinstance(type1, ArrayType) and isinstance(type2, ArrayType): + return _implicitly_convertible_to_for_arrays(type1, type2) + + return False + + +def _implicitly_convertible_to_for_arrays(type1: ArrayType, type2: ArrayType) -> bool: + """ + Returns True if type1 may be implicitly converted to type2. + """ + return _implicitly_convertible_to(type1.type, type2.type) + + +def _implicitly_convertible_to_for_mappings(type1: MappingType, type2: MappingType) -> bool: + """ + Returns True if type1 may be implicitly converted to type2. + """ + return type1.type_from == type2.type_from and type1.type_to == type2.type_to + + +def _implicitly_convertible_to_for_elementary_types( + type1: ElementaryType, type2: ElementaryType +) -> bool: + """ + Returns True if type1 may be implicitly converted to type2. + """ + if type1.type == "bool" and type2.type == "bool": + return True + if type1.type == "string" and type2.type == "string": + return True + if type1.type == "bytes" and type2.type == "bytes": + return True + if type1.type == "address" and type2.type == "address": + return _implicitly_convertible_to_for_addresses(type1, type2) + if type1.type in Uint and type2.type in Uint: + return _implicitly_convertible_to_for_uints(type1, type2) + if type1.type in Int and type2.type in Int: + return _implicitly_convertible_to_for_ints(type1, type2) + if ( + type1.type != "bytes" + and type2.type != "bytes" + and type1.type in Byte + and type2.type in Byte + ): + return _implicitly_convertible_to_for_bytes(type1, type2) + return False + + +def _implicitly_convertible_to_for_bytes(type1: ElementaryType, type2: ElementaryType) -> bool: + """ + Returns True if type1 may be implicitly converted to type2 assuming they are both bytes. + """ + assert type1.type in Byte and type2.type in Byte + assert type1.size is not None + assert type2.size is not None + + return type1.size <= type2.size + + +def _implicitly_convertible_to_for_addresses(type1: ElementaryType, type2: ElementaryType) -> bool: + """ + Returns True if type1 may be implicitly converted to type2 assuming they are both addresses. + """ + assert type1.type == "address" and type2.type == "address" + # payable attribute to be implemented; for now, always return True + return True + + +def _implicitly_convertible_to_for_ints(type1: ElementaryType, type2: ElementaryType) -> bool: + """ + Returns True if type1 may be implicitly converted to type2 assuming they are both ints. + """ + assert type1.type in Int and type2.type in Int + assert type1.size is not None + assert type2.size is not None + + return type1.size <= type2.size + + +def _implicitly_convertible_to_for_uints(type1: ElementaryType, type2: ElementaryType) -> bool: + """ + Returns True if type1 may be implicitly converted to type2 assuming they are both uints. + """ + assert type1.type in Uint and type2.type in Uint + assert type1.size is not None + assert type2.size is not None + + return type1.size <= type2.size + + +def _implicitly_convertible_to_for_contracts(contract1: Contract, contract2: Contract) -> bool: + """ + Returns True if contract1 may be implicitly converted to contract2. + """ + return contract1 == contract2 or contract2 in contract1.inheritance + + +class IncorrectUsingFor(AbstractDetector): + """ + Detector for incorrect using-for statement usage. + """ + + ARGUMENT = "incorrect-using-for" + HELP = "Detects using-for statement usage when no function from a given library matches a given type" + IMPACT = DetectorClassification.INFORMATIONAL + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-using-for-usage" + + WIKI_TITLE = "Incorrect usage of using-for statement" + WIKI_DESCRIPTION = ( + "In Solidity, it is possible to use libraries for certain types, by the `using-for` statement " + "(`using for `). However, the Solidity compiler doesn't check whether a given " + "library has at least one function matching a given type. If it doesn't, such a statement has " + "no effect and may be confusing. " + ) + + # region wiki_exploit_scenario + WIKI_EXPLOIT_SCENARIO = """ + ```solidity + library L { + function f(bool) public pure {} + } + + using L for uint; + ``` + Such a code will compile despite the fact that `L` has no function with `uint` as its first argument.""" + # endregion wiki_exploit_scenario + WIKI_RECOMMENDATION = ( + "Make sure that the libraries used in `using-for` statements have at least one function " + "matching a type used in these statements. " + ) + + def _append_result( + self, results: List[Output], uf: UsingForTopLevel, type_: Type, library: Contract + ) -> None: + info: DETECTOR_INFO = [ + f"using-for statement at {uf.source_mapping} is incorrect - no matching function for {type_} found in ", + library, + ".\n", + ] + res = self.generate_result(info) + results.append(res) + + def _detect(self) -> List[Output]: + results: List[Output] = [] + + for uf in self.compilation_unit.using_for_top_level: + # UsingForTopLevel.using_for is a dict with a single entry, which is mapped to a list of functions/libraries + # the following code extracts the type from using-for and skips using-for statements with functions + type_ = list(uf.using_for.keys())[0] + for lib_or_fcn in uf.using_for[type_]: + # checking for using-for with functions is already performed by the compiler; we only consider libraries + if isinstance(lib_or_fcn, UserDefinedType): + lib_or_fcn_type = lib_or_fcn.type + if ( + isinstance(type_, Type) + and isinstance(lib_or_fcn_type, Contract) + and not _is_correctly_used(type_, lib_or_fcn_type) + ): + self._append_result(results, uf, type_, lib_or_fcn_type) + + return results diff --git a/slither/detectors/statements/msg_value_in_loop.py b/slither/detectors/statements/msg_value_in_loop.py index 55bd9bfc2..83c5658ca 100644 --- a/slither/detectors/statements/msg_value_in_loop.py +++ b/slither/detectors/statements/msg_value_in_loop.py @@ -79,7 +79,7 @@ contract MsgValueInLoop{ # endregion wiki_exploit_scenario WIKI_RECOMMENDATION = """ -Track msg.value through a local variable and decrease its amount on every iteration/usage. +Provide an explicit array of amounts alongside the receivers array, and check that the sum of all amounts matches `msg.value`. """ def _detect(self) -> List[Output]: diff --git a/slither/detectors/variables/similar_variables.py b/slither/detectors/variables/similar_variables.py index 465e1ce01..dccaf09c4 100644 --- a/slither/detectors/variables/similar_variables.py +++ b/slither/detectors/variables/similar_variables.py @@ -47,9 +47,7 @@ class SimilarVarsDetection(AbstractDetector): Returns: bool: true if names are similar """ - if len(seq1) != len(seq2): - return False - val = difflib.SequenceMatcher(a=seq1.lower(), b=seq2.lower()).ratio() + val = difflib.SequenceMatcher(a=seq1, b=seq2).ratio() ret = val > 0.90 return ret @@ -65,17 +63,23 @@ class SimilarVarsDetection(AbstractDetector): contract_var = contract.variables - all_var = set(all_var + contract_var) + all_var = list(set(all_var + contract_var)) + + ret = set() + # pylint: disable=consider-using-enumerate + for i in range(len(all_var)): + v1 = all_var[i] + _v1_name_lower = v1.name.lower() + for j in range(i, len(all_var)): + v2 = all_var[j] + if len(v1.name) != len(v2.name): + continue + _v2_name_lower = v2.name.lower() + if _v1_name_lower != _v2_name_lower: + if SimilarVarsDetection.similar(_v1_name_lower, _v2_name_lower): + ret.add((v1, v2)) - ret = [] - for v1 in all_var: - for v2 in all_var: - if v1.name.lower() != v2.name.lower(): - if SimilarVarsDetection.similar(v1.name, v2.name): - if (v2, v1) not in ret: - ret.append((v1, v2)) - - return set(ret) + return ret def _detect(self) -> List[Output]: """Detect similar variables name diff --git a/slither/detectors/variables/unchanged_state_variables.py b/slither/detectors/variables/unchanged_state_variables.py index f12cc5784..0e73ab57b 100644 --- a/slither/detectors/variables/unchanged_state_variables.py +++ b/slither/detectors/variables/unchanged_state_variables.py @@ -87,6 +87,8 @@ class UnchangedStateVariables: def detect(self) -> None: """Detect state variables that could be constant or immutable""" for c in self.compilation_unit.contracts_derived: + if c.is_signature_only(): + continue variables = [] functions = [] diff --git a/slither/detectors/variables/unused_state_variables.py b/slither/detectors/variables/unused_state_variables.py index afb4e3ac5..830ca34ca 100644 --- a/slither/detectors/variables/unused_state_variables.py +++ b/slither/detectors/variables/unused_state_variables.py @@ -20,8 +20,6 @@ from slither.visitors.expression.export_values import ExportValues def detect_unused(contract: Contract) -> Optional[List[StateVariable]]: - if contract.is_signature_only(): - return None # Get all the variables read in all the functions and modifiers all_functions = [ @@ -73,6 +71,8 @@ class UnusedStateVars(AbstractDetector): """Detect unused state variables""" results = [] for c in self.compilation_unit.contracts_derived: + if c.is_signature_only(): + continue unusedVars = detect_unused(c) if unusedVars: for var in unusedVars: diff --git a/slither/printers/all_printers.py b/slither/printers/all_printers.py index 6dc8dddbd..5555fe708 100644 --- a/slither/printers/all_printers.py +++ b/slither/printers/all_printers.py @@ -1,6 +1,7 @@ # pylint: disable=unused-import,relative-beyond-top-level from .summary.function import FunctionSummary from .summary.contract import ContractSummary +from .summary.loc import LocPrinter from .inheritance.inheritance import PrinterInheritance from .inheritance.inheritance_graph import PrinterInheritanceGraph from .call.call_graph import PrinterCallGraph diff --git a/slither/printers/guidance/echidna.py b/slither/printers/guidance/echidna.py index acbf5b015..25e0968cd 100644 --- a/slither/printers/guidance/echidna.py +++ b/slither/printers/guidance/echidna.py @@ -32,7 +32,7 @@ from slither.slithir.operations import ( from slither.slithir.operations.binary import Binary from slither.slithir.variables import Constant from slither.utils.output import Output -from slither.visitors.expression.constants_folding import ConstantFolding +from slither.visitors.expression.constants_folding import ConstantFolding, NotConstant def _get_name(f: Union[Function, Variable]) -> str: @@ -178,11 +178,16 @@ def _extract_constants_from_irs( # pylint: disable=too-many-branches,too-many-n all_cst_used_in_binary[str(ir.type)].append( ConstantValue(str(r.value), str(r.type)) ) - if isinstance(ir.variable_left, Constant) and isinstance(ir.variable_right, Constant): - if ir.lvalue: - type_ = ir.lvalue.type - cst = ConstantFolding(ir.expression, type_).result() - all_cst_used.append(ConstantValue(str(cst.value), str(type_))) + if isinstance(ir.variable_left, Constant) or isinstance( + ir.variable_right, Constant + ): + if ir.lvalue: + try: + type_ = ir.lvalue.type + cst = ConstantFolding(ir.expression, type_).result() + all_cst_used.append(ConstantValue(str(cst.value), str(type_))) + except NotConstant: + pass if isinstance(ir, TypeConversion): if isinstance(ir.variable, Constant): if isinstance(ir.type, TypeAlias): diff --git a/slither/printers/summary/human_summary.py b/slither/printers/summary/human_summary.py index 9eacb97c6..314335ebf 100644 --- a/slither/printers/summary/human_summary.py +++ b/slither/printers/summary/human_summary.py @@ -2,12 +2,12 @@ Module printing summary of the contract """ import logging -from pathlib import Path from typing import Tuple, List, Dict from slither.core.declarations import SolidityFunction, Function from slither.core.variables.state_variable import StateVariable from slither.printers.abstract_printer import AbstractPrinter +from slither.printers.summary.loc import compute_loc_metrics from slither.slithir.operations import ( LowLevelCall, HighLevelCall, @@ -21,7 +21,6 @@ from slither.utils.colors import green, red, yellow from slither.utils.myprettytable import MyPrettyTable from slither.utils.standard_libraries import is_standard_library from slither.core.cfg.node import NodeType -from slither.utils.tests_pattern import is_test_file class PrinterHumanSummary(AbstractPrinter): @@ -32,7 +31,6 @@ class PrinterHumanSummary(AbstractPrinter): @staticmethod def _get_summary_erc20(contract): - functions_name = [f.name for f in contract.functions] state_variables = [v.name for v in contract.state_variables] @@ -165,28 +163,7 @@ class PrinterHumanSummary(AbstractPrinter): def _number_functions(contract): return len(contract.functions) - def _lines_number(self): - if not self.slither.source_code: - return None - total_dep_lines = 0 - total_lines = 0 - total_tests_lines = 0 - - for filename, source_code in self.slither.source_code.items(): - lines = len(source_code.splitlines()) - is_dep = False - if self.slither.crytic_compile: - is_dep = self.slither.crytic_compile.is_dependency(filename) - if is_dep: - total_dep_lines += lines - else: - if is_test_file(Path(filename)): - total_tests_lines += lines - else: - total_lines += lines - return total_lines, total_dep_lines, total_tests_lines - - def _get_number_of_assembly_lines(self): + def _get_number_of_assembly_lines(self) -> int: total_asm_lines = 0 for contract in self.contracts: for function in contract.functions_declared: @@ -202,9 +179,7 @@ class PrinterHumanSummary(AbstractPrinter): return "Compilation non standard\n" return f"Compiled with {str(self.slither.crytic_compile.type)}\n" - def _number_contracts(self): - if self.slither.crytic_compile is None: - return len(self.slither.contracts), 0, 0 + def _number_contracts(self) -> Tuple[int, int, int]: contracts = self.slither.contracts deps = [c for c in contracts if c.is_from_dependency()] tests = [c for c in contracts if c.is_test] @@ -226,7 +201,6 @@ class PrinterHumanSummary(AbstractPrinter): return list(set(ercs)) def _get_features(self, contract): # pylint: disable=too-many-branches - has_payable = False can_send_eth = False can_selfdestruct = False @@ -291,6 +265,36 @@ class PrinterHumanSummary(AbstractPrinter): "Proxy": contract.is_upgradeable_proxy, } + def _get_contracts(self, txt: str) -> str: + ( + number_contracts, + number_contracts_deps, + number_contracts_tests, + ) = self._number_contracts() + txt += f"Total number of contracts in source files: {number_contracts}\n" + if number_contracts_deps > 0: + txt += f"Number of contracts in dependencies: {number_contracts_deps}\n" + if number_contracts_tests > 0: + txt += f"Number of contracts in tests : {number_contracts_tests}\n" + return txt + + def _get_number_lines(self, txt: str, results: Dict) -> Tuple[str, Dict]: + loc = compute_loc_metrics(self.slither) + txt += "Source lines of code (SLOC) in source files: " + txt += f"{loc.src.sloc}\n" + if loc.dep.sloc > 0: + txt += "Source lines of code (SLOC) in dependencies: " + txt += f"{loc.dep.sloc}\n" + if loc.test.sloc > 0: + txt += "Source lines of code (SLOC) in tests : " + txt += f"{loc.test.sloc}\n" + results["number_lines"] = loc.src.sloc + results["number_lines__dependencies"] = loc.dep.sloc + total_asm_lines = self._get_number_of_assembly_lines() + txt += f"Number of assembly lines: {total_asm_lines}\n" + results["number_lines_assembly"] = total_asm_lines + return txt, results + def output(self, _filename): # pylint: disable=too-many-locals,too-many-statements """ _filename is not used @@ -311,24 +315,8 @@ class PrinterHumanSummary(AbstractPrinter): "number_findings": {}, "detectors": [], } - - lines_number = self._lines_number() - if lines_number: - total_lines, total_dep_lines, total_tests_lines = lines_number - txt += f"Number of lines: {total_lines} (+ {total_dep_lines} in dependencies, + {total_tests_lines} in tests)\n" - results["number_lines"] = total_lines - results["number_lines__dependencies"] = total_dep_lines - total_asm_lines = self._get_number_of_assembly_lines() - txt += f"Number of assembly lines: {total_asm_lines}\n" - results["number_lines_assembly"] = total_asm_lines - - ( - number_contracts, - number_contracts_deps, - number_contracts_tests, - ) = self._number_contracts() - txt += f"Number of contracts: {number_contracts} (+ {number_contracts_deps} in dependencies, + {number_contracts_tests} tests) \n\n" - + txt = self._get_contracts(txt) + txt, results = self._get_number_lines(txt, results) ( txt_detectors, detectors_results, @@ -352,7 +340,7 @@ class PrinterHumanSummary(AbstractPrinter): libs = self._standard_libraries() if libs: txt += f'\nUse: {", ".join(libs)}\n' - results["standard_libraries"] = [str(l) for l in libs] + results["standard_libraries"] = [str(lib) for lib in libs] ercs = self._ercs() if ercs: @@ -363,7 +351,6 @@ class PrinterHumanSummary(AbstractPrinter): ["Name", "# functions", "ERCS", "ERC20 info", "Complex code", "Features"] ) for contract in self.slither.contracts_derived: - if contract.is_from_dependency() or contract.is_test: continue diff --git a/slither/printers/summary/loc.py b/slither/printers/summary/loc.py new file mode 100644 index 000000000..35bb20fc4 --- /dev/null +++ b/slither/printers/summary/loc.py @@ -0,0 +1,35 @@ +""" + Lines of Code (LOC) printer + + Definitions: + cloc: comment lines of code containing only comments + sloc: source lines of code with no whitespace or comments + loc: all lines of code including whitespace and comments + src: source files (excluding tests and dependencies) + dep: dependency files + test: test files +""" + +from slither.printers.abstract_printer import AbstractPrinter +from slither.utils.loc import compute_loc_metrics +from slither.utils.output import Output + + +class LocPrinter(AbstractPrinter): + ARGUMENT = "loc" + HELP = """Count the total number lines of code (LOC), source lines of code (SLOC), \ + and comment lines of code (CLOC) found in source files (SRC), dependencies (DEP), \ + and test files (TEST).""" + + WIKI = "https://github.com/trailofbits/slither/wiki/Printer-documentation#loc" + + def output(self, _filename: str) -> Output: + # compute loc metrics + loc = compute_loc_metrics(self.slither) + + table = loc.to_pretty_table() + txt = "Lines of Code \n" + str(table) + self.info(txt) + res = self.generate_output(txt) + res.add_pretty_table(table, "Code Lines") + return res diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 8bc9b3247..d40715c4f 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -1077,7 +1077,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call, return op if isinstance(ins.ori, TmpNewArray): - n = NewArray(ins.ori.depth, ins.ori.array_type, ins.lvalue) + n = NewArray(ins.ori.array_type, ins.lvalue) n.set_expression(ins.expression) return n @@ -1363,11 +1363,12 @@ def convert_to_pop(ir: HighLevelCall, node: "Node") -> List[Operation]: # TODO the following is equivalent to length.points_to = arr # Should it be removed? ir_length.lvalue.points_to = arr - # Note bytes is an ElementaryType not ArrayType so in that case we use ir.destination.type + # Note bytes is an ElementaryType not ArrayType and bytes1 should be returned + # since bytes is bytes1[] without padding between the elements # while in other cases such as uint256[] (ArrayType) we use ir.destination.type.type # in this way we will have the type always set to the corresponding ElementaryType element_to_delete.set_type( - ir.destination.type + ElementaryType("bytes1") if isinstance(ir.destination.type, ElementaryType) else ir.destination.type.type ) @@ -1583,7 +1584,9 @@ def _convert_to_structure_to_list(return_type: Type) -> List[Type]: # } if isinstance(return_type, (MappingType, ArrayType)): return [] - return [return_type.type] + + assert isinstance(return_type, (ElementaryType, UserDefinedType, TypeAlias)) + return [return_type] def convert_type_of_high_and_internal_level_call( diff --git a/slither/slithir/operations/index.py b/slither/slithir/operations/index.py index f38a25927..4fcfb8a6d 100644 --- a/slither/slithir/operations/index.py +++ b/slither/slithir/operations/index.py @@ -3,6 +3,7 @@ from typing import List, Union from slither.core.declarations import SolidityVariableComposed from slither.core.source_mapping.source_mapping import SourceMapping from slither.core.variables.variable import Variable +from slither.core.variables.top_level_variable import TopLevelVariable from slither.slithir.operations.lvalue import OperationWithLValue from slither.slithir.utils.utils import is_valid_lvalue, is_valid_rvalue, RVALUE, LVALUE from slither.slithir.variables.reference import ReferenceVariable @@ -13,8 +14,10 @@ class Index(OperationWithLValue): self, result: ReferenceVariable, left_variable: Variable, right_variable: RVALUE ) -> None: super().__init__() - assert is_valid_lvalue(left_variable) or left_variable == SolidityVariableComposed( - "msg.data" + assert ( + is_valid_lvalue(left_variable) + or left_variable == SolidityVariableComposed("msg.data") + or isinstance(left_variable, TopLevelVariable) ) assert is_valid_rvalue(right_variable) assert isinstance(result, ReferenceVariable) diff --git a/slither/slithir/operations/init_array.py b/slither/slithir/operations/init_array.py index 4f6b2f9fa..e0754c770 100644 --- a/slither/slithir/operations/init_array.py +++ b/slither/slithir/operations/init_array.py @@ -41,7 +41,7 @@ class InitArray(OperationWithLValue): def convert(elem): if isinstance(elem, (list,)): return str([convert(x) for x in elem]) - return str(elem) + return f"{elem}({elem.type})" init_values = convert(self.init_values) return f"{self.lvalue}({self.lvalue.type}) = {init_values}" diff --git a/slither/slithir/operations/new_array.py b/slither/slithir/operations/new_array.py index 34e8f99f7..39b989459 100644 --- a/slither/slithir/operations/new_array.py +++ b/slither/slithir/operations/new_array.py @@ -1,10 +1,10 @@ from typing import List, Union, TYPE_CHECKING -from slither.slithir.operations.lvalue import OperationWithLValue + +from slither.core.solidity_types.array_type import ArrayType from slither.slithir.operations.call import Call -from slither.core.solidity_types.type import Type +from slither.slithir.operations.lvalue import OperationWithLValue if TYPE_CHECKING: - from slither.core.solidity_types.type_alias import TypeAliasTopLevel from slither.slithir.variables.constant import Constant from slither.slithir.variables.temporary import TemporaryVariable from slither.slithir.variables.temporary_ssa import TemporaryVariableSSA @@ -13,32 +13,24 @@ if TYPE_CHECKING: class NewArray(Call, OperationWithLValue): def __init__( self, - depth: int, - array_type: "TypeAliasTopLevel", + array_type: "ArrayType", lvalue: Union["TemporaryVariableSSA", "TemporaryVariable"], ) -> None: super().__init__() - assert isinstance(array_type, Type) - self._depth = depth + assert isinstance(array_type, ArrayType) self._array_type = array_type self._lvalue = lvalue @property - def array_type(self) -> "TypeAliasTopLevel": + def array_type(self) -> "ArrayType": return self._array_type @property def read(self) -> List["Constant"]: return self._unroll(self.arguments) - @property - def depth(self) -> int: - return self._depth - def __str__(self): args = [str(a) for a in self.arguments] lvalue = self.lvalue - return ( - f"{lvalue}({lvalue.type}) = new {self.array_type}{'[]' * self.depth}({','.join(args)})" - ) + return f"{lvalue}{lvalue.type}) = new {self.array_type}({','.join(args)})" diff --git a/slither/slithir/tmp_operations/tmp_new_array.py b/slither/slithir/tmp_operations/tmp_new_array.py index efbdb6242..04acb4b9e 100644 --- a/slither/slithir/tmp_operations/tmp_new_array.py +++ b/slither/slithir/tmp_operations/tmp_new_array.py @@ -6,13 +6,11 @@ from slither.slithir.variables.temporary import TemporaryVariable class TmpNewArray(OperationWithLValue): def __init__( self, - depth: int, array_type: Type, lvalue: TemporaryVariable, ) -> None: super().__init__() assert isinstance(array_type, Type) - self._depth = depth self._array_type = array_type self._lvalue = lvalue @@ -24,9 +22,5 @@ class TmpNewArray(OperationWithLValue): def read(self): return [] - @property - def depth(self) -> int: - return self._depth - def __str__(self): - return f"{self.lvalue} = new {self.array_type}{'[]' * self._depth}" + return f"{self.lvalue} = new {self.array_type}" diff --git a/slither/slithir/utils/ssa.py b/slither/slithir/utils/ssa.py index 9a180d14f..4c958798b 100644 --- a/slither/slithir/utils/ssa.py +++ b/slither/slithir/utils/ssa.py @@ -789,10 +789,9 @@ def copy_ir(ir: Operation, *instances) -> Operation: variable_right = get_variable(ir, lambda x: x.variable_right, *instances) return Member(variable_left, variable_right, lvalue) if isinstance(ir, NewArray): - depth = ir.depth array_type = ir.array_type lvalue = get_variable(ir, lambda x: x.lvalue, *instances) - new_ir = NewArray(depth, array_type, lvalue) + new_ir = NewArray(array_type, lvalue) new_ir.arguments = get_rec_values(ir, lambda x: x.arguments, *instances) return new_ir if isinstance(ir, NewElementaryType): diff --git a/slither/slithir/variables/reference.py b/slither/slithir/variables/reference.py index 9ab51be65..2f99d322e 100644 --- a/slither/slithir/variables/reference.py +++ b/slither/slithir/variables/reference.py @@ -2,6 +2,7 @@ from typing import Optional, TYPE_CHECKING from slither.core.declarations import Contract, Enum, SolidityVariable, Function from slither.core.variables.variable import Variable +from slither.core.variables.top_level_variable import TopLevelVariable if TYPE_CHECKING: from slither.core.cfg.node import Node @@ -46,7 +47,7 @@ class ReferenceVariable(Variable): from slither.slithir.utils.utils import is_valid_lvalue assert is_valid_lvalue(points_to) or isinstance( - points_to, (SolidityVariable, Contract, Enum) + points_to, (SolidityVariable, Contract, Enum, TopLevelVariable) ) self._points_to = points_to diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index 5a29a9b50..35ca51aeb 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -354,7 +354,7 @@ class FunctionSolc(CallerContextExpression): ################################################################################### ################################################################################### - def _parse_if(self, if_statement: Dict, node: NodeSolc) -> NodeSolc: + def _parse_if(self, if_statement: Dict, node: NodeSolc, scope: Scope) -> NodeSolc: # IfStatement = 'if' '(' Expression ')' Statement ( 'else' Statement )? falseStatement = None @@ -362,21 +362,15 @@ class FunctionSolc(CallerContextExpression): condition = if_statement["condition"] # Note: check if the expression could be directly # parsed here - condition_node = self._new_node( - NodeType.IF, condition["src"], node.underlying_node.scope - ) + condition_node = self._new_node(NodeType.IF, condition["src"], scope) condition_node.add_unparsed_expression(condition) link_underlying_nodes(node, condition_node) - true_scope = Scope( - node.underlying_node.scope.is_checked, False, node.underlying_node.scope - ) + true_scope = Scope(scope.is_checked, False, scope) trueStatement = self._parse_statement( if_statement["trueBody"], condition_node, true_scope ) if "falseBody" in if_statement and if_statement["falseBody"]: - false_scope = Scope( - node.underlying_node.scope.is_checked, False, node.underlying_node.scope - ) + false_scope = Scope(scope.is_checked, False, scope) falseStatement = self._parse_statement( if_statement["falseBody"], condition_node, false_scope ) @@ -385,22 +379,16 @@ class FunctionSolc(CallerContextExpression): condition = children[0] # Note: check if the expression could be directly # parsed here - condition_node = self._new_node( - NodeType.IF, condition["src"], node.underlying_node.scope - ) + condition_node = self._new_node(NodeType.IF, condition["src"], scope) condition_node.add_unparsed_expression(condition) link_underlying_nodes(node, condition_node) - true_scope = Scope( - node.underlying_node.scope.is_checked, False, node.underlying_node.scope - ) + true_scope = Scope(scope.is_checked, False, scope) trueStatement = self._parse_statement(children[1], condition_node, true_scope) if len(children) == 3: - false_scope = Scope( - node.underlying_node.scope.is_checked, False, node.underlying_node.scope - ) + false_scope = Scope(scope.is_checked, False, scope) falseStatement = self._parse_statement(children[2], condition_node, false_scope) - endIf_node = self._new_node(NodeType.ENDIF, if_statement["src"], node.underlying_node.scope) + endIf_node = self._new_node(NodeType.ENDIF, if_statement["src"], scope) link_underlying_nodes(trueStatement, endIf_node) if falseStatement: @@ -409,32 +397,26 @@ class FunctionSolc(CallerContextExpression): link_underlying_nodes(condition_node, endIf_node) return endIf_node - def _parse_while(self, whilte_statement: Dict, node: NodeSolc) -> NodeSolc: + def _parse_while(self, whilte_statement: Dict, node: NodeSolc, scope: Scope) -> NodeSolc: # WhileStatement = 'while' '(' Expression ')' Statement - node_startWhile = self._new_node( - NodeType.STARTLOOP, whilte_statement["src"], node.underlying_node.scope - ) + node_startWhile = self._new_node(NodeType.STARTLOOP, whilte_statement["src"], scope) - body_scope = Scope(node.underlying_node.scope.is_checked, False, node.underlying_node.scope) + body_scope = Scope(scope.is_checked, False, scope) if self.is_compact_ast: node_condition = self._new_node( - NodeType.IFLOOP, whilte_statement["condition"]["src"], node.underlying_node.scope + NodeType.IFLOOP, whilte_statement["condition"]["src"], scope ) node_condition.add_unparsed_expression(whilte_statement["condition"]) statement = self._parse_statement(whilte_statement["body"], node_condition, body_scope) else: children = whilte_statement[self.get_children("children")] expression = children[0] - node_condition = self._new_node( - NodeType.IFLOOP, expression["src"], node.underlying_node.scope - ) + node_condition = self._new_node(NodeType.IFLOOP, expression["src"], scope) node_condition.add_unparsed_expression(expression) statement = self._parse_statement(children[1], node_condition, body_scope) - node_endWhile = self._new_node( - NodeType.ENDLOOP, whilte_statement["src"], node.underlying_node.scope - ) + node_endWhile = self._new_node(NodeType.ENDLOOP, whilte_statement["src"], scope) link_underlying_nodes(node, node_startWhile) link_underlying_nodes(node_startWhile, node_condition) @@ -562,7 +544,7 @@ class FunctionSolc(CallerContextExpression): return pre, cond, post, body - def _parse_for(self, statement: Dict, node: NodeSolc) -> NodeSolc: + def _parse_for(self, statement: Dict, node: NodeSolc, scope: Scope) -> NodeSolc: # ForStatement = 'for' '(' (SimpleStatement)? ';' (Expression)? ';' (ExpressionStatement)? ')' Statement if self.is_compact_ast: @@ -570,17 +552,13 @@ class FunctionSolc(CallerContextExpression): else: pre, cond, post, body = self._parse_for_legacy_ast(statement) - node_startLoop = self._new_node( - NodeType.STARTLOOP, statement["src"], node.underlying_node.scope - ) - node_endLoop = self._new_node( - NodeType.ENDLOOP, statement["src"], node.underlying_node.scope - ) + node_startLoop = self._new_node(NodeType.STARTLOOP, statement["src"], scope) + node_endLoop = self._new_node(NodeType.ENDLOOP, statement["src"], scope) - last_scope = node.underlying_node.scope + last_scope = scope if pre: - pre_scope = Scope(node.underlying_node.scope.is_checked, False, last_scope) + pre_scope = Scope(scope.is_checked, False, last_scope) last_scope = pre_scope node_init_expression = self._parse_statement(pre, node, pre_scope) link_underlying_nodes(node_init_expression, node_startLoop) @@ -588,7 +566,7 @@ class FunctionSolc(CallerContextExpression): link_underlying_nodes(node, node_startLoop) if cond: - cond_scope = Scope(node.underlying_node.scope.is_checked, False, last_scope) + cond_scope = Scope(scope.is_checked, False, last_scope) last_scope = cond_scope node_condition = self._new_node(NodeType.IFLOOP, cond["src"], cond_scope) node_condition.add_unparsed_expression(cond) @@ -599,7 +577,7 @@ class FunctionSolc(CallerContextExpression): node_condition = None node_beforeBody = node_startLoop - body_scope = Scope(node.underlying_node.scope.is_checked, False, last_scope) + body_scope = Scope(scope.is_checked, False, last_scope) last_scope = body_scope node_body = self._parse_statement(body, node_beforeBody, body_scope) @@ -619,14 +597,10 @@ class FunctionSolc(CallerContextExpression): return node_endLoop - def _parse_dowhile(self, do_while_statement: Dict, node: NodeSolc) -> NodeSolc: + def _parse_dowhile(self, do_while_statement: Dict, node: NodeSolc, scope: Scope) -> NodeSolc: - node_startDoWhile = self._new_node( - NodeType.STARTLOOP, do_while_statement["src"], node.underlying_node.scope - ) - condition_scope = Scope( - node.underlying_node.scope.is_checked, False, node.underlying_node.scope - ) + node_startDoWhile = self._new_node(NodeType.STARTLOOP, do_while_statement["src"], scope) + condition_scope = Scope(scope.is_checked, False, scope) if self.is_compact_ast: node_condition = self._new_node( @@ -644,7 +618,7 @@ class FunctionSolc(CallerContextExpression): node_condition.add_unparsed_expression(expression) statement = self._parse_statement(children[1], node_condition, condition_scope) - body_scope = Scope(node.underlying_node.scope.is_checked, False, condition_scope) + body_scope = Scope(scope.is_checked, False, condition_scope) node_endDoWhile = self._new_node(NodeType.ENDLOOP, do_while_statement["src"], body_scope) link_underlying_nodes(node, node_startDoWhile) @@ -660,46 +634,124 @@ class FunctionSolc(CallerContextExpression): link_underlying_nodes(node_condition, node_endDoWhile) return node_endDoWhile - def _parse_try_catch(self, statement: Dict, node: NodeSolc) -> NodeSolc: + # pylint: disable=no-self-use + def _construct_try_expression(self, externalCall: Dict, parameters_list: Dict) -> Dict: + # if the parameters are more than 1 we make the leftHandSide of the Assignment node + # a TupleExpression otherwise an Identifier + + # case when there isn't returns(...) + # e.g. external call that doesn't have any return variable + if not parameters_list: + return externalCall + + ret: Dict = {"nodeType": "Assignment", "operator": "=", "src": parameters_list["src"]} + + parameters = parameters_list.get("parameters", None) + + # if the name is "" it means the return variable is not used + if len(parameters) == 1: + if parameters[0]["name"] != "": + self._add_param(parameters[0]) + ret["typeDescriptions"] = { + "typeString": parameters[0]["typeName"]["typeDescriptions"]["typeString"] + } + leftHandSide = { + "name": parameters[0]["name"], + "nodeType": "Identifier", + "src": parameters[0]["src"], + "referencedDeclaration": parameters[0]["id"], + "typeDescriptions": parameters[0]["typeDescriptions"], + } + else: + # we don't need an Assignment so we return only the external call + return externalCall + else: + ret["typeDescriptions"] = {"typeString": "tuple()"} + leftHandSide = { + "components": [], + "nodeType": "TupleExpression", + "src": parameters_list["src"], + } + + for i, p in enumerate(parameters): + if p["name"] == "": + continue + + new_statement = { + "nodeType": "VariableDefinitionStatement", + "src": p["src"], + "declarations": [p], + } + self._add_param_init_tuple(new_statement, i) + + ident = { + "name": p["name"], + "nodeType": "Identifier", + "src": p["src"], + "referencedDeclaration": p["id"], + "typeDescriptions": p["typeDescriptions"], + } + leftHandSide["components"].append(ident) + + ret["leftHandSide"] = leftHandSide + ret["rightHandSide"] = externalCall + + return ret + + def _parse_try_catch(self, statement: Dict, node: NodeSolc, scope: Scope) -> NodeSolc: externalCall = statement.get("externalCall", None) if externalCall is None: raise ParsingError(f"Try/Catch not correctly parsed by Slither {statement}") - catch_scope = Scope( - node.underlying_node.scope.is_checked, False, node.underlying_node.scope - ) + catch_scope = Scope(scope.is_checked, False, scope) new_node = self._new_node(NodeType.TRY, statement["src"], catch_scope) - new_node.add_unparsed_expression(externalCall) + clauses = statement.get("clauses", []) + # the first clause is the try scope + returned_variables = clauses[0].get("parameters", None) + constructed_try_expression = self._construct_try_expression( + externalCall, returned_variables + ) + new_node.add_unparsed_expression(constructed_try_expression) link_underlying_nodes(node, new_node) node = new_node - for clause in statement.get("clauses", []): - self._parse_catch(clause, node) + for index, clause in enumerate(clauses): + # clauses after the first one are related to catch cases + # we set the parameters (e.g. data in this case. catch(string memory data) ...) + # to be initialized so they are not reported by the uninitialized-local-variables detector + if index >= 1: + self._parse_catch(clause, node, catch_scope, True) + else: + # the parameters for the try scope were already added in _construct_try_expression + self._parse_catch(clause, node, catch_scope, False) return node - def _parse_catch(self, statement: Dict, node: NodeSolc) -> NodeSolc: + def _parse_catch( + self, statement: Dict, node: NodeSolc, scope: Scope, add_param: bool + ) -> NodeSolc: block = statement.get("block", None) if block is None: raise ParsingError(f"Catch not correctly parsed by Slither {statement}") - try_scope = Scope(node.underlying_node.scope.is_checked, False, node.underlying_node.scope) + try_scope = Scope(scope.is_checked, False, scope) try_node = self._new_node(NodeType.CATCH, statement["src"], try_scope) link_underlying_nodes(node, try_node) - if self.is_compact_ast: - params = statement.get("parameters", None) - else: - params = statement[self.get_children("children")] + if add_param: + if self.is_compact_ast: + params = statement.get("parameters", None) + else: + params = statement[self.get_children("children")] - if params: - for param in params.get("parameters", []): - assert param[self.get_key()] == "VariableDeclaration" - self._add_param(param) + if params: + for param in params.get("parameters", []): + assert param[self.get_key()] == "VariableDeclaration" + self._add_param(param, True) return self._parse_statement(block, try_node, try_scope) - def _parse_variable_definition(self, statement: Dict, node: NodeSolc) -> NodeSolc: + def _parse_variable_definition(self, statement: Dict, node: NodeSolc, scope: Scope) -> NodeSolc: try: local_var = LocalVariable() local_var.set_function(self._function) @@ -709,9 +761,7 @@ class FunctionSolc(CallerContextExpression): self._add_local_variable(local_var_parser) # local_var.analyze(self) - new_node = self._new_node( - NodeType.VARIABLE, statement["src"], node.underlying_node.scope - ) + new_node = self._new_node(NodeType.VARIABLE, statement["src"], scope) new_node.underlying_node.add_variable_declaration(local_var) link_underlying_nodes(node, new_node) return new_node @@ -741,7 +791,7 @@ class FunctionSolc(CallerContextExpression): "declarations": [variable], "initialValue": init, } - new_node = self._parse_variable_definition(new_statement, new_node) + new_node = self._parse_variable_definition(new_statement, new_node, scope) else: # If we have @@ -763,7 +813,7 @@ class FunctionSolc(CallerContextExpression): variables.append(variable) new_node = self._parse_variable_definition_init_tuple( - new_statement, i, new_node + new_statement, i, new_node, scope ) i = i + 1 @@ -795,9 +845,7 @@ class FunctionSolc(CallerContextExpression): "typeDescriptions": {"typeString": "tuple()"}, } node = new_node - new_node = self._new_node( - NodeType.EXPRESSION, statement["src"], node.underlying_node.scope - ) + new_node = self._new_node(NodeType.EXPRESSION, statement["src"], scope) new_node.add_unparsed_expression(expression) link_underlying_nodes(node, new_node) @@ -828,7 +876,7 @@ class FunctionSolc(CallerContextExpression): self.get_children("children"): [variable, init], } - new_node = self._parse_variable_definition(new_statement, new_node) + new_node = self._parse_variable_definition(new_statement, new_node, scope) else: # If we have # var (a, b) = f() @@ -847,7 +895,7 @@ class FunctionSolc(CallerContextExpression): variables.append(variable) new_node = self._parse_variable_definition_init_tuple( - new_statement, i, new_node + new_statement, i, new_node, scope ) i = i + 1 var_identifiers = [] @@ -877,16 +925,14 @@ class FunctionSolc(CallerContextExpression): ], } node = new_node - new_node = self._new_node( - NodeType.EXPRESSION, statement["src"], node.underlying_node.scope - ) + new_node = self._new_node(NodeType.EXPRESSION, statement["src"], scope) new_node.add_unparsed_expression(expression) link_underlying_nodes(node, new_node) return new_node def _parse_variable_definition_init_tuple( - self, statement: Dict, index: int, node: NodeSolc + self, statement: Dict, index: int, node: NodeSolc, scope ) -> NodeSolc: local_var = LocalVariableInitFromTuple() local_var.set_function(self._function) @@ -896,7 +942,7 @@ class FunctionSolc(CallerContextExpression): self._add_local_variable(local_var_parser) - new_node = self._new_node(NodeType.VARIABLE, statement["src"], node.underlying_node.scope) + new_node = self._new_node(NodeType.VARIABLE, statement["src"], scope) new_node.underlying_node.add_variable_declaration(local_var) link_underlying_nodes(node, new_node) return new_node @@ -917,15 +963,15 @@ class FunctionSolc(CallerContextExpression): name = statement[self.get_key()] # SimpleStatement = VariableDefinition | ExpressionStatement if name == "IfStatement": - node = self._parse_if(statement, node) + node = self._parse_if(statement, node, scope) elif name == "WhileStatement": - node = self._parse_while(statement, node) + node = self._parse_while(statement, node, scope) elif name == "ForStatement": - node = self._parse_for(statement, node) + node = self._parse_for(statement, node, scope) elif name == "Block": - node = self._parse_block(statement, node) + node = self._parse_block(statement, node, scope) elif name == "UncheckedBlock": - node = self._parse_unchecked_block(statement, node) + node = self._parse_unchecked_block(statement, node, scope) elif name == "InlineAssembly": # Added with solc 0.6 - the yul code is an AST if "AST" in statement and not self.compilation_unit.core.skip_assembly: @@ -947,7 +993,7 @@ class FunctionSolc(CallerContextExpression): link_underlying_nodes(node, asm_node) node = asm_node elif name == "DoWhileStatement": - node = self._parse_dowhile(statement, node) + node = self._parse_dowhile(statement, node, scope) # For Continue / Break / Return / Throw # The is fixed later elif name == "Continue": @@ -988,7 +1034,7 @@ class FunctionSolc(CallerContextExpression): link_underlying_nodes(node, new_node) node = new_node elif name in ["VariableDefinitionStatement", "VariableDeclarationStatement"]: - node = self._parse_variable_definition(statement, node) + node = self._parse_variable_definition(statement, node, scope) elif name == "ExpressionStatement": # assert len(statement[self.get_children('expression')]) == 1 # assert not 'attributes' in statement @@ -1002,7 +1048,7 @@ class FunctionSolc(CallerContextExpression): link_underlying_nodes(node, new_node) node = new_node elif name == "TryStatement": - node = self._parse_try_catch(statement, node) + node = self._parse_try_catch(statement, node, scope) # elif name == 'TryCatchClause': # self._parse_catch(statement, node) elif name == "RevertStatement": @@ -1019,7 +1065,7 @@ class FunctionSolc(CallerContextExpression): return node - def _parse_block(self, block: Dict, node: NodeSolc, check_arithmetic: bool = False) -> NodeSolc: + def _parse_block(self, block: Dict, node: NodeSolc, scope: Scope) -> NodeSolc: """ Return: Node @@ -1031,13 +1077,12 @@ class FunctionSolc(CallerContextExpression): else: statements = block[self.get_children("children")] - check_arithmetic = check_arithmetic | node.underlying_node.scope.is_checked - new_scope = Scope(check_arithmetic, False, node.underlying_node.scope) + new_scope = Scope(scope.is_checked, False, scope) for statement in statements: node = self._parse_statement(statement, node, new_scope) return node - def _parse_unchecked_block(self, block: Dict, node: NodeSolc): + def _parse_unchecked_block(self, block: Dict, node: NodeSolc, scope): """ Return: Node @@ -1049,7 +1094,8 @@ class FunctionSolc(CallerContextExpression): else: statements = block[self.get_children("children")] - new_scope = Scope(False, False, node.underlying_node.scope) + new_scope = Scope(False, False, scope) + for statement in statements: node = self._parse_statement(statement, node, new_scope) return node @@ -1070,8 +1116,7 @@ class FunctionSolc(CallerContextExpression): self._function.is_empty = True else: self._function.is_empty = False - check_arithmetic = self.compilation_unit.solc_version >= "0.8.0" - self._parse_block(cfg, node, check_arithmetic=check_arithmetic) + self._parse_block(cfg, node, self.underlying_function) self._remove_incorrect_edges() self._remove_alone_endif() @@ -1162,7 +1207,7 @@ class FunctionSolc(CallerContextExpression): visited.add(son) self._fix_catch(son, end_node, visited) - def _add_param(self, param: Dict) -> LocalVariableSolc: + def _add_param(self, param: Dict, initialized: bool = False) -> LocalVariableSolc: local_var = LocalVariable() local_var.set_function(self._function) @@ -1172,6 +1217,9 @@ class FunctionSolc(CallerContextExpression): local_var_parser.analyze(self) + if initialized: + local_var.initialized = True + # see https://solidity.readthedocs.io/en/v0.4.24/types.html?highlight=storage%20location#data-location if local_var.location == "default": local_var.set_location("memory") @@ -1179,6 +1227,17 @@ class FunctionSolc(CallerContextExpression): self._add_local_variable(local_var_parser) return local_var_parser + def _add_param_init_tuple(self, statement: Dict, index: int) -> LocalVariableInitFromTupleSolc: + + local_var = LocalVariableInitFromTuple() + local_var.set_function(self._function) + local_var.set_offset(statement["src"], self._function.compilation_unit) + + local_var_parser = LocalVariableInitFromTupleSolc(local_var, statement, index) + + self._add_local_variable(local_var_parser) + return local_var_parser + def _parse_params(self, params: Dict): assert params[self.get_key()] == "ParameterList" diff --git a/slither/solc_parsing/declarations/using_for_top_level.py b/slither/solc_parsing/declarations/using_for_top_level.py index 707ad83ac..fe72e5780 100644 --- a/slither/solc_parsing/declarations/using_for_top_level.py +++ b/slither/solc_parsing/declarations/using_for_top_level.py @@ -55,22 +55,29 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- self._propagate_global(type_name) else: for f in self._functions: - full_name_split = f["function"]["name"].split(".") - if len(full_name_split) == 1: + # User defined operator + if "operator" in f: # Top level function - function_name: str = full_name_split[0] - self._analyze_top_level_function(function_name, type_name) - elif len(full_name_split) == 2: - # It can be a top level function behind an aliased import - # or a library function - first_part = full_name_split[0] - function_name = full_name_split[1] - self._check_aliased_import(first_part, function_name, type_name) + function_name: str = f["definition"]["name"] + operator: str = f["operator"] + self._analyze_operator(operator, function_name, type_name) else: - # MyImport.MyLib.a we don't care of the alias - library_name_str = full_name_split[1] - function_name = full_name_split[2] - self._analyze_library_function(library_name_str, function_name, type_name) + full_name_split = f["function"]["name"].split(".") + if len(full_name_split) == 1: + # Top level function + function_name: str = full_name_split[0] + self._analyze_top_level_function(function_name, type_name) + elif len(full_name_split) == 2: + # It can be a top level function behind an aliased import + # or a library function + first_part = full_name_split[0] + function_name = full_name_split[1] + self._check_aliased_import(first_part, function_name, type_name) + else: + # MyImport.MyLib.a we don't care of the alias + library_name_str = full_name_split[1] + function_name = full_name_split[2] + self._analyze_library_function(library_name_str, function_name, type_name) def _check_aliased_import( self, @@ -101,6 +108,19 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- self._propagate_global(type_name) break + def _analyze_operator( + self, operator: str, function_name: str, type_name: TypeAliasTopLevel + ) -> None: + for tl_function in self._using_for.file_scope.functions: + # The library function is bound to the first parameter's type + if ( + tl_function.name == function_name + and tl_function.parameters + and type_name == tl_function.parameters[0].type + ): + type_name.operators[operator] = tl_function + break + def _analyze_library_function( self, library_name: str, diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index d0dc4c7e0..a0bce044c 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -1,6 +1,6 @@ import logging import re -from typing import Union, Dict, TYPE_CHECKING +from typing import Union, Dict, TYPE_CHECKING, List, Any import slither.core.expressions.type_conversion from slither.core.declarations.solidity_variables import ( @@ -236,6 +236,24 @@ if TYPE_CHECKING: pass +def _user_defined_op_call( + caller_context: CallerContextExpression, src, function_id: int, args: List[Any], type_call: str +) -> CallExpression: + var, was_created = find_variable(None, caller_context, function_id) + + if was_created: + var.set_offset(src, caller_context.compilation_unit) + + identifier = Identifier(var) + identifier.set_offset(src, caller_context.compilation_unit) + + var.references.append(identifier.source_mapping) + + call = CallExpression(identifier, args, type_call) + call.set_offset(src, caller_context.compilation_unit) + return call + + def parse_expression(expression: Dict, caller_context: CallerContextExpression) -> "Expression": # pylint: disable=too-many-nested-blocks,too-many-statements """ @@ -274,16 +292,24 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression) if name == "UnaryOperation": if is_compact_ast: attributes = expression - else: - attributes = expression["attributes"] - assert "prefix" in attributes - operation_type = UnaryOperationType.get_type(attributes["operator"], attributes["prefix"]) - - if is_compact_ast: expression = parse_expression(expression["subExpression"], caller_context) else: + attributes = expression["attributes"] assert len(expression["children"]) == 1 expression = parse_expression(expression["children"][0], caller_context) + assert "prefix" in attributes + + # Use of user defined operation + if "function" in attributes: + return _user_defined_op_call( + caller_context, + src, + attributes["function"], + [expression], + attributes["typeDescriptions"]["typeString"], + ) + + operation_type = UnaryOperationType.get_type(attributes["operator"], attributes["prefix"]) unary_op = UnaryOperation(expression, operation_type) unary_op.set_offset(src, caller_context.compilation_unit) return unary_op @@ -291,17 +317,25 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression) if name == "BinaryOperation": if is_compact_ast: attributes = expression - else: - attributes = expression["attributes"] - operation_type = BinaryOperationType.get_type(attributes["operator"]) - - if is_compact_ast: left_expression = parse_expression(expression["leftExpression"], caller_context) right_expression = parse_expression(expression["rightExpression"], caller_context) else: assert len(expression["children"]) == 2 + attributes = expression["attributes"] left_expression = parse_expression(expression["children"][0], caller_context) right_expression = parse_expression(expression["children"][1], caller_context) + + # Use of user defined operation + if "function" in attributes: + return _user_defined_op_call( + caller_context, + src, + attributes["function"], + [left_expression, right_expression], + attributes["typeDescriptions"]["typeString"], + ) + + operation_type = BinaryOperationType.get_type(attributes["operator"]) binary_op = BinaryOperation(left_expression, right_expression, operation_type) binary_op.set_offset(src, caller_context.compilation_unit) return binary_op @@ -433,6 +467,8 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression) type_candidate = ElementaryType("uint256") else: type_candidate = ElementaryType("string") + elif type_candidate.startswith("rational_const "): + type_candidate = ElementaryType("uint256") elif type_candidate.startswith("int_const "): type_candidate = ElementaryType("uint256") elif type_candidate.startswith("bool"): @@ -559,37 +595,9 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression) type_name = children[0] if type_name[caller_context.get_key()] == "ArrayTypeName": - depth = 0 - while type_name[caller_context.get_key()] == "ArrayTypeName": - # Note: dont conserve the size of the array if provided - # We compute it directly - if is_compact_ast: - type_name = type_name["baseType"] - else: - type_name = type_name["children"][0] - depth += 1 - if type_name[caller_context.get_key()] == "ElementaryTypeName": - if is_compact_ast: - array_type = ElementaryType(type_name["name"]) - else: - array_type = ElementaryType(type_name["attributes"]["name"]) - elif type_name[caller_context.get_key()] == "UserDefinedTypeName": - if is_compact_ast: - if "name" not in type_name: - name_type = type_name["pathNode"]["name"] - else: - name_type = type_name["name"] - - array_type = parse_type(UnknownType(name_type), caller_context) - else: - array_type = parse_type( - UnknownType(type_name["attributes"]["name"]), caller_context - ) - elif type_name[caller_context.get_key()] == "FunctionTypeName": - array_type = parse_type(type_name, caller_context) - else: - raise ParsingError(f"Incorrect type array {type_name}") - array = NewArray(depth, array_type) + array_type = parse_type(type_name, caller_context) + assert isinstance(array_type, ArrayType) + array = NewArray(array_type) array.set_offset(src, caller_context.compilation_unit) return array diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index cb94a7d9e..00ac3a519 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -33,6 +33,10 @@ logger = logging.getLogger("SlitherSolcParsing") logger.setLevel(logging.INFO) +class InheritanceResolutionError(SlitherException): + pass + + def _handle_import_aliases( symbol_aliases: Dict, import_directive: Import, scope: FileScope ) -> None: @@ -194,6 +198,13 @@ class SlitherCompilationUnitSolc(CallerContextExpression): # pylint: disable=too-many-branches,too-many-statements,too-many-locals def parse_top_level_from_loaded_json(self, data_loaded: Dict, filename: str) -> None: + if not data_loaded or data_loaded is None: + logger.error( + "crytic-compile returned an empty AST. " + "If you are trying to analyze a contract from etherscan or similar make sure it has source code available." + ) + return + if "nodeType" in data_loaded: self._is_compact_ast = True @@ -432,7 +443,12 @@ Please rename it, this name is reserved for Slither's internals""" target = contract_parser.underlying_contract.file_scope.get_contract_from_name( contract_name ) - assert target + if target == contract_parser.underlying_contract: + raise InheritanceResolutionError( + "Could not resolve contract inheritance. This is likely caused by an import renaming that collides with existing names (see https://github.com/crytic/slither/issues/1758)." + f"\n Try changing `contract {target}` ({target.source_mapping}) to a unique name." + ) + assert target, f"Contract {contract_name} not found" ancestors.append(target) elif i in self._contracts_by_id: ancestors.append(self._contracts_by_id[i]) @@ -742,12 +758,46 @@ Please rename it, this name is reserved for Slither's internals""" self._underlying_contract_to_parser[contract].log_incorrect_parsing( f"Impossible to generate IR for {contract.name}.{func.name} ({func.source_mapping}):\n {e}" ) - - contract.convert_expression_to_slithir_ssa() + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to generate IR for {contract.name}.{func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{contract.name}.{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e + try: + contract.convert_expression_to_slithir_ssa() + except Exception as e: + logger.error( + f"\nFailed to convert IR to SSA for {contract.name} contract. Please open an issue https://github.com/crytic/slither/issues.\n " + ) + raise e for func in self._compilation_unit.functions_top_level: - func.generate_slithir_and_analyze() - func.generate_slithir_ssa({}) + try: + func.generate_slithir_and_analyze() + except AttributeError as e: + logger.error( + f"Impossible to generate IR for top level function {func.name} ({func.source_mapping}):\n {e}" + ) + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to generate IR for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e + + try: + func.generate_slithir_ssa({}) + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to convert IR to SSA for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e + self._compilation_unit.propagate_function_calls() for contract in self._compilation_unit.contracts: contract.fix_phi() diff --git a/slither/solc_parsing/yul/evm_functions.py b/slither/solc_parsing/yul/evm_functions.py index dfb52a244..28ea70e93 100644 --- a/slither/solc_parsing/yul/evm_functions.py +++ b/slither/solc_parsing/yul/evm_functions.py @@ -51,6 +51,7 @@ evm_opcodes = [ "TIMESTAMP", "NUMBER", "DIFFICULTY", + "PREVRANDAO", "GASLIMIT", "CHAINID", "SELFBALANCE", @@ -168,6 +169,7 @@ builtins = [ ) ] + yul_funcs +# "identifier": [input_count, output_count] function_args = { "byte": [2, 1], "addmod": [3, 1], @@ -221,6 +223,7 @@ function_args = { "timestamp": [0, 1], "number": [0, 1], "difficulty": [0, 1], + "prevrandao": [0, 1], "gaslimit": [0, 1], } diff --git a/slither/solc_parsing/yul/parse_yul.py b/slither/solc_parsing/yul/parse_yul.py index 35d5cdd9d..8657947ea 100644 --- a/slither/solc_parsing/yul/parse_yul.py +++ b/slither/solc_parsing/yul/parse_yul.py @@ -181,7 +181,7 @@ class YulScope(metaclass=abc.ABCMeta): def add_yul_local_function(self, func: "YulFunction") -> None: self._yul_local_functions.append(func) - def get_yul_local_function_from_name(self, func_name: str) -> Optional["YulLocalVariable"]: + def get_yul_local_function_from_name(self, func_name: str) -> Optional["YulFunction"]: return next( (v for v in self._yul_local_functions if v.underlying.name == func_name), None, @@ -252,6 +252,10 @@ class YulFunction(YulScope): def function(self) -> Function: return self._function + @property + def root(self) -> YulScope: + return self._root + def convert_body(self) -> None: node = self.new_node(NodeType.ENTRYPOINT, self._ast["src"]) link_underlying_nodes(self._entrypoint, node) @@ -271,6 +275,9 @@ class YulFunction(YulScope): def parse_body(self) -> None: for node in self._nodes: node.analyze_expressions() + for f in self._yul_local_functions: + if f != self: + f.parse_body() def new_node(self, node_type: NodeType, src: str) -> YulNode: if self._function: @@ -325,7 +332,10 @@ class YulBlock(YulScope): return yul_node def convert(self, ast: Dict) -> YulNode: - return convert_yul(self, self._entrypoint, ast, self.node_scope) + yul_node = convert_yul(self, self._entrypoint, ast, self.node_scope) + for f in self._yul_local_functions: + f.parse_body() + return yul_node def analyze_expressions(self) -> None: for node in self._nodes: @@ -390,7 +400,6 @@ def convert_yul_function_definition( root.add_yul_local_function(yul_function) yul_function.convert_body() - yul_function.parse_body() return parent @@ -778,6 +787,7 @@ def _parse_yul_magic_suffixes(name: str, root: YulScope) -> Optional[Expression] return None +# pylint: disable=too-many-branches def parse_yul_identifier(root: YulScope, _node: YulNode, ast: Dict) -> Optional[Expression]: name = ast["name"] @@ -809,6 +819,23 @@ def parse_yul_identifier(root: YulScope, _node: YulNode, ast: Dict) -> Optional[ if func: return Identifier(func.underlying) + # check yul-block scoped function + if isinstance(root, YulFunction): + yul_block = root.root + + # Iterate until we searched in all the scopes until the YulBlock scope + while not isinstance(yul_block, YulBlock): + func = yul_block.get_yul_local_function_from_name(name) + if func: + return Identifier(func.underlying) + + if isinstance(yul_block, YulFunction): + yul_block = yul_block.root + + func = yul_block.get_yul_local_function_from_name(name) + if func: + return Identifier(func.underlying) + magic_suffix = _parse_yul_magic_suffixes(name, root) if magic_suffix: return magic_suffix diff --git a/slither/tools/read_storage/__init__.py b/slither/tools/read_storage/__init__.py index dbc1e5bc0..df9b8280d 100644 --- a/slither/tools/read_storage/__init__.py +++ b/slither/tools/read_storage/__init__.py @@ -1 +1 @@ -from .read_storage import SlitherReadStorage +from .read_storage import SlitherReadStorage, RpcInfo diff --git a/slither/tools/read_storage/__main__.py b/slither/tools/read_storage/__main__.py index 1a8901321..8415ae185 100644 --- a/slither/tools/read_storage/__main__.py +++ b/slither/tools/read_storage/__main__.py @@ -7,7 +7,7 @@ import argparse from crytic_compile import cryticparser from slither import Slither -from slither.tools.read_storage.read_storage import SlitherReadStorage +from slither.tools.read_storage.read_storage import SlitherReadStorage, RpcInfo def parse_args() -> argparse.Namespace: @@ -104,6 +104,12 @@ def parse_args() -> argparse.Namespace: default="latest", ) + parser.add_argument( + "--unstructured", + action="store_true", + help="Include unstructured storage slots", + ) + cryticparser.init(parser) return parser.parse_args() @@ -126,22 +132,20 @@ def main() -> None: else: contracts = slither.contracts - srs = SlitherReadStorage(contracts, args.max_depth) - - try: - srs.block = int(args.block) - except ValueError: - srs.block = str(args.block or "latest") - + rpc_info = None if args.rpc_url: - # Remove target prefix e.g. rinkeby:0x0 -> 0x0. - address = target[target.find(":") + 1 :] - # Default to implementation address unless a storage address is given. - if not args.storage_address: - args.storage_address = address - srs.storage_address = args.storage_address - - srs.rpc = args.rpc_url + valid = ["latest", "earliest", "pending", "safe", "finalized"] + block = args.block if args.block in valid else int(args.block) + rpc_info = RpcInfo(args.rpc_url, block) + + srs = SlitherReadStorage(contracts, args.max_depth, rpc_info) + srs.unstructured = bool(args.unstructured) + # Remove target prefix e.g. rinkeby:0x0 -> 0x0. + address = target[target.find(":") + 1 :] + # Default to implementation address unless a storage address is given. + if not args.storage_address: + args.storage_address = address + srs.storage_address = args.storage_address if args.variable_name: # Use a lambda func to only return variables that have same name as target. diff --git a/slither/tools/read_storage/read_storage.py b/slither/tools/read_storage/read_storage.py index 2947081da..8c0cf515d 100644 --- a/slither/tools/read_storage/read_storage.py +++ b/slither/tools/read_storage/read_storage.py @@ -6,15 +6,30 @@ import dataclasses from eth_abi import decode, encode from eth_typing.evm import ChecksumAddress -from eth_utils import keccak +from eth_utils import keccak, to_checksum_address from web3 import Web3 +from web3.types import BlockIdentifier +from web3.exceptions import ExtraDataLengthError +from web3.middleware import geth_poa_middleware 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.cfg.node import NodeType from slither.core.variables.state_variable import StateVariable from slither.core.variables.structure_variable import StructureVariable +from slither.core.expressions import ( + AssignmentOperation, + Literal, + Identifier, + BinaryOperation, + UnaryOperation, + TupleExpression, + TypeConversion, + CallExpression, +) from slither.utils.myprettytable import MyPrettyTable +from slither.visitors.expression.constants_folding import ConstantFolding, NotConstant from .utils import coerce_type, get_offset_value, get_storage_data @@ -42,20 +57,47 @@ class SlitherReadStorageException(Exception): pass -# pylint: disable=too-many-instance-attributes +class RpcInfo: + def __init__(self, rpc_url: str, block: BlockIdentifier = "latest") -> None: + assert isinstance(block, int) or block in [ + "latest", + "earliest", + "pending", + "safe", + "finalized", + ] + self.rpc: str = rpc_url + self._web3: Web3 = Web3(Web3.HTTPProvider(self.rpc)) + """If the RPC is for a POA network, the first call to get_block fails, so we inject geth_poa_middleware""" + try: + self._block: int = self.web3.eth.get_block(block)["number"] + except ExtraDataLengthError: + self._web3.middleware_onion.inject(geth_poa_middleware, layer=0) + self._block: int = self.web3.eth.get_block(block)["number"] + + @property + def web3(self) -> Web3: + return self._web3 + + @property + def block(self) -> int: + return self._block + + +# pylint: disable=too-many-instance-attributes,too-many-public-methods class SlitherReadStorage: - def __init__(self, contracts: List[Contract], max_depth: int) -> None: + def __init__(self, contracts: List[Contract], max_depth: int, rpc_info: RpcInfo = None) -> None: self._checksum_address: Optional[ChecksumAddress] = None self._contracts: List[Contract] = contracts self._log: str = "" self._max_depth: int = max_depth self._slot_info: Dict[str, SlotInfo] = {} self._target_variables: List[Tuple[Contract, StateVariable]] = [] - self._web3: Optional[Web3] = None - self.block: Union[str, int] = "latest" - self.rpc: Optional[str] = None + self._constant_storage_slots: List[Tuple[Contract, StateVariable]] = [] + self.rpc_info: Optional[RpcInfo] = rpc_info self.storage_address: Optional[str] = None self.table: Optional[MyPrettyTable] = None + self.unstructured: bool = False @property def contracts(self) -> List[Contract]: @@ -73,18 +115,12 @@ class SlitherReadStorage: def log(self, log: str) -> None: self._log = log - @property - def web3(self) -> Web3: - if not self._web3: - self._web3 = Web3(Web3.HTTPProvider(self.rpc)) - return self._web3 - @property def checksum_address(self) -> ChecksumAddress: if not self.storage_address: raise ValueError if not self._checksum_address: - self._checksum_address = self.web3.to_checksum_address(self.storage_address) + self._checksum_address = to_checksum_address(self.storage_address) return self._checksum_address @property @@ -92,6 +128,11 @@ class SlitherReadStorage: """Storage variables (not constant or immutable) and their associated contract.""" return self._target_variables + @property + def constant_slots(self) -> List[Tuple[Contract, StateVariable]]: + """Constant bytes32 variables and their associated contract.""" + return self._constant_storage_slots + @property def slot_info(self) -> Dict[str, SlotInfo]: """Contains the location, type, size, offset, and value of contract slots.""" @@ -111,9 +152,48 @@ class SlitherReadStorage: elif isinstance(type_, ArrayType): elems = self._all_array_slots(var, contract, type_, info.slot) tmp[var.name].elems = elems - + if self.unstructured: + tmp.update(self.get_unstructured_layout()) self._slot_info = tmp + def get_unstructured_layout(self) -> Dict[str, SlotInfo]: + tmp: Dict[str, SlotInfo] = {} + for _, var in self.constant_slots: + var_name = var.name + try: + exp = var.expression + if isinstance( + exp, + ( + BinaryOperation, + UnaryOperation, + Identifier, + TupleExpression, + TypeConversion, + CallExpression, + ), + ): + exp = ConstantFolding(exp, "bytes32").result() + if isinstance(exp, Literal): + slot = coerce_type("int", exp.value) + else: + continue + offset = 0 + type_string, size = self.find_constant_slot_storage_type(var) + if type_string: + tmp[var.name] = SlotInfo( + name=var_name, type_string=type_string, slot=slot, size=size, offset=offset + ) + self.log += ( + f"\nSlot Name: {var_name}\nType: bytes32" + f"\nStorage Type: {type_string}\nSlot: {str(exp)}\n" + ) + logger.info(self.log) + self.log = "" + except NotConstant: + continue + return tmp + # TODO: remove this pylint exception (montyly) # pylint: disable=too-many-locals def get_storage_slot( @@ -122,7 +202,8 @@ class SlitherReadStorage: contract: Contract, **kwargs: Any, ) -> Union[SlotInfo, None]: - """Finds the storage slot of a variable in a given contract. + """ + Finds the storage slot of a variable in a given contract. Args: target_variable (`StateVariable`): The variable to retrieve the slot for. contracts (`Contract`): The contract that contains the given state variable. @@ -208,6 +289,78 @@ class SlitherReadStorage: if slot_info: self._slot_info[f"{contract.name}.{var.name}"] = slot_info + def find_constant_slot_storage_type( + self, var: StateVariable + ) -> Tuple[Optional[str], Optional[int]]: + """ + Given a constant bytes32 StateVariable, tries to determine which variable type is stored there, using the + heuristic that if a function reads from the slot and returns a value, it probably stores that type of value. + Also uses the StorageSlot library as a heuristic when a function has no return but uses the library's getters. + Args: + var (StateVariable): The constant bytes32 storage slot. + + Returns: + type (str): The type of value stored in the slot. + size (int): The type's size in bits. + """ + assert var.is_constant and var.type == ElementaryType("bytes32") + storage_type = None + size = None + funcs = [] + for c in self.contracts: + c_funcs = c.get_functions_reading_from_variable(var) + c_funcs.extend( + f + for f in c.functions + if any(str(v.expression) == str(var.expression) for v in f.variables) + ) + c_funcs = list(set(c_funcs)) + funcs.extend(c_funcs) + fallback = [f for f in var.contract.functions if f.is_fallback] + funcs += fallback + for func in funcs: + rets = func.return_type if func.return_type is not None else [] + for ret in rets: + size, _ = ret.storage_size + if size <= 32: + return str(ret), size * 8 + for node in func.all_nodes(): + exp = node.expression + # Look for use of the common OpenZeppelin StorageSlot library + if f"getAddressSlot({var.name})" in str(exp): + return "address", 160 + if f"getBooleanSlot({var.name})" in str(exp): + return "bool", 1 + if f"getBytes32Slot({var.name})" in str(exp): + return "bytes32", 256 + if f"getUint256Slot({var.name})" in str(exp): + return "uint256", 256 + # Look for variable assignment in assembly loaded from a hardcoded slot + if ( + isinstance(exp, AssignmentOperation) + and isinstance(exp.expression_left, Identifier) + and isinstance(exp.expression_right, CallExpression) + and "sload" in str(exp.expression_right.called) + and str(exp.expression_right.arguments[0]) == str(var.expression) + ): + if func.is_fallback: + return "address", 160 + storage_type = exp.expression_left.value.type.name + size, _ = exp.expression_left.value.type.storage_size + return storage_type, size * 8 + # Look for variable storage in assembly stored to a hardcoded slot + if ( + isinstance(exp, CallExpression) + and "sstore" in str(exp.called) + and isinstance(exp.arguments[0], Identifier) + and isinstance(exp.arguments[1], Identifier) + and str(exp.arguments[0].value.expression) == str(var.expression) + ): + storage_type = exp.arguments[1].value.type.name + size, _ = exp.arguments[1].value.type.storage_size + return storage_type, size * 8 + return storage_type, size + def walk_slot_info(self, func: Callable) -> None: stack = list(self.slot_info.values()) while stack: @@ -220,39 +373,178 @@ class SlitherReadStorage: func(slot_info) def get_slot_values(self, slot_info: SlotInfo) -> None: - """Fetches the slot value of `SlotInfo` object + """ + Fetches the slot value of `SlotInfo` object :param slot_info: """ + assert self.rpc_info is not None hex_bytes = get_storage_data( - self.web3, + self.rpc_info.web3, self.checksum_address, int.to_bytes(slot_info.slot, 32, byteorder="big"), - self.block, + self.rpc_info.block, ) slot_info.value = self.convert_value_to_type( hex_bytes, slot_info.size, slot_info.offset, slot_info.type_string ) logger.info(f"\nValue: {slot_info.value}\n") - def get_all_storage_variables(self, func: Callable = None) -> None: - """Fetches all storage variables from a list of contracts. + def get_all_storage_variables(self, func: Callable = lambda x: x) -> None: + """ + Fetches all storage variables from a list of contracts. kwargs: func (Callable, optional): A criteria to filter functions e.g. name. """ for contract in self.contracts: - self._target_variables.extend( - filter( - func, - [ - (contract, var) - for var in contract.state_variables_ordered - if not var.is_constant and not var.is_immutable - ], - ) + for var in contract.state_variables_ordered: + if func(var): + if not var.is_constant and not var.is_immutable: + self._target_variables.append((contract, var)) + elif ( + self.unstructured + and var.is_constant + and var.type == ElementaryType("bytes32") + ): + self._constant_storage_slots.append((contract, var)) + if self.unstructured: + hardcoded_slot = self.find_hardcoded_slot_in_fallback(contract) + if hardcoded_slot is not None: + self._constant_storage_slots.append((contract, hardcoded_slot)) + + def find_hardcoded_slot_in_fallback(self, contract: Contract) -> Optional[StateVariable]: + """ + Searches the contract's fallback function for a sload from a literal storage slot, i.e., + `let contractLogic := sload(0xc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7)`. + + Args: + contract: a Contract object, which should have a fallback function. + + Returns: + A newly created StateVariable representing the Literal bytes32 slot, if one is found, otherwise None. + """ + fallback = None + for func in contract.functions_entry_points: + if func.is_fallback: + fallback = func + break + if fallback is None: + return None + queue = [fallback.entry_point] + visited = [] + while len(queue) > 0: + node = queue.pop(0) + visited.append(node) + queue.extend(son for son in node.sons if son not in visited) + if node.type == NodeType.ASSEMBLY and isinstance(node.inline_asm, str): + return SlitherReadStorage.find_hardcoded_slot_in_asm_str(node.inline_asm, contract) + if node.type == NodeType.EXPRESSION: + sv = self.find_hardcoded_slot_in_exp(node.expression, contract) + if sv is not None: + return sv + return None + + @staticmethod + def find_hardcoded_slot_in_asm_str( + inline_asm: str, contract: Contract + ) -> Optional[StateVariable]: + """ + Searches a block of assembly code (given as a string) for a sload from a literal storage slot. + Does not work if the argument passed to sload does not start with "0x", i.e., `sload(add(1,1))` + or `and(sload(0), 0xffffffffffffffffffffffffffffffffffffffff)`. + + Args: + inline_asm: a string containing all the code in an assembly node (node.inline_asm for solc < 0.6.0). + + Returns: + A newly created StateVariable representing the Literal bytes32 slot, if one is found, otherwise None. + """ + asm_split = inline_asm.split("\n") + for asm in asm_split: + if "sload(" in asm: # Only handle literals + arg = asm.split("sload(")[1].split(")")[0] + if arg.startswith("0x"): + exp = Literal(arg, ElementaryType("bytes32")) + sv = StateVariable() + sv.name = "fallback_sload_hardcoded" + sv.expression = exp + sv.is_constant = True + sv.type = exp.type + sv.set_contract(contract) + return sv + return None + + def find_hardcoded_slot_in_exp( + self, exp: "Expression", contract: Contract + ) -> Optional[StateVariable]: + """ + Parses an expression to see if it contains a sload from a literal storage slot, + unrolling nested expressions if necessary to determine which slot it loads from. + Args: + exp: an Expression object to search. + contract: the Contract containing exp. + + Returns: + A newly created StateVariable representing the Literal bytes32 slot, if one is found, otherwise None. + """ + if isinstance(exp, AssignmentOperation): + exp = exp.expression_right + while isinstance(exp, BinaryOperation): + exp = next( + (e for e in exp.expressions if isinstance(e, (CallExpression, BinaryOperation))), + exp.expression_left, ) + while isinstance(exp, CallExpression) and len(exp.arguments) > 0: + called = exp.called + exp = exp.arguments[0] + if "sload" in str(called): + break + if isinstance( + exp, + ( + BinaryOperation, + UnaryOperation, + Identifier, + TupleExpression, + TypeConversion, + CallExpression, + ), + ): + try: + exp = ConstantFolding(exp, "bytes32").result() + except NotConstant: + return None + if ( + isinstance(exp, Literal) + and isinstance(exp.type, ElementaryType) + and exp.type.name in ["bytes32", "uint256"] + ): + sv = StateVariable() + sv.name = "fallback_sload_hardcoded" + value = exp.value + str_value = str(value) + if str_value.isdecimal(): + value = int(value) + if isinstance(value, (int, bytes)): + if isinstance(value, bytes): + str_value = "0x" + value.hex() + value = int(str_value, 16) + exp = Literal(str_value, ElementaryType("bytes32")) + state_var_slots = [ + self.get_variable_info(contract, var)[0] + for contract, var in self.target_variables + ] + if value in state_var_slots: + return None + sv.expression = exp + sv.is_constant = True + sv.type = ElementaryType("bytes32") + sv.set_contract(contract) + return sv + return None def convert_slot_info_to_rows(self, slot_info: SlotInfo) -> None: - """Convert and append slot info to table. Create table if it + """ + Convert and append slot info to table. Create table if it does not yet exist :param slot_info: """ @@ -270,7 +562,8 @@ class SlitherReadStorage: def _find_struct_var_slot( elems: List[StructureVariable], slot_as_bytes: bytes, struct_var: str ) -> Tuple[str, str, bytes, int, int]: - """Finds the slot of a structure variable. + """ + Finds the slot of a structure variable. Args: elems (List[StructureVariable]): Ordered list of structure variables. slot_as_bytes (bytes): The slot of the struct to begin searching at. @@ -312,7 +605,8 @@ class SlitherReadStorage: deep_key: int = None, struct_var: str = None, ) -> Tuple[str, str, bytes, int, int]: - """Finds the slot of array's index. + """ + Finds the slot of array's index. Args: target_variable (`StateVariable`): The array that contains the target variable. slot (bytes): The starting slot of the array. @@ -415,7 +709,8 @@ class SlitherReadStorage: deep_key: Union[int, str] = None, struct_var: str = None, ) -> Tuple[str, str, bytes, int, int]: - """Finds the data slot of a target variable within a mapping. + """ + Finds the data slot of a target variable within a mapping. target_variable (`StateVariable`): The mapping that contains the target variable. slot (bytes): The starting slot of the mapping. key (Union[int, str]): The key the variable is stored at. @@ -486,7 +781,7 @@ class SlitherReadStorage: ) info += info_tmp - # TODO: suppory mapping with dynamic arrays + # TODO: support mapping with dynamic arrays # mapping(elem => elem) elif isinstance(target_variable_type.type_to, ElementaryType): @@ -592,7 +887,8 @@ class SlitherReadStorage: return elems def _get_array_length(self, type_: Type, slot: int) -> int: - """Gets the length of dynamic and fixed arrays. + """ + Gets the length of dynamic and fixed arrays. Args: type_ (`AbstractType`): The array type. slot (int): Slot a dynamic array's length is stored at. @@ -600,15 +896,15 @@ class SlitherReadStorage: (int): The length of the array. """ val = 0 - if self.rpc: + if self.rpc_info: # The length of dynamic arrays is stored at the starting slot. # Convert from hexadecimal to decimal. val = int( get_storage_data( - self.web3, + self.rpc_info.web3, self.checksum_address, int.to_bytes(slot, 32, byteorder="big"), - self.block, + self.rpc_info.block, ).hex(), 16, ) diff --git a/slither/tools/read_storage/utils/utils.py b/slither/tools/read_storage/utils/utils.py index 4a04a5b6d..20e7c1372 100644 --- a/slither/tools/read_storage/utils/utils.py +++ b/slither/tools/read_storage/utils/utils.py @@ -37,6 +37,8 @@ def coerce_type( (Union[int, bool, str, ChecksumAddress, hex]): The type representation of the value. """ if "int" in solidity_type: + if str(value).startswith("0x"): + return to_int(hexstr=value) return to_int(value) if "bool" in solidity_type: return bool(to_int(value)) diff --git a/slither/utils/command_line.py b/slither/utils/command_line.py index 911609bf6..082472582 100644 --- a/slither/utils/command_line.py +++ b/slither/utils/command_line.py @@ -1,4 +1,5 @@ import argparse +import enum import json import os import re @@ -27,6 +28,15 @@ JSON_OUTPUT_TYPES = [ "list-printers", ] + +class FailOnLevel(enum.Enum): + PEDANTIC = "pedantic" + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + NONE = "none" + + # Those are the flags shared by the command line and the config file defaults_flag_in_config = { "codex": False, @@ -44,10 +54,7 @@ defaults_flag_in_config = { "exclude_low": False, "exclude_medium": False, "exclude_high": False, - "fail_pedantic": True, - "fail_low": False, - "fail_medium": False, - "fail_high": False, + "fail_on": FailOnLevel.PEDANTIC, "json": None, "sarif": None, "json-types": ",".join(DEFAULT_JSON_OUTPUT_TYPES), @@ -64,6 +71,13 @@ defaults_flag_in_config = { **DEFAULTS_FLAG_IN_CONFIG_CRYTIC_COMPILE, } +deprecated_flags = { + "fail_pedantic": True, + "fail_low": False, + "fail_medium": False, + "fail_high": False, +} + def read_config_file(args: argparse.Namespace) -> None: # No config file was provided as an argument @@ -80,6 +94,12 @@ def read_config_file(args: argparse.Namespace) -> None: with open(args.config_file, encoding="utf8") as f: config = json.load(f) for key, elem in config.items(): + if key in deprecated_flags: + logger.info( + yellow(f"{args.config_file} has a deprecated key: {key} : {elem}") + ) + migrate_config_options(args, key, elem) + continue if key not in defaults_flag_in_config: logger.info( yellow(f"{args.config_file} has an unknown key: {key} : {elem}") @@ -94,6 +114,28 @@ def read_config_file(args: argparse.Namespace) -> None: logger.error(yellow("Falling back to the default settings...")) +def migrate_config_options(args: argparse.Namespace, key: str, elem): + if key.startswith("fail_") and getattr(args, "fail_on") == defaults_flag_in_config["fail_on"]: + if key == "fail_pedantic": + pedantic_setting = elem + fail_on = FailOnLevel.PEDANTIC if pedantic_setting else FailOnLevel.NONE + setattr(args, "fail_on", fail_on) + logger.info(f"Migrating fail_pedantic: {pedantic_setting} as fail_on: {fail_on.value}") + elif key == "fail_low" and elem is True: + logger.info("Migrating fail_low: true -> fail_on: low") + setattr(args, "fail_on", FailOnLevel.LOW) + + elif key == "fail_medium" and elem is True: + logger.info("Migrating fail_medium: true -> fail_on: medium") + setattr(args, "fail_on", FailOnLevel.MEDIUM) + + elif key == "fail_high" and elem is True: + logger.info("Migrating fail_high: true -> fail_on: high") + setattr(args, "fail_on", FailOnLevel.HIGH) + else: + logger.warning(yellow(f"Key {key} was deprecated but no migration was provided")) + + def output_to_markdown( detector_classes: List[Type[AbstractDetector]], printer_classes: List[Type[AbstractPrinter]], diff --git a/slither/utils/integer_conversion.py b/slither/utils/integer_conversion.py index e4dff333c..99064f564 100644 --- a/slither/utils/integer_conversion.py +++ b/slither/utils/integer_conversion.py @@ -4,7 +4,9 @@ from typing import Union from slither.exceptions import SlitherError -def convert_string_to_fraction(val: Union[str, int]) -> Fraction: +def convert_string_to_fraction(val: Union[str, bytes, int]) -> Fraction: + if isinstance(val, bytes): + return int.from_bytes(val, byteorder="big") if isinstance(val, int): return Fraction(val) if val.startswith(("0x", "0X")): diff --git a/slither/utils/loc.py b/slither/utils/loc.py new file mode 100644 index 000000000..0e51dfa46 --- /dev/null +++ b/slither/utils/loc.py @@ -0,0 +1,105 @@ +from dataclasses import dataclass +from pathlib import Path +from typing import List, Tuple + +from slither import Slither +from slither.utils.myprettytable import MyPrettyTable +from slither.utils.tests_pattern import is_test_file + + +@dataclass +class LoCInfo: + loc: int = 0 + sloc: int = 0 + cloc: int = 0 + + def total(self) -> int: + return self.loc + self.sloc + self.cloc + + +@dataclass +class LoC: + src: LoCInfo = LoCInfo() + dep: LoCInfo = LoCInfo() + test: LoCInfo = LoCInfo() + + def to_pretty_table(self) -> MyPrettyTable: + table = MyPrettyTable(["", "src", "dep", "test"]) + + table.add_row(["loc", str(self.src.loc), str(self.dep.loc), str(self.test.loc)]) + table.add_row(["sloc", str(self.src.sloc), str(self.dep.sloc), str(self.test.sloc)]) + table.add_row(["cloc", str(self.src.cloc), str(self.dep.cloc), str(self.test.cloc)]) + table.add_row( + ["Total", str(self.src.total()), str(self.dep.total()), str(self.test.total())] + ) + return table + + +def count_lines(contract_lines: List[str]) -> Tuple[int, int, int]: + """Function to count and classify the lines of code in a contract. + Args: + contract_lines: list(str) representing the lines of a contract. + Returns: + tuple(int, int, int) representing (cloc, sloc, loc) + """ + multiline_comment = False + cloc = 0 + sloc = 0 + loc = 0 + + for line in contract_lines: + loc += 1 + stripped_line = line.strip() + if not multiline_comment: + if stripped_line.startswith("//"): + cloc += 1 + elif "/*" in stripped_line: + # Account for case where /* is followed by */ on the same line. + # If it is, then multiline_comment does not need to be set to True + start_idx = stripped_line.find("/*") + end_idx = stripped_line.find("*/", start_idx + 2) + if end_idx == -1: + multiline_comment = True + cloc += 1 + elif stripped_line: + sloc += 1 + else: + cloc += 1 + if "*/" in stripped_line: + multiline_comment = False + + return cloc, sloc, loc + + +def _update_lines(loc_info: LoCInfo, lines: list) -> None: + """An internal function used to update (mutate in place) the loc_info. + + Args: + loc_info: LoCInfo to be updated + lines: list(str) representing the lines of a contract. + """ + cloc, sloc, loc = count_lines(lines) + loc_info.loc += loc + loc_info.cloc += cloc + loc_info.sloc += sloc + + +def compute_loc_metrics(slither: Slither) -> LoC: + """Used to compute the lines of code metrics for a Slither object. + + Args: + slither: A Slither object + Returns: + A LoC object + """ + + loc = LoC() + + for filename, source_code in slither.source_code.items(): + current_lines = source_code.splitlines() + is_dep = False + if slither.crytic_compile: + is_dep = slither.crytic_compile.is_dependency(filename) + loc_type = loc.dep if is_dep else loc.test if is_test_file(Path(filename)) else loc.src + _update_lines(loc_type, current_lines) + return loc diff --git a/slither/utils/upgradeability.py b/slither/utils/upgradeability.py index 7b4e8493a..3bef3b9b6 100644 --- a/slither/utils/upgradeability.py +++ b/slither/utils/upgradeability.py @@ -19,10 +19,12 @@ from slither.core.variables.local_variable_init_from_tuple import LocalVariableI from slither.core.variables.state_variable import StateVariable from slither.analyses.data_dependency.data_dependency import get_dependencies from slither.core.variables.variable import Variable -from slither.core.expressions.literal import Literal -from slither.core.expressions.identifier import Identifier -from slither.core.expressions.call_expression import CallExpression -from slither.core.expressions.assignment_operation import AssignmentOperation +from slither.core.expressions import ( + Literal, + Identifier, + CallExpression, + AssignmentOperation, +) from slither.core.cfg.node import Node, NodeType from slither.slithir.operations import ( Operation, @@ -61,11 +63,42 @@ from slither.slithir.variables import ( from slither.tools.read_storage.read_storage import SlotInfo, SlitherReadStorage +class TaintedExternalContract: + def __init__(self, contract: "Contract") -> None: + self._contract: Contract = contract + self._tainted_functions: List[Function] = [] + self._tainted_variables: List[Variable] = [] + + @property + def contract(self) -> Contract: + return self._contract + + @property + def tainted_functions(self) -> List[Function]: + return self._tainted_functions + + def add_tainted_function(self, f: Function): + self._tainted_functions.append(f) + + @property + def tainted_variables(self) -> List[Variable]: + return self._tainted_variables + + def add_tainted_variable(self, v: Variable): + self._tainted_variables.append(v) + + # pylint: disable=too-many-locals def compare( - v1: Contract, v2: Contract + v1: Contract, v2: Contract, include_external: bool = False ) -> Tuple[ - List[Variable], List[Variable], List[Variable], List[Function], List[Function], List[Function] + List[Variable], + List[Variable], + List[Variable], + List[Function], + List[Function], + List[Function], + List[TaintedExternalContract], ]: """ Compares two versions of a contract. Most useful for upgradeable (logic) contracts, @@ -74,6 +107,7 @@ def compare( Args: v1: Original version of (upgradeable) contract v2: Updated version of (upgradeable) contract + include_external: Optional flag to enable cross-contract external taint analysis Returns: missing-vars-in-v2: list[Variable], @@ -82,6 +116,7 @@ def compare( new-functions: list[Function], modified-functions: list[Function], tainted-functions: list[Function] + tainted-contracts: list[TaintedExternalContract] """ order_vars1 = [ @@ -113,17 +148,13 @@ def compare( if sig not in func_sigs1: new_modified_functions.append(function) new_functions.append(function) - new_modified_function_vars += ( - function.state_variables_read + function.state_variables_written - ) + new_modified_function_vars += function.all_state_variables_written() elif not function.is_constructor_variables and is_function_modified( orig_function, function ): new_modified_functions.append(function) modified_functions.append(function) - new_modified_function_vars += ( - function.state_variables_read + function.state_variables_written - ) + new_modified_function_vars += function.all_state_variables_written() # Find all unmodified functions that call a modified function or read/write the # same state variable(s) as a new/modified function, i.e., tainted functions @@ -140,25 +171,28 @@ def compare( tainted_vars = [ var for var in set(new_modified_function_vars) - if var in function.variables_read_or_written + if var in function.all_state_variables_read() + function.all_state_variables_written() and not var.is_constant and not var.is_immutable ] if len(modified_calls) > 0 or len(tainted_vars) > 0: tainted_functions.append(function) - # Find all new or tainted variables, i.e., variables that are read or written by a new/modified/tainted function + # Find all new or tainted variables, i.e., variables that are written by a new/modified/tainted function for var in order_vars2: - read_by = v2.get_functions_reading_from_variable(var) written_by = v2.get_functions_writing_to_variable(var) - if v1.get_state_variable_from_name(var.name) is None: + if next((v for v in v1.state_variables_ordered if v.name == var.name), None) is None: new_variables.append(var) - elif any( - func in read_by or func in written_by - for func in new_modified_functions + tainted_functions - ): + elif any(func in written_by for func in new_modified_functions + tainted_functions): tainted_variables.append(var) + tainted_contracts = [] + if include_external: + # Find all external contracts and functions called by new/modified/tainted functions + tainted_contracts = tainted_external_contracts( + new_functions + modified_functions + tainted_functions + ) + return ( missing_vars_in_v2, new_variables, @@ -166,9 +200,137 @@ def compare( new_functions, modified_functions, tainted_functions, + tainted_contracts, ) +def tainted_external_contracts(funcs: List[Function]) -> List[TaintedExternalContract]: + """ + Takes a list of functions from one contract, finds any calls in these to functions in external contracts, + and determines which variables and functions in the external contracts are tainted by these external calls. + Args: + funcs: a list of Function objects to search for external calls. + + Returns: + TaintedExternalContract() ( + contract: Contract, + tainted_functions: List[TaintedFunction], + tainted_variables: List[TaintedVariable] + ) + """ + tainted_contracts: dict[str, TaintedExternalContract] = {} + tainted_list: list[TaintedExternalContract] = [] + + for func in funcs: + for contract, target in func.all_high_level_calls(): + if contract.is_library: + # Not interested in library calls + continue + if contract.name not in tainted_contracts: + # A contract may be tainted by multiple function calls - only make one TaintedExternalContract object + tainted_contracts[contract.name] = TaintedExternalContract(contract) + if ( + isinstance(target, Function) + and target not in funcs + and target not in (f for f in tainted_contracts[contract.name].tainted_functions) + and not (target.is_constructor or target.is_fallback or target.is_receive) + ): + # Found a high-level call to a new tainted function + tainted_contracts[contract.name].add_tainted_function(target) + for var in target.all_state_variables_written(): + # Consider as tainted all variables written by the tainted function + if var not in (v for v in tainted_contracts[contract.name].tainted_variables): + tainted_contracts[contract.name].add_tainted_variable(var) + elif ( + isinstance(target, StateVariable) + and target not in (v for v in tainted_contracts[contract.name].tainted_variables) + and not (target.is_constant or target.is_immutable) + ): + # Found a new high-level call to a public state variable getter + tainted_contracts[contract.name].add_tainted_variable(target) + for c in tainted_contracts.values(): + tainted_list.append(c) + contract = c.contract + variables = c.tainted_variables + for var in variables: + # For each tainted variable, consider as tainted any function that reads or writes to it + read_write = set( + contract.get_functions_reading_from_variable(var) + + contract.get_functions_writing_to_variable(var) + ) + for f in read_write: + if f not in tainted_contracts[contract.name].tainted_functions and not ( + f.is_constructor or f.is_fallback or f.is_receive + ): + c.add_tainted_function(f) + return tainted_list + + +def tainted_inheriting_contracts( + tainted_contracts: List[TaintedExternalContract], contracts: List[Contract] = None +) -> List[TaintedExternalContract]: + """ + Takes a list of TaintedExternalContract obtained from tainted_external_contracts, and finds any contracts which + inherit a tainted contract, as well as any functions that call tainted functions or read tainted variables in + the inherited contract. + Args: + tainted_contracts: the list obtained from `tainted_external_contracts` or `compare`. + contracts: (optional) the list of contracts to check for inheritance. If not provided, defaults to + `contract.compilation_unit.contracts` for each contract in tainted_contracts. + + Returns: + An updated list of TaintedExternalContract, including all from the input list. + """ + for tainted in tainted_contracts: + contract = tainted.contract + check_contracts = contracts + if contracts is None: + check_contracts = contract.compilation_unit.contracts + # We are only interested in checking contracts that inherit a tainted contract + check_contracts = [ + c + for c in check_contracts + if c.name not in [t.contract.name for t in tainted_contracts] + and contract.name in [i.name for i in c.inheritance] + ] + for c in check_contracts: + new_taint = TaintedExternalContract(c) + for f in c.functions_declared: + # Search for functions that call an inherited tainted function or access an inherited tainted variable + internal_calls = [c for c in f.all_internal_calls() if isinstance(c, Function)] + if any( + call.canonical_name == t.canonical_name + for t in tainted.tainted_functions + for call in internal_calls + ) or any( + var.canonical_name == t.canonical_name + for t in tainted.tainted_variables + for var in f.all_state_variables_read() + f.all_state_variables_written() + ): + new_taint.add_tainted_function(f) + for f in new_taint.tainted_functions: + # For each newly found tainted function, consider as tainted any variable it writes to + for var in f.all_state_variables_written(): + if var not in ( + v for v in tainted.tainted_variables + new_taint.tainted_variables + ): + new_taint.add_tainted_variable(var) + for var in new_taint.tainted_variables: + # For each newly found tainted variable, consider as tainted any function that reads or writes to it + read_write = set( + contract.get_functions_reading_from_variable(var) + + contract.get_functions_writing_to_variable(var) + ) + for f in read_write: + if f not in ( + t for t in tainted.tainted_functions + new_taint.tainted_functions + ) and not (f.is_constructor or f.is_fallback or f.is_receive): + new_taint.add_tainted_function(f) + if len(new_taint.tainted_functions) > 0: + tainted_contracts.append(new_taint) + return tainted_contracts + + def get_missing_vars(v1: Contract, v2: Contract) -> List[StateVariable]: """ Gets all non-constant/immutable StateVariables that appear in v1 but not v2 @@ -220,6 +382,8 @@ def is_function_modified(f1: Function, f2: Function) -> bool: visited.extend([node_f1, node_f2]) queue_f1.extend(son for son in node_f1.sons if son not in visited) queue_f2.extend(son for son in node_f2.sons if son not in visited) + if len(node_f1.irs) != len(node_f2.irs): + return True for i, ir in enumerate(node_f1.irs): if encode_ir_for_compare(ir) != encode_ir_for_compare(node_f2.irs[i]): return True @@ -273,13 +437,13 @@ def encode_ir_for_compare(ir: Operation) -> str: if isinstance(ir, Assignment): return f"({encode_var_for_compare(ir.lvalue)}):=({encode_var_for_compare(ir.rvalue)})" if isinstance(ir, Index): - return f"index({ntype(ir.index_type)})" + return f"index({ntype(ir.variable_right.type)})" if isinstance(ir, Member): return "member" # .format(ntype(ir._type)) if isinstance(ir, Length): return "length" if isinstance(ir, Binary): - return f"binary({str(ir.variable_left)}{str(ir.type)}{str(ir.variable_right)})" + return f"binary({encode_var_for_compare(ir.variable_left)}{ir.type}{encode_var_for_compare(ir.variable_right)})" if isinstance(ir, Unary): return f"unary({str(ir.type)})" if isinstance(ir, Condition): @@ -330,7 +494,7 @@ def encode_var_for_compare(var: Variable) -> str: # variables if isinstance(var, Constant): - return f"constant({ntype(var.type)})" + return f"constant({ntype(var.type)},{var.value})" if isinstance(var, SolidityVariableComposed): return f"solidity_variable_composed({var.name})" if isinstance(var, SolidityVariable): @@ -340,9 +504,16 @@ def encode_var_for_compare(var: Variable) -> str: if isinstance(var, ReferenceVariable): return f"reference({ntype(var.type)})" if isinstance(var, LocalVariable): - return f"local_solc_variable({var.location})" + return f"local_solc_variable({ntype(var.type)},{var.location})" if isinstance(var, StateVariable): - return f"state_solc_variable({ntype(var.type)})" + if not (var.is_constant or var.is_immutable): + try: + slot, _ = var.contract.compilation_unit.storage_layout_of(var.contract, var) + except KeyError: + slot = var.name + else: + slot = var.name + return f"state_solc_variable({ntype(var.type)},{slot})" if isinstance(var, LocalVariableInitFromTuple): return "local_variable_init_tuple" if isinstance(var, TupleVariable): @@ -398,6 +569,7 @@ def get_proxy_implementation_var(proxy: Contract) -> Optional[Variable]: try: delegate = next(var for var in dependencies if isinstance(var, StateVariable)) except StopIteration: + # TODO: Handle cases where get_dependencies doesn't return any state variables. return delegate return delegate diff --git a/slither/visitors/expression/constants_folding.py b/slither/visitors/expression/constants_folding.py index 12eb6be9d..b1fa570c6 100644 --- a/slither/visitors/expression/constants_folding.py +++ b/slither/visitors/expression/constants_folding.py @@ -1,5 +1,6 @@ from fractions import Fraction from typing import Union +from Crypto.Hash import keccak from slither.core import expressions from slither.core.expressions import ( @@ -11,9 +12,9 @@ from slither.core.expressions import ( UnaryOperation, TupleExpression, TypeConversion, + CallExpression, ) from slither.core.variables import Variable - from slither.utils.integer_conversion import convert_string_to_fraction, convert_string_to_int from slither.visitors.expression.expression import ExpressionVisitor from slither.core.solidity_types.elementary_type import ElementaryType @@ -65,23 +66,31 @@ class ConstantFolding(ExpressionVisitor): value = value & (2**256 - 1) return Literal(value, self._type) + # pylint: disable=import-outside-toplevel def _post_identifier(self, expression: Identifier) -> None: - if not isinstance(expression.value, Variable): - return - if not expression.value.is_constant: + from slither.core.declarations.solidity_variables import SolidityFunction + + if isinstance(expression.value, Variable): + if expression.value.is_constant: + expr = expression.value.expression + # assumption that we won't have infinite loop + # Everything outside of literal + if isinstance( + expr, + (BinaryOperation, UnaryOperation, Identifier, TupleExpression, TypeConversion), + ): + cf = ConstantFolding(expr, self._type) + expr = cf.result() + assert isinstance(expr, Literal) + set_val(expression, convert_string_to_int(expr.converted_value)) + else: + raise NotConstant + elif isinstance(expression.value, SolidityFunction): + set_val(expression, expression.value) + else: raise NotConstant - expr = expression.value.expression - # assumption that we won't have infinite loop - # Everything outside of literal - if isinstance( - expr, (BinaryOperation, UnaryOperation, Identifier, TupleExpression, TypeConversion) - ): - cf = ConstantFolding(expr, self._type) - expr = cf.result() - assert isinstance(expr, Literal) - set_val(expression, convert_string_to_int(expr.converted_value)) - # pylint: disable=too-many-branches + # pylint: disable=too-many-branches,too-many-statements def _post_binary_operation(self, expression: BinaryOperation) -> None: expression_left = expression.expression_left expression_right = expression.expression_right @@ -95,7 +104,6 @@ class ConstantFolding(ExpressionVisitor): (Literal, BinaryOperation, UnaryOperation, Identifier, TupleExpression, TypeConversion), ): raise NotConstant - left = get_val(expression_left) right = get_val(expression_right) @@ -183,7 +191,9 @@ class ConstantFolding(ExpressionVisitor): raise NotConstant def _post_literal(self, expression: Literal) -> None: - if expression.converted_value in ["true", "false"]: + if str(expression.type) == "bool": + set_val(expression, expression.converted_value) + elif str(expression.type) == "string": set_val(expression, expression.converted_value) else: try: @@ -195,7 +205,14 @@ class ConstantFolding(ExpressionVisitor): raise NotConstant def _post_call_expression(self, expression: expressions.CallExpression) -> None: - raise NotConstant + called = get_val(expression.called) + args = [get_val(arg) for arg in expression.arguments] + if called.name == "keccak256(bytes)": + digest = keccak.new(digest_bits=256) + digest.update(str(args[0]).encode("utf-8")) + set_val(expression, digest.digest()) + else: + raise NotConstant def _post_conditional_expression(self, expression: expressions.ConditionalExpression) -> None: raise NotConstant @@ -247,10 +264,24 @@ class ConstantFolding(ExpressionVisitor): expr = expression.expression if not isinstance( expr, - (Literal, BinaryOperation, UnaryOperation, Identifier, TupleExpression, TypeConversion), + ( + Literal, + BinaryOperation, + UnaryOperation, + Identifier, + TupleExpression, + TypeConversion, + CallExpression, + ), ): raise NotConstant cf = ConstantFolding(expr, self._type) expr = cf.result() assert isinstance(expr, Literal) - set_val(expression, convert_string_to_fraction(expr.converted_value)) + if str(expression.type).startswith("uint") and isinstance(expr.value, bytes): + value = int.from_bytes(expr.value, "big") + elif str(expression.type).startswith("byte") and isinstance(expr.value, int): + value = int.to_bytes(expr.value, 32, "big") + else: + value = convert_string_to_fraction(expr.converted_value) + set_val(expression, value) diff --git a/slither/visitors/expression/expression_printer.py b/slither/visitors/expression/expression_printer.py index 601627c02..61af4f1c9 100644 --- a/slither/visitors/expression/expression_printer.py +++ b/slither/visitors/expression/expression_printer.py @@ -76,8 +76,7 @@ class ExpressionPrinter(ExpressionVisitor): def _post_new_array(self, expression: expressions.NewArray) -> None: array = str(expression.array_type) - depth = expression.depth - val = f"new {array}{'[]' * depth}" + val = f"new {array}" set_val(expression, val) def _post_new_contract(self, expression: expressions.NewContract) -> None: diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index e4fa5a1b1..005ad81a4 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -532,7 +532,7 @@ class ExpressionToSlithIR(ExpressionVisitor): def _post_new_array(self, expression: NewArray) -> None: val = TemporaryVariable(self._node) - operation = TmpNewArray(expression.depth, expression.array_type, val) + operation = TmpNewArray(expression.array_type, val) operation.set_expression(expression) self._result.append(operation) set_val(expression, val) diff --git a/tests/conftest.py b/tests/conftest.py index 5ea228fd3..63fccfa12 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,12 +1,12 @@ # pylint: disable=redefined-outer-name import os -from pathlib import Path -import tempfile import shutil +import tempfile +from pathlib import Path from contextlib import contextmanager -import pytest from filelock import FileLock from solc_select import solc_select +import pytest from slither import Slither diff --git a/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_4_25_array_by_reference_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_4_25_array_by_reference_sol__0.txt index f056bea10..5cb8add39 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_4_25_array_by_reference_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_4_25_array_by_reference_sol__0.txt @@ -1,12 +1,14 @@ -D.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#39)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#25-28)which only takes arrays by value +D.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#39) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#21-23) which only takes arrays by value -D.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#39)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#21-23)which only takes arrays by value +C.g() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#11) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#25-28) which only takes arrays by value -C.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#2)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#21-23)which only takes arrays by value +C.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#2) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#25-28) which only takes arrays by value -C.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#2)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#25-28)which only takes arrays by value +C.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#2) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#21-23) which only takes arrays by value -C.g() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#11)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#25-28)which only takes arrays by value +D.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#39) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#25-28) which only takes arrays by value -C.g() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#11)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#21-23)which only takes arrays by value +C.g() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#11) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#21-23) which only takes arrays by value + +E.f() (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#57-61) passes array E.x (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#54) by reference to E.setByValue(uint256[1],uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol#63-66) which only takes arrays by value diff --git a/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_5_16_array_by_reference_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_5_16_array_by_reference_sol__0.txt index 4264c809a..6e97d8cc2 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_5_16_array_by_reference_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_5_16_array_by_reference_sol__0.txt @@ -1,12 +1,14 @@ -D.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#39)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#25-28)which only takes arrays by value +D.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#39) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#21-23) which only takes arrays by value -D.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#39)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#21-23)which only takes arrays by value +C.g() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#11) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#25-28) which only takes arrays by value -C.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#2)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#21-23)which only takes arrays by value +C.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#2) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#25-28) which only takes arrays by value -C.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#2)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#25-28)which only takes arrays by value +C.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#2) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#21-23) which only takes arrays by value -C.g() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#11)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#25-28)which only takes arrays by value +D.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#39) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#25-28) which only takes arrays by value -C.g() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#11)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#21-23)which only takes arrays by value +C.g() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#11) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#21-23) which only takes arrays by value + +E.f() (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#57-61) passes array E.x (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#54) by reference to E.setByValue(uint256[1],uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol#63-66) which only takes arrays by value diff --git a/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_6_11_array_by_reference_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_6_11_array_by_reference_sol__0.txt index e71930b51..39574b5f5 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_6_11_array_by_reference_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_6_11_array_by_reference_sol__0.txt @@ -1,12 +1,14 @@ -D.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#39)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#25-28)which only takes arrays by value +D.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#39) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#21-23) which only takes arrays by value -D.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#39)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#21-23)which only takes arrays by value +C.g() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#11) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#25-28) which only takes arrays by value -C.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#2)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#21-23)which only takes arrays by value +C.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#2) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#25-28) which only takes arrays by value -C.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#2)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#25-28)which only takes arrays by value +C.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#2) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#21-23) which only takes arrays by value -C.g() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#11)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#25-28)which only takes arrays by value +D.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#39) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#25-28) which only takes arrays by value -C.g() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#11)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#21-23)which only takes arrays by value +C.g() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#11) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#21-23) which only takes arrays by value + +E.f() (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#57-61) passes array E.x (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#54) by reference to E.setByValue(uint256[1],uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol#63-66) which only takes arrays by value diff --git a/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_7_6_array_by_reference_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_7_6_array_by_reference_sol__0.txt index 7c0f9ccd9..74ea36a0c 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_7_6_array_by_reference_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_ArrayByReference_0_7_6_array_by_reference_sol__0.txt @@ -1,12 +1,14 @@ -D.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#39)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#25-28)which only takes arrays by value +D.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#39) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#21-23) which only takes arrays by value -D.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#39)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#21-23)which only takes arrays by value +C.g() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#11) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#25-28) which only takes arrays by value -C.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#2)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#21-23)which only takes arrays by value +C.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#2) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#25-28) which only takes arrays by value -C.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#2)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#25-28)which only takes arrays by value +C.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#4-8) passes array C.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#2) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#21-23) which only takes arrays by value -C.g() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#11)by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#25-28)which only takes arrays by value +D.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#42-48) passes array D.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#39) by reference to C.setByValueAndReturn(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#25-28) which only takes arrays by value -C.g() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#11)by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#21-23)which only takes arrays by value +C.g() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#10-15) passes array C.g().y (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#11) by reference to C.setByValue(uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#21-23) which only takes arrays by value + +E.f() (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#57-61) passes array E.x (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#54) by reference to E.setByValue(uint256[1],uint256[1]) (tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol#63-66) which only takes arrays by value diff --git a/tests/e2e/detectors/snapshots/detectors__detector_BuiltinSymbolShadowing_0_4_25_shadowing_builtin_symbols_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_BuiltinSymbolShadowing_0_4_25_shadowing_builtin_symbols_sol__0.txt index 7c94735da..283b93ee5 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_BuiltinSymbolShadowing_0_4_25_shadowing_builtin_symbols_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_BuiltinSymbolShadowing_0_4_25_shadowing_builtin_symbols_sol__0.txt @@ -12,12 +12,12 @@ FurtherExtendedContract.this (tests/e2e/detectors/test_data/shadowing-builtin/0. BaseContract.now (tests/e2e/detectors/test_data/shadowing-builtin/0.4.25/shadowing_builtin_symbols.sol#5) (state variable) shadows built-in symbol" -BaseContractrevert(bool) (tests/e2e/detectors/test_data/shadowing-builtin/0.4.25/shadowing_builtin_symbols.sol#7) (event) shadows built-in symbol" - ExtendedContract.assert(bool).msg (tests/e2e/detectors/test_data/shadowing-builtin/0.4.25/shadowing_builtin_symbols.sol#14) (local variable) shadows built-in symbol" ExtendedContract.assert(bool) (tests/e2e/detectors/test_data/shadowing-builtin/0.4.25/shadowing_builtin_symbols.sol#13-15) (function) shadows built-in symbol" +BaseContract.revert(bool) (tests/e2e/detectors/test_data/shadowing-builtin/0.4.25/shadowing_builtin_symbols.sol#7) (event) shadows built-in symbol" + FurtherExtendedContract.require().sha3 (tests/e2e/detectors/test_data/shadowing-builtin/0.4.25/shadowing_builtin_symbols.sol#26) (local variable) shadows built-in symbol" FurtherExtendedContract.blockhash (tests/e2e/detectors/test_data/shadowing-builtin/0.4.25/shadowing_builtin_symbols.sol#19) (state variable) shadows built-in symbol" diff --git a/tests/e2e/detectors/snapshots/detectors__detector_BuiltinSymbolShadowing_0_5_16_shadowing_builtin_symbols_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_BuiltinSymbolShadowing_0_5_16_shadowing_builtin_symbols_sol__0.txt index f576e19e0..2780f9e62 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_BuiltinSymbolShadowing_0_5_16_shadowing_builtin_symbols_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_BuiltinSymbolShadowing_0_5_16_shadowing_builtin_symbols_sol__0.txt @@ -10,12 +10,12 @@ FurtherExtendedContract.this (tests/e2e/detectors/test_data/shadowing-builtin/0. BaseContract.now (tests/e2e/detectors/test_data/shadowing-builtin/0.5.16/shadowing_builtin_symbols.sol#5) (state variable) shadows built-in symbol" -BaseContractrevert(bool) (tests/e2e/detectors/test_data/shadowing-builtin/0.5.16/shadowing_builtin_symbols.sol#7) (event) shadows built-in symbol" - ExtendedContract.assert(bool).msg (tests/e2e/detectors/test_data/shadowing-builtin/0.5.16/shadowing_builtin_symbols.sol#14) (local variable) shadows built-in symbol" ExtendedContract.assert(bool) (tests/e2e/detectors/test_data/shadowing-builtin/0.5.16/shadowing_builtin_symbols.sol#13-15) (function) shadows built-in symbol" +BaseContract.revert(bool) (tests/e2e/detectors/test_data/shadowing-builtin/0.5.16/shadowing_builtin_symbols.sol#7) (event) shadows built-in symbol" + FurtherExtendedContract.require().sha3 (tests/e2e/detectors/test_data/shadowing-builtin/0.5.16/shadowing_builtin_symbols.sol#26) (local variable) shadows built-in symbol" FurtherExtendedContract.blockhash (tests/e2e/detectors/test_data/shadowing-builtin/0.5.16/shadowing_builtin_symbols.sol#19) (state variable) shadows built-in symbol" diff --git a/tests/e2e/detectors/snapshots/detectors__detector_CacheArrayLength_0_8_17_CacheArrayLength_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_CacheArrayLength_0_8_17_CacheArrayLength_sol__0.txt new file mode 100644 index 000000000..63d4b883c --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_CacheArrayLength_0_8_17_CacheArrayLength_sol__0.txt @@ -0,0 +1,18 @@ +Loop condition at i < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#36) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at i_scope_22 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#166) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at j_scope_11 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#108) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at i_scope_6 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#79) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at i_scope_21 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#160) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at k_scope_9 < array2.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#98) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at k_scope_17 < array2.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#132) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at j_scope_15 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#125) should use cached array length instead of referencing `length` member of the storage array. + +Loop condition at i_scope_4 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#67) should use cached array length instead of referencing `length` member of the storage array. + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_IncorrectUsingFor_0_8_17_IncorrectUsingForTopLevel_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_IncorrectUsingFor_0_8_17_IncorrectUsingForTopLevel_sol__0.txt new file mode 100644 index 000000000..518fba20d --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_IncorrectUsingFor_0_8_17_IncorrectUsingForTopLevel_sol__0.txt @@ -0,0 +1,24 @@ +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#84 is incorrect - no matching function for bytes17[] found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#85 is incorrect - no matching function for uint256 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#90 is incorrect - no matching function for mapping(int256 => uint128) found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#86 is incorrect - no matching function for int256 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#89 is incorrect - no matching function for E2 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#93 is incorrect - no matching function for bytes[][] found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#92 is incorrect - no matching function for string[][] found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#91 is incorrect - no matching function for mapping(int128 => uint256) found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#87 is incorrect - no matching function for bytes18 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#88 is incorrect - no matching function for S2 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#83 is incorrect - no matching function for C3 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + +using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#94 is incorrect - no matching function for custom_int found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64). + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_4_25_shadowing_local_variable_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_4_25_shadowing_local_variable_sol__0.txt index 913acf4ea..0f98475aa 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_4_25_shadowing_local_variable_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_4_25_shadowing_local_variable_sol__0.txt @@ -10,7 +10,7 @@ FurtherExtendedContract.shadowingParent(uint256).y (tests/e2e/detectors/test_dat - BaseContract.y (tests/e2e/detectors/test_data/shadowing-local/0.4.25/shadowing_local_variable.sol#5) (state variable) FurtherExtendedContract.shadowingParent(uint256).v (tests/e2e/detectors/test_data/shadowing-local/0.4.25/shadowing_local_variable.sol#25) shadows: - - ExtendedContractv() (tests/e2e/detectors/test_data/shadowing-local/0.4.25/shadowing_local_variable.sol#13) (event) + - ExtendedContract.v() (tests/e2e/detectors/test_data/shadowing-local/0.4.25/shadowing_local_variable.sol#13) (event) FurtherExtendedContract.shadowingParent(uint256).w (tests/e2e/detectors/test_data/shadowing-local/0.4.25/shadowing_local_variable.sol#25) shadows: - FurtherExtendedContract.w() (tests/e2e/detectors/test_data/shadowing-local/0.4.25/shadowing_local_variable.sol#20-23) (modifier) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_5_16_shadowing_local_variable_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_5_16_shadowing_local_variable_sol__0.txt index 7fcbe83d2..56d139bea 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_5_16_shadowing_local_variable_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_5_16_shadowing_local_variable_sol__0.txt @@ -10,7 +10,7 @@ FurtherExtendedContract.shadowingParent(uint256).y (tests/e2e/detectors/test_dat - BaseContract.y (tests/e2e/detectors/test_data/shadowing-local/0.5.16/shadowing_local_variable.sol#5) (state variable) FurtherExtendedContract.shadowingParent(uint256).v (tests/e2e/detectors/test_data/shadowing-local/0.5.16/shadowing_local_variable.sol#25) shadows: - - ExtendedContractv() (tests/e2e/detectors/test_data/shadowing-local/0.5.16/shadowing_local_variable.sol#13) (event) + - ExtendedContract.v() (tests/e2e/detectors/test_data/shadowing-local/0.5.16/shadowing_local_variable.sol#13) (event) FurtherExtendedContract.shadowingParent(uint256).w (tests/e2e/detectors/test_data/shadowing-local/0.5.16/shadowing_local_variable.sol#25) shadows: - FurtherExtendedContract.w() (tests/e2e/detectors/test_data/shadowing-local/0.5.16/shadowing_local_variable.sol#20-23) (modifier) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_6_11_shadowing_local_variable_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_6_11_shadowing_local_variable_sol__0.txt index b4de3265f..a52ac48af 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_6_11_shadowing_local_variable_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_6_11_shadowing_local_variable_sol__0.txt @@ -5,7 +5,7 @@ FurtherExtendedContract.shadowingParent(uint256).y (tests/e2e/detectors/test_dat - BaseContract.y (tests/e2e/detectors/test_data/shadowing-local/0.6.11/shadowing_local_variable.sol#5) (state variable) FurtherExtendedContract.shadowingParent(uint256).v (tests/e2e/detectors/test_data/shadowing-local/0.6.11/shadowing_local_variable.sol#25) shadows: - - ExtendedContractv() (tests/e2e/detectors/test_data/shadowing-local/0.6.11/shadowing_local_variable.sol#13) (event) + - ExtendedContract.v() (tests/e2e/detectors/test_data/shadowing-local/0.6.11/shadowing_local_variable.sol#13) (event) FurtherExtendedContract.shadowingParent(uint256).w (tests/e2e/detectors/test_data/shadowing-local/0.6.11/shadowing_local_variable.sol#25) shadows: - FurtherExtendedContract.w() (tests/e2e/detectors/test_data/shadowing-local/0.6.11/shadowing_local_variable.sol#20-23) (modifier) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_7_6_shadowing_local_variable_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_7_6_shadowing_local_variable_sol__0.txt index 43ecfab68..e618b988c 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_7_6_shadowing_local_variable_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_LocalShadowing_0_7_6_shadowing_local_variable_sol__0.txt @@ -5,7 +5,7 @@ FurtherExtendedContract.shadowingParent(uint256).y (tests/e2e/detectors/test_dat - BaseContract.y (tests/e2e/detectors/test_data/shadowing-local/0.7.6/shadowing_local_variable.sol#5) (state variable) FurtherExtendedContract.shadowingParent(uint256).v (tests/e2e/detectors/test_data/shadowing-local/0.7.6/shadowing_local_variable.sol#25) shadows: - - ExtendedContractv() (tests/e2e/detectors/test_data/shadowing-local/0.7.6/shadowing_local_variable.sol#13) (event) + - ExtendedContract.v() (tests/e2e/detectors/test_data/shadowing-local/0.7.6/shadowing_local_variable.sol#13) (event) FurtherExtendedContract.shadowingParent(uint256).w (tests/e2e/detectors/test_data/shadowing-local/0.7.6/shadowing_local_variable.sol#25) shadows: - FurtherExtendedContract.w() (tests/e2e/detectors/test_data/shadowing-local/0.7.6/shadowing_local_variable.sol#20-23) (modifier) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_4_25_locked_ether_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_4_25_locked_ether_sol__0.txt index edca6eb2e..680f77d0d 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_4_25_locked_ether_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_4_25_locked_ether_sol__0.txt @@ -1,5 +1,10 @@ Contract locking ether found: - Contract OnlyLocked (tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol#26) has payable functions: + Contract OnlyLocked (tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol#37) has payable functions: + - Locked.receive() (tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol#4-6) + But does not have a function to withdraw the ether + +Contract locking ether found: + Contract UnlockedAssembly (tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol#27-35) has payable functions: - Locked.receive() (tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol#4-6) But does not have a function to withdraw the ether diff --git a/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_5_16_locked_ether_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_5_16_locked_ether_sol__0.txt index d1ff3314b..961ba8c48 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_5_16_locked_ether_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_5_16_locked_ether_sol__0.txt @@ -1,5 +1,10 @@ Contract locking ether found: - Contract OnlyLocked (tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol#26) has payable functions: + Contract OnlyLocked (tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol#37) has payable functions: + - Locked.receive() (tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol#4-6) + But does not have a function to withdraw the ether + +Contract locking ether found: + Contract UnlockedAssembly (tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol#27-35) has payable functions: - Locked.receive() (tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol#4-6) But does not have a function to withdraw the ether diff --git a/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_6_11_locked_ether_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_6_11_locked_ether_sol__0.txt index 212015c29..079104879 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_6_11_locked_ether_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_6_11_locked_ether_sol__0.txt @@ -1,5 +1,5 @@ Contract locking ether found: - Contract OnlyLocked (tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol#26) has payable functions: + Contract OnlyLocked (tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol#36) has payable functions: - Locked.receive_eth() (tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol#4-6) But does not have a function to withdraw the ether diff --git a/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_7_6_locked_ether_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_7_6_locked_ether_sol__0.txt index 8b6ddfa59..14835871f 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_7_6_locked_ether_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_LockedEther_0_7_6_locked_ether_sol__0.txt @@ -1,5 +1,5 @@ Contract locking ether found: - Contract OnlyLocked (tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol#26) has payable functions: + Contract OnlyLocked (tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol#36) has payable functions: - Locked.receive_eth() (tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol#4-6) But does not have a function to withdraw the ether diff --git a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_4_25_naming_convention_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_4_25_naming_convention_sol__0.txt index 0419c1b9a..ed4177ca1 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_4_25_naming_convention_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_4_25_naming_convention_sol__0.txt @@ -18,10 +18,10 @@ Parameter T.test(uint256,uint256)._used (tests/e2e/detectors/test_data/naming-co Variable T._myPublicVar (tests/e2e/detectors/test_data/naming-convention/0.4.25/naming_convention.sol#56) is not in mixedCase -Event namingevent_(uint256) (tests/e2e/detectors/test_data/naming-convention/0.4.25/naming_convention.sol#23) is not in CapWords - Variable T.O (tests/e2e/detectors/test_data/naming-convention/0.4.25/naming_convention.sol#68) is single letter l, O, or I, which should not be used +Event naming.event_(uint256) (tests/e2e/detectors/test_data/naming-convention/0.4.25/naming_convention.sol#23) is not in CapWords + Modifier naming.CantDo() (tests/e2e/detectors/test_data/naming-convention/0.4.25/naming_convention.sol#41-43) is not in mixedCase Function naming.GetOne() (tests/e2e/detectors/test_data/naming-convention/0.4.25/naming_convention.sol#30-33) is not in mixedCase diff --git a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_5_16_naming_convention_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_5_16_naming_convention_sol__0.txt index c5fd1f30f..35c11193f 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_5_16_naming_convention_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_5_16_naming_convention_sol__0.txt @@ -18,10 +18,10 @@ Parameter T.test(uint256,uint256)._used (tests/e2e/detectors/test_data/naming-co Variable T._myPublicVar (tests/e2e/detectors/test_data/naming-convention/0.5.16/naming_convention.sol#56) is not in mixedCase -Event namingevent_(uint256) (tests/e2e/detectors/test_data/naming-convention/0.5.16/naming_convention.sol#23) is not in CapWords - Variable T.O (tests/e2e/detectors/test_data/naming-convention/0.5.16/naming_convention.sol#68) is single letter l, O, or I, which should not be used +Event naming.event_(uint256) (tests/e2e/detectors/test_data/naming-convention/0.5.16/naming_convention.sol#23) is not in CapWords + Modifier naming.CantDo() (tests/e2e/detectors/test_data/naming-convention/0.5.16/naming_convention.sol#41-43) is not in mixedCase Function naming.GetOne() (tests/e2e/detectors/test_data/naming-convention/0.5.16/naming_convention.sol#30-33) is not in mixedCase diff --git a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_6_11_naming_convention_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_6_11_naming_convention_sol__0.txt index 1bbe28843..f692e211b 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_6_11_naming_convention_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_6_11_naming_convention_sol__0.txt @@ -18,10 +18,10 @@ Parameter T.test(uint256,uint256)._used (tests/e2e/detectors/test_data/naming-co Variable T._myPublicVar (tests/e2e/detectors/test_data/naming-convention/0.6.11/naming_convention.sol#56) is not in mixedCase -Event namingevent_(uint256) (tests/e2e/detectors/test_data/naming-convention/0.6.11/naming_convention.sol#23) is not in CapWords - Variable T.O (tests/e2e/detectors/test_data/naming-convention/0.6.11/naming_convention.sol#68) is single letter l, O, or I, which should not be used +Event naming.event_(uint256) (tests/e2e/detectors/test_data/naming-convention/0.6.11/naming_convention.sol#23) is not in CapWords + Modifier naming.CantDo() (tests/e2e/detectors/test_data/naming-convention/0.6.11/naming_convention.sol#41-43) is not in mixedCase Function naming.GetOne() (tests/e2e/detectors/test_data/naming-convention/0.6.11/naming_convention.sol#30-33) is not in mixedCase diff --git a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_7_6_naming_convention_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_7_6_naming_convention_sol__0.txt index 5f560ba51..af17cabe8 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_7_6_naming_convention_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_7_6_naming_convention_sol__0.txt @@ -18,10 +18,10 @@ Parameter T.test(uint256,uint256)._used (tests/e2e/detectors/test_data/naming-co Variable T._myPublicVar (tests/e2e/detectors/test_data/naming-convention/0.7.6/naming_convention.sol#56) is not in mixedCase -Event namingevent_(uint256) (tests/e2e/detectors/test_data/naming-convention/0.7.6/naming_convention.sol#23) is not in CapWords - Variable T.O (tests/e2e/detectors/test_data/naming-convention/0.7.6/naming_convention.sol#68) is single letter l, O, or I, which should not be used +Event naming.event_(uint256) (tests/e2e/detectors/test_data/naming-convention/0.7.6/naming_convention.sol#23) is not in CapWords + Modifier naming.CantDo() (tests/e2e/detectors/test_data/naming-convention/0.7.6/naming_convention.sol#41-43) is not in mixedCase Function naming.GetOne() (tests/e2e/detectors/test_data/naming-convention/0.7.6/naming_convention.sol#30-33) is not in mixedCase diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_4_25_erc20_indexed_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_4_25_erc20_indexed_sol__0.txt index a571ef77a..b4097bd10 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_4_25_erc20_indexed_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_4_25_erc20_indexed_sol__0.txt @@ -1,8 +1,8 @@ -ERC20 event IERC20BadTransfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.4.25/erc20_indexed.sol#19)does not index parameter to +ERC20 event IERC20Bad.Approval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.4.25/erc20_indexed.sol#20)does not index parameter owner -ERC20 event IERC20BadApproval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.4.25/erc20_indexed.sol#20)does not index parameter owner +ERC20 event IERC20Bad.Transfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.4.25/erc20_indexed.sol#19)does not index parameter from -ERC20 event IERC20BadTransfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.4.25/erc20_indexed.sol#19)does not index parameter from +ERC20 event IERC20Bad.Approval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.4.25/erc20_indexed.sol#20)does not index parameter spender -ERC20 event IERC20BadApproval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.4.25/erc20_indexed.sol#20)does not index parameter spender +ERC20 event IERC20Bad.Transfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.4.25/erc20_indexed.sol#19)does not index parameter to diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_5_16_erc20_indexed_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_5_16_erc20_indexed_sol__0.txt index 09171d188..77ab9c2bd 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_5_16_erc20_indexed_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_5_16_erc20_indexed_sol__0.txt @@ -1,8 +1,8 @@ -ERC20 event IERC20BadTransfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.5.16/erc20_indexed.sol#19)does not index parameter to +ERC20 event IERC20Bad.Approval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.5.16/erc20_indexed.sol#20)does not index parameter owner -ERC20 event IERC20BadApproval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.5.16/erc20_indexed.sol#20)does not index parameter owner +ERC20 event IERC20Bad.Transfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.5.16/erc20_indexed.sol#19)does not index parameter from -ERC20 event IERC20BadTransfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.5.16/erc20_indexed.sol#19)does not index parameter from +ERC20 event IERC20Bad.Approval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.5.16/erc20_indexed.sol#20)does not index parameter spender -ERC20 event IERC20BadApproval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.5.16/erc20_indexed.sol#20)does not index parameter spender +ERC20 event IERC20Bad.Transfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.5.16/erc20_indexed.sol#19)does not index parameter to diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_6_11_erc20_indexed_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_6_11_erc20_indexed_sol__0.txt index c0cbe9270..9e11f44c8 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_6_11_erc20_indexed_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_6_11_erc20_indexed_sol__0.txt @@ -1,8 +1,8 @@ -ERC20 event IERC20BadTransfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.6.11/erc20_indexed.sol#19)does not index parameter to +ERC20 event IERC20Bad.Approval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.6.11/erc20_indexed.sol#20)does not index parameter owner -ERC20 event IERC20BadApproval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.6.11/erc20_indexed.sol#20)does not index parameter owner +ERC20 event IERC20Bad.Transfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.6.11/erc20_indexed.sol#19)does not index parameter from -ERC20 event IERC20BadTransfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.6.11/erc20_indexed.sol#19)does not index parameter from +ERC20 event IERC20Bad.Approval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.6.11/erc20_indexed.sol#20)does not index parameter spender -ERC20 event IERC20BadApproval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.6.11/erc20_indexed.sol#20)does not index parameter spender +ERC20 event IERC20Bad.Transfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.6.11/erc20_indexed.sol#19)does not index parameter to diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_7_6_erc20_indexed_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_7_6_erc20_indexed_sol__0.txt index 532a8b6cd..8083e8f71 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_7_6_erc20_indexed_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnindexedERC20EventParameters_0_7_6_erc20_indexed_sol__0.txt @@ -1,8 +1,8 @@ -ERC20 event IERC20BadTransfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.7.6/erc20_indexed.sol#19)does not index parameter to +ERC20 event IERC20Bad.Approval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.7.6/erc20_indexed.sol#20)does not index parameter owner -ERC20 event IERC20BadApproval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.7.6/erc20_indexed.sol#20)does not index parameter owner +ERC20 event IERC20Bad.Transfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.7.6/erc20_indexed.sol#19)does not index parameter from -ERC20 event IERC20BadTransfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.7.6/erc20_indexed.sol#19)does not index parameter from +ERC20 event IERC20Bad.Approval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.7.6/erc20_indexed.sol#20)does not index parameter spender -ERC20 event IERC20BadApproval(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.7.6/erc20_indexed.sol#20)does not index parameter spender +ERC20 event IERC20Bad.Transfer(address,address,uint256) (tests/e2e/detectors/test_data/erc20-indexed/0.7.6/erc20_indexed.sol#19)does not index parameter to diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UninitializedLocalVars_0_6_11_uninitialized_local_variable_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UninitializedLocalVars_0_6_11_uninitialized_local_variable_sol__0.txt index 8e5dc65e8..7e5fa9559 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UninitializedLocalVars_0_6_11_uninitialized_local_variable_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UninitializedLocalVars_0_6_11_uninitialized_local_variable_sol__0.txt @@ -1,2 +1,2 @@ -Uninitialized.func().uint_not_init (tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol#4) is a local variable never initialized +Uninitialized.func().uint_not_init (tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol#8) is a local variable never initialized diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UninitializedLocalVars_0_7_6_uninitialized_local_variable_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UninitializedLocalVars_0_7_6_uninitialized_local_variable_sol__0.txt index 495859ec1..7bf1564d7 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UninitializedLocalVars_0_7_6_uninitialized_local_variable_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UninitializedLocalVars_0_7_6_uninitialized_local_variable_sol__0.txt @@ -1,2 +1,2 @@ -Uninitialized.func().uint_not_init (tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol#4) is a local variable never initialized +Uninitialized.func().uint_not_init (tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol#8) is a local variable never initialized diff --git a/tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol b/tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol index 304af6a48..c2707601a 100644 --- a/tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol +++ b/tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol @@ -48,4 +48,25 @@ contract D { } +} + +contract E { + uint[1] public x; // storage + uint[1] public y; // storage + + function f() public { + uint[1] memory temp; + setByValue(temp, x); // can set temp, but cannot set x + setByRef(temp, y); // can set temp and y + } + + function setByValue(uint[1] memory arr, uint[1] memory arr2) internal { + arr[0] = 1; + arr2[0] = 2; + } + + function setByRef(uint[1] memory arr, uint[1] storage arr2) internal { + arr[0] = 2; + arr2[0] = 3; + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol-0.4.25.zip b/tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol-0.4.25.zip index d9daf8f4d..96bdaa0f8 100644 Binary files a/tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol-0.4.25.zip and b/tests/e2e/detectors/test_data/array-by-reference/0.4.25/array_by_reference.sol-0.4.25.zip differ diff --git a/tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol b/tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol index c477c6b0e..1a4d4cfe3 100644 --- a/tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol +++ b/tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol @@ -48,4 +48,25 @@ contract D { } +} + +contract E { + uint[1] public x; // storage + uint[1] public y; // storage + + function f() public { + uint[1] memory temp; + setByValue(temp, x); // can set temp, but cannot set x + setByRef(temp, y); // can set temp and y + } + + function setByValue(uint[1] memory arr, uint[1] memory arr2) internal { + arr[0] = 1; + arr2[0] = 2; + } + + function setByRef(uint[1] memory arr, uint[1] storage arr2) internal { + arr[0] = 2; + arr2[0] = 3; + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol-0.5.16.zip b/tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol-0.5.16.zip index 73286c85a..406a93f73 100644 Binary files a/tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol-0.5.16.zip and b/tests/e2e/detectors/test_data/array-by-reference/0.5.16/array_by_reference.sol-0.5.16.zip differ diff --git a/tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol b/tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol index c477c6b0e..1a4d4cfe3 100644 --- a/tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol +++ b/tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol @@ -48,4 +48,25 @@ contract D { } +} + +contract E { + uint[1] public x; // storage + uint[1] public y; // storage + + function f() public { + uint[1] memory temp; + setByValue(temp, x); // can set temp, but cannot set x + setByRef(temp, y); // can set temp and y + } + + function setByValue(uint[1] memory arr, uint[1] memory arr2) internal { + arr[0] = 1; + arr2[0] = 2; + } + + function setByRef(uint[1] memory arr, uint[1] storage arr2) internal { + arr[0] = 2; + arr2[0] = 3; + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol-0.6.11.zip b/tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol-0.6.11.zip index 8a48717bc..7d9a573d1 100644 Binary files a/tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol-0.6.11.zip and b/tests/e2e/detectors/test_data/array-by-reference/0.6.11/array_by_reference.sol-0.6.11.zip differ diff --git a/tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol b/tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol index c477c6b0e..1a4d4cfe3 100644 --- a/tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol +++ b/tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol @@ -48,4 +48,25 @@ contract D { } +} + +contract E { + uint[1] public x; // storage + uint[1] public y; // storage + + function f() public { + uint[1] memory temp; + setByValue(temp, x); // can set temp, but cannot set x + setByRef(temp, y); // can set temp and y + } + + function setByValue(uint[1] memory arr, uint[1] memory arr2) internal { + arr[0] = 1; + arr2[0] = 2; + } + + function setByRef(uint[1] memory arr, uint[1] storage arr2) internal { + arr[0] = 2; + arr2[0] = 3; + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol-0.7.6.zip b/tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol-0.7.6.zip index 776064577..a57470fbe 100644 Binary files a/tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol-0.7.6.zip and b/tests/e2e/detectors/test_data/array-by-reference/0.7.6/array_by_reference.sol-0.7.6.zip differ diff --git a/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol b/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol new file mode 100644 index 000000000..704d6bed9 --- /dev/null +++ b/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol @@ -0,0 +1,171 @@ +pragma solidity 0.8.17; + +contract CacheArrayLength +{ + struct S + { + uint s; + } + + S[] array; + S[] array2; + + function h() external + { + + } + + function g() internal + { + this.h(); + } + + function h_view() external view + { + + } + + function g_view() internal view + { + this.h_view(); + } + + function f() public + { + // array accessed but length doesn't change + for (uint i = 0; i < array.length; i++) // warning should appear + { + array[i] = S(0); + } + + // array.length doesn't change, but array.length not used in loop condition + for (uint i = array.length; i >= 0; i--) + { + + } + + // array.length changes in the inner loop + for (uint i = 0; i < array.length; i++) + { + for (uint j = i; j < 2 * i; j++) + array.push(S(j)); + } + + // array.length changes + for (uint i = 0; i < array.length; i++) + { + array.pop(); + } + + // array.length changes + for (uint i = 0; i < array.length; i++) + { + delete array; + } + + // array.length doesn't change despite using delete + for (uint i = 0; i < array.length; i++) // warning should appear + { + delete array[i]; + } + + // array.length changes; push used in more complex expression + for (uint i = 0; i < array.length; i++) + { + array.push() = S(i); + } + + // array.length doesn't change + for (uint i = 0; i < array.length; i++) // warning should appear + { + array2.pop(); + array2.push(); + array2.push(S(i)); + delete array2; + delete array[0]; + } + + // array.length changes; array2.length doesn't change + for (uint i = 0; i < 7; i++) + { + for (uint j = i; j < array.length; j++) + { + for (uint k = 0; k < j; k++) + { + + } + + for (uint k = 0; k < array2.length; k++) // warning should appear + { + array.pop(); + } + } + } + + // array.length doesn't change; array2.length changes + for (uint i = 0; i < 7; i++) + { + for (uint j = i; j < array.length; j++) // warning should appear + { + for (uint k = 0; k < j; k++) + { + + } + + for (uint k = 0; k < array2.length; k++) + { + array2.pop(); + } + } + } + + // none of array.length and array2.length changes + for (uint i = 0; i < 7; i++) + { + for (uint j = i; j < array.length; j++) // warning should appear + { + for (uint k = 0; k < j; k++) + { + + } + + for (uint k = 0; k < array2.length; k++) // warning should appear + { + + } + } + } + + S[] memory array3; + + // array3 not modified, but it's not a storage array + for (uint i = 0; i < array3.length; i++) + { + + } + + // array not modified, but it may potentially change in an internal function call + for (uint i = 0; i < array.length; i++) + { + g(); + } + + // array not modified, but it may potentially change in an external function call + for (uint i = 0; i < array.length; i++) + { + this.h(); + } + + // array not modified and it cannot be changed in a function call since g_view is a view function + for (uint i = 0; i < array.length; i++) // warning should appear + { + g_view(); + } + + // array not modified and it cannot be changed in a function call since h_view is a view function + for (uint i = 0; i < array.length; i++) // warning should appear + { + this.h_view(); + } + } +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol-0.8.17.zip b/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol-0.8.17.zip new file mode 100644 index 000000000..dafdfc431 Binary files /dev/null and b/tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol-0.8.17.zip differ diff --git a/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol b/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol new file mode 100644 index 000000000..5b173e7b5 --- /dev/null +++ b/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol @@ -0,0 +1,94 @@ +pragma solidity 0.8.17; + +struct S1 +{ + uint __; +} + +struct S2 +{ + uint128 __; +} + +enum E1 +{ + A, + B +} + +enum E2 +{ + A, + B +} + +contract C0 +{ + +} + +contract C1 is C0 +{ + +} + +contract C2 is C1 +{ + +} + +contract C3 +{ + +} + +type custom_uint is uint248; +type custom_int is int248; + +library L +{ + function f0(C0) public pure {} + function f1(bool) public pure {} + function f2(string memory) public pure {} + function f3(bytes memory) public pure {} + function f4(uint248) public pure {} + function f5(int248) public pure {} + function f6(address) public pure {} + function f7(bytes17) public pure {} + function f8(S1 memory) public pure {} + function f9(E1) public pure {} + function f10(mapping(int => uint) storage) public pure {} + function f11(string[] memory) public pure {} + function f12(bytes[][][] memory) public pure {} + function f13(custom_uint) public pure {} +} + +// the following statements are correct +using L for C2; +using L for bool; +using L for string; +using L for bytes; +using L for uint240; +using L for int16; +using L for address; +using L for bytes16; +using L for S1; +using L for E1; +using L for mapping(int => uint); +using L for string[]; +using L for bytes[][][]; +using L for custom_uint; + +// the following statements are incorrect +using L for C3; +using L for bytes17[]; +using L for uint; +using L for int; +using L for bytes18; +using L for S2; +using L for E2; +using L for mapping(int => uint128); +using L for mapping(int128 => uint); +using L for string[][]; +using L for bytes[][]; +using L for custom_int; diff --git a/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol-0.8.17.zip b/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol-0.8.17.zip new file mode 100644 index 000000000..9afc56d39 Binary files /dev/null and b/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol-0.8.17.zip differ diff --git a/tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol b/tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol index 65942ed2e..f3be911be 100644 --- a/tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol +++ b/tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol @@ -23,4 +23,15 @@ contract Unlocked is Locked, Send{ } +// Still reported because solidity < 0.6.0 doesn't have assembly in the AST +contract UnlockedAssembly is Locked{ + + function withdraw() public { + assembly { + let success := call(gas(), caller(),100,0,0,0,0) + } + } + +} + contract OnlyLocked is Locked{ } diff --git a/tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol-0.4.25.zip b/tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol-0.4.25.zip index b6092ecdb..e3e6f6c21 100644 Binary files a/tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol-0.4.25.zip and b/tests/e2e/detectors/test_data/locked-ether/0.4.25/locked_ether.sol-0.4.25.zip differ diff --git a/tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol b/tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol index 0269ce855..e5671b7ad 100644 --- a/tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol +++ b/tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol @@ -23,4 +23,15 @@ contract Unlocked is Locked, Send{ } +// Still reported because solidity < 0.6.0 doesn't have assembly in the AST +contract UnlockedAssembly is Locked{ + + function withdraw() public { + assembly { + let success := call(gas(), caller(),100,0,0,0,0) + } + } + +} + contract OnlyLocked is Locked{ } diff --git a/tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol-0.5.16.zip b/tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol-0.5.16.zip index 88255d730..5dfba9e3f 100644 Binary files a/tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol-0.5.16.zip and b/tests/e2e/detectors/test_data/locked-ether/0.5.16/locked_ether.sol-0.5.16.zip differ diff --git a/tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol b/tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol index 7f02028e7..aef9ca6e7 100644 --- a/tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol +++ b/tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol @@ -23,4 +23,14 @@ contract Unlocked is Locked, Send{ } +contract UnlockedAssembly is Locked{ + + function withdraw() public { + assembly { + let success := call(gas(), caller(),100,0,0,0,0) + } + } + +} + contract OnlyLocked is Locked{ } diff --git a/tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol-0.6.11.zip b/tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol-0.6.11.zip index 2408eeb82..48b675d9a 100644 Binary files a/tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol-0.6.11.zip and b/tests/e2e/detectors/test_data/locked-ether/0.6.11/locked_ether.sol-0.6.11.zip differ diff --git a/tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol b/tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol index 7f02028e7..aef9ca6e7 100644 --- a/tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol +++ b/tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol @@ -23,4 +23,14 @@ contract Unlocked is Locked, Send{ } +contract UnlockedAssembly is Locked{ + + function withdraw() public { + assembly { + let success := call(gas(), caller(),100,0,0,0,0) + } + } + +} + contract OnlyLocked is Locked{ } diff --git a/tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol-0.7.6.zip b/tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol-0.7.6.zip index ecf7e7944..8d339e3c2 100644 Binary files a/tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol-0.7.6.zip and b/tests/e2e/detectors/test_data/locked-ether/0.7.6/locked_ether.sol-0.7.6.zip differ diff --git a/tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol b/tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol index 37d4650e2..22e583ec2 100644 --- a/tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol +++ b/tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol @@ -1,10 +1,22 @@ +interface I { + function a() external; +} + contract Uninitialized{ function func() external returns(uint){ uint uint_not_init; uint uint_init = 1; return uint_not_init + uint_init; - } + } + + function func_try_catch(I i) external returns(uint) { + try i.a() { + return 1; + } catch (bytes memory data) { + data; + } + } function noreportfor() public { for(uint i; i < 6; i++) { diff --git a/tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol-0.6.11.zip b/tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol-0.6.11.zip index d1ac3a5ab..d5b120306 100644 Binary files a/tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol-0.6.11.zip and b/tests/e2e/detectors/test_data/uninitialized-local/0.6.11/uninitialized_local_variable.sol-0.6.11.zip differ diff --git a/tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol b/tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol index 37d4650e2..22e583ec2 100644 --- a/tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol +++ b/tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol @@ -1,10 +1,22 @@ +interface I { + function a() external; +} + contract Uninitialized{ function func() external returns(uint){ uint uint_not_init; uint uint_init = 1; return uint_not_init + uint_init; - } + } + + function func_try_catch(I i) external returns(uint) { + try i.a() { + return 1; + } catch (bytes memory data) { + data; + } + } function noreportfor() public { for(uint i; i < 6; i++) { diff --git a/tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol-0.7.6.zip b/tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol-0.7.6.zip index eaef1050d..30b474ac2 100644 Binary files a/tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol-0.7.6.zip and b/tests/e2e/detectors/test_data/uninitialized-local/0.7.6/uninitialized_local_variable.sol-0.7.6.zip differ diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index b3947a62a..6fc04e4e1 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -56,7 +56,7 @@ def id_test(test_item: Test): return f"{test_item.detector.__name__}-{test_item.solc_ver}-{test_item.test_file}" -ALL_TEST_OBJECTS = [ +ALL_TESTS = [ Test( all_detectors.UninitializedFunctionPtrsConstructor, "uninitialized_function_ptr_constructor.sol", @@ -1639,6 +1639,16 @@ ALL_TEST_OBJECTS = [ "LowCyclomaticComplexity.sol", "0.8.16", ), + Test( + all_detectors.CacheArrayLength, + "CacheArrayLength.sol", + "0.8.17", + ), + Test( + all_detectors.IncorrectUsingFor, + "IncorrectUsingForTopLevel.sol", + "0.8.17", + ), Test( all_detectors.EncodePackedCollision, "encode_packed_collision.sol", @@ -1651,7 +1661,7 @@ GENERIC_PATH = "/GENERIC_PATH" TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" # pylint: disable=too-many-locals -@pytest.mark.parametrize("test_item", ALL_TEST_OBJECTS, ids=id_test) +@pytest.mark.parametrize("test_item", ALL_TESTS, ids=id_test) def test_detector(test_item: Test, snapshot): test_dir_path = Path( TEST_DATA_DIR, @@ -1699,5 +1709,5 @@ if __name__ == "__main__": "To generate the zip artifacts run\n\tpython tests/e2e/tests/test_detectors.py --compile" ) elif sys.argv[1] == "--compile": - for next_test in ALL_TEST_OBJECTS: + for next_test in ALL_TESTS: _generate_compile(next_test, skip_existing=True) diff --git a/tests/e2e/solc_parsing/test_ast_parsing.py b/tests/e2e/solc_parsing/test_ast_parsing.py index a561343de..307e6736f 100644 --- a/tests/e2e/solc_parsing/test_ast_parsing.py +++ b/tests/e2e/solc_parsing/test_ast_parsing.py @@ -308,6 +308,7 @@ ALL_TESTS = [ Test("units_and_global_variables-0.8.0.sol", VERSIONS_08), Test("units_and_global_variables-0.8.4.sol", make_version(8, 4, 6)), Test("units_and_global_variables-0.8.7.sol", make_version(8, 7, 9)), + Test("global_variables-0.8.18.sol", make_version(8, 18, 18)), Test( "push-all.sol", ALL_VERSIONS, @@ -453,6 +454,11 @@ ALL_TESTS = [ Test("complex_imports/import_aliases_issue_1319/test.sol", ["0.5.12"]), Test("yul-state-constant-access.sol", ["0.8.16"]), Test("negate-unary-element.sol", ["0.8.16"]), + Test( + "assembly-functions.sol", + ["0.6.9", "0.7.6", "0.8.16"], + ), + Test("user_defined_operators-0.8.19.sol", ["0.8.19"]), ] # create the output folder if needed try: @@ -495,12 +501,9 @@ class TestASTParsing: actual = generate_output(sl) - try: - with open(expected, "r", encoding="utf8") as f: - expected = json.load(f) - except OSError: - pytest.xfail("the file for this test was not generated") - raise + assert os.path.isfile(expected), f"Expected file {expected} does not exist" + with open(expected, "r", encoding="utf8") as f: + expected = json.load(f) diff = DeepDiff(expected, actual, ignore_order=True, verbose_level=2, view="tree") if diff: diff --git a/tests/e2e/solc_parsing/test_data/assembly-functions.sol b/tests/e2e/solc_parsing/test_data/assembly-functions.sol new file mode 100644 index 000000000..224e16bab --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/assembly-functions.sol @@ -0,0 +1,12 @@ +contract A { + function foo() public { + assembly { + function f() { function z() { function x() { g() } x() } z() } + function w() { function a() {} function b() { a() } b() } + function g() { + f() + } + g() + } + } +} diff --git a/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.6.9-compact.zip b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.6.9-compact.zip new file mode 100644 index 000000000..8389eb6f5 Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.6.9-compact.zip differ diff --git a/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.6.9-legacy.zip b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.6.9-legacy.zip new file mode 100644 index 000000000..a05a1cd47 Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.6.9-legacy.zip differ diff --git a/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.7.6-compact.zip b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.7.6-compact.zip new file mode 100644 index 000000000..b21bdae6f Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.7.6-compact.zip differ diff --git a/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.7.6-legacy.zip b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.7.6-legacy.zip new file mode 100644 index 000000000..c1285733d Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.7.6-legacy.zip differ diff --git a/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.8.16-compact.zip b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.8.16-compact.zip new file mode 100644 index 000000000..a2b78d7b0 Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/assembly-functions.sol-0.8.16-compact.zip differ diff --git a/tests/e2e/solc_parsing/test_data/compile/global_variables-0.8.18.sol-0.8.18-compact.zip b/tests/e2e/solc_parsing/test_data/compile/global_variables-0.8.18.sol-0.8.18-compact.zip new file mode 100644 index 000000000..04dde0da9 Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/global_variables-0.8.18.sol-0.8.18-compact.zip differ diff --git a/tests/e2e/solc_parsing/test_data/compile/user_defined_operators-0.8.19.sol-0.8.19-compact.zip b/tests/e2e/solc_parsing/test_data/compile/user_defined_operators-0.8.19.sol-0.8.19-compact.zip new file mode 100644 index 000000000..7159a1486 Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/user_defined_operators-0.8.19.sol-0.8.19-compact.zip differ diff --git a/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.6.9-compact.json b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.6.9-compact.json new file mode 100644 index 000000000..a48faa23d --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.6.9-compact.json @@ -0,0 +1,12 @@ +{ + "A": { + "foo()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: INLINE ASM 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f.z()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f.z.x()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.w()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.w.a()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n}\n", + "foo.asm_0.w.b()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.g()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.6.9-legacy.json b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.6.9-legacy.json new file mode 100644 index 000000000..09c0a51f7 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.6.9-legacy.json @@ -0,0 +1,5 @@ +{ + "A": { + "foo()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: INLINE ASM 1\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.7.6-compact.json b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.7.6-compact.json new file mode 100644 index 000000000..a48faa23d --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.7.6-compact.json @@ -0,0 +1,12 @@ +{ + "A": { + "foo()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: INLINE ASM 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f.z()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f.z.x()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.w()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.w.a()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n}\n", + "foo.asm_0.w.b()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.g()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.7.6-legacy.json b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.7.6-legacy.json new file mode 100644 index 000000000..09c0a51f7 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.7.6-legacy.json @@ -0,0 +1,5 @@ +{ + "A": { + "foo()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: INLINE ASM 1\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.8.16-compact.json b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.8.16-compact.json new file mode 100644 index 000000000..a48faa23d --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/assembly-functions.sol-0.8.16-compact.json @@ -0,0 +1,12 @@ +{ + "A": { + "foo()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: INLINE ASM 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f.z()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.f.z.x()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.w()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.w.a()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n}\n", + "foo.asm_0.w.b()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n", + "foo.asm_0.g()": "digraph{\n0[label=\"Node Type: INLINE ASM 0\n\"];\n0->1;\n1[label=\"Node Type: ENTRY_POINT 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/expected/global_variables-0.8.18.sol-0.8.18-compact.json b/tests/e2e/solc_parsing/test_data/expected/global_variables-0.8.18.sol-0.8.18-compact.json new file mode 100644 index 000000000..b74cf7115 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/global_variables-0.8.18.sol-0.8.18-compact.json @@ -0,0 +1,6 @@ +{ + "C": { + "f()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "g()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: INLINE ASM 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/expected/user_defined_operators-0.8.19.sol-0.8.19-compact.json b/tests/e2e/solc_parsing/test_data/expected/user_defined_operators-0.8.19.sol-0.8.19-compact.json new file mode 100644 index 000000000..bee7819a6 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/user_defined_operators-0.8.19.sol-0.8.19-compact.json @@ -0,0 +1,13 @@ +{ + "Lib": { + "f(Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n}\n" + }, + "T": { + "add_function_call(Int,Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: RETURN 2\n\"];\n}\n", + "add_op(Int,Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "lib_call(Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "neg_usertype(Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: RETURN 2\n\"];\n}\n", + "neg_int(int256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "eq_op(Int,Int)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/expected/yul-top-level-0.8.0.sol-0.8.0-compact.json b/tests/e2e/solc_parsing/test_data/expected/yul-top-level-0.8.0.sol-0.8.0-compact.json new file mode 100644 index 000000000..f9655dff5 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/expected/yul-top-level-0.8.0.sol-0.8.0-compact.json @@ -0,0 +1,5 @@ +{ + "Test": { + "test()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: EXPRESSION 1\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/global_variables-0.8.18.sol b/tests/e2e/solc_parsing/test_data/global_variables-0.8.18.sol new file mode 100644 index 000000000..f21ae5d8f --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/global_variables-0.8.18.sol @@ -0,0 +1,11 @@ +contract C { + function f() public view returns (uint256) { + return block.prevrandao; + } + + function g() public view returns (uint256 ret) { + assembly { + ret := prevrandao() + } + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/user_defined_operators-0.8.19.sol b/tests/e2e/solc_parsing/test_data/user_defined_operators-0.8.19.sol new file mode 100644 index 000000000..e4df845fb --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/user_defined_operators-0.8.19.sol @@ -0,0 +1,48 @@ +pragma solidity ^0.8.19; + +type Int is int; +using {add as +, eq as ==, add, neg as -, Lib.f} for Int global; + +function add(Int a, Int b) pure returns (Int) { + return Int.wrap(Int.unwrap(a) + Int.unwrap(b)); +} + +function eq(Int a, Int b) pure returns (bool) { + return true; +} + +function neg(Int a) pure returns (Int) { + return a; +} + +library Lib { + function f(Int r) internal {} +} + +contract T { + function add_function_call(Int b, Int c) public returns(Int) { + Int res = add(b,c); + return res; + } + + function add_op(Int b, Int c) public returns(Int) { + return b + c; + } + + function lib_call(Int b) public { + return b.f(); + } + + function neg_usertype(Int b) public returns(Int) { + Int res = -b; + return res; + } + + function neg_int(int b) public returns(int) { + return -b; + } + + function eq_op(Int b, Int c) public returns(bool) { + return b == c; + } +} diff --git a/tests/tools/check-erc/erc20.sol b/tests/tools/check_erc/erc20.sol similarity index 100% rename from tests/tools/check-erc/erc20.sol rename to tests/tools/check_erc/erc20.sol diff --git a/tests/tools/check-erc/test_1.txt b/tests/tools/check_erc/test_1.txt similarity index 100% rename from tests/tools/check-erc/test_1.txt rename to tests/tools/check_erc/test_1.txt diff --git a/tests/tools/check-kspec/safeAdd/safeAdd.sol b/tests/tools/check_kspec/safeAdd/safeAdd.sol similarity index 100% rename from tests/tools/check-kspec/safeAdd/safeAdd.sol rename to tests/tools/check_kspec/safeAdd/safeAdd.sol diff --git a/tests/tools/check-kspec/safeAdd/spec.md b/tests/tools/check_kspec/safeAdd/spec.md similarity index 100% rename from tests/tools/check-kspec/safeAdd/spec.md rename to tests/tools/check_kspec/safeAdd/spec.md diff --git a/tests/tools/check-kspec/test_1.txt b/tests/tools/check_kspec/test_1.txt similarity index 100% rename from tests/tools/check-kspec/test_1.txt rename to tests/tools/check_kspec/test_1.txt diff --git a/tests/tools/check-upgradeability/contractV1.sol b/tests/tools/check_upgradeability/contractV1.sol similarity index 100% rename from tests/tools/check-upgradeability/contractV1.sol rename to tests/tools/check_upgradeability/contractV1.sol diff --git a/tests/tools/check-upgradeability/contractV1_struct.sol b/tests/tools/check_upgradeability/contractV1_struct.sol similarity index 100% rename from tests/tools/check-upgradeability/contractV1_struct.sol rename to tests/tools/check_upgradeability/contractV1_struct.sol diff --git a/tests/tools/check-upgradeability/contractV2.sol b/tests/tools/check_upgradeability/contractV2.sol similarity index 100% rename from tests/tools/check-upgradeability/contractV2.sol rename to tests/tools/check_upgradeability/contractV2.sol diff --git a/tests/tools/check-upgradeability/contractV2_bug.sol b/tests/tools/check_upgradeability/contractV2_bug.sol similarity index 100% rename from tests/tools/check-upgradeability/contractV2_bug.sol rename to tests/tools/check_upgradeability/contractV2_bug.sol diff --git a/tests/tools/check-upgradeability/contractV2_bug2.sol b/tests/tools/check_upgradeability/contractV2_bug2.sol similarity index 100% rename from tests/tools/check-upgradeability/contractV2_bug2.sol rename to tests/tools/check_upgradeability/contractV2_bug2.sol diff --git a/tests/tools/check-upgradeability/contractV2_struct.sol b/tests/tools/check_upgradeability/contractV2_struct.sol similarity index 100% rename from tests/tools/check-upgradeability/contractV2_struct.sol rename to tests/tools/check_upgradeability/contractV2_struct.sol diff --git a/tests/tools/check-upgradeability/contractV2_struct_bug.sol b/tests/tools/check_upgradeability/contractV2_struct_bug.sol similarity index 100% rename from tests/tools/check-upgradeability/contractV2_struct_bug.sol rename to tests/tools/check_upgradeability/contractV2_struct_bug.sol diff --git a/tests/tools/check-upgradeability/contract_initialization.sol b/tests/tools/check_upgradeability/contract_initialization.sol similarity index 100% rename from tests/tools/check-upgradeability/contract_initialization.sol rename to tests/tools/check_upgradeability/contract_initialization.sol diff --git a/tests/tools/check-upgradeability/contract_v1_var_init.sol b/tests/tools/check_upgradeability/contract_v1_var_init.sol similarity index 100% rename from tests/tools/check-upgradeability/contract_v1_var_init.sol rename to tests/tools/check_upgradeability/contract_v1_var_init.sol diff --git a/tests/tools/check-upgradeability/contract_v2_constant.sol b/tests/tools/check_upgradeability/contract_v2_constant.sol similarity index 100% rename from tests/tools/check-upgradeability/contract_v2_constant.sol rename to tests/tools/check_upgradeability/contract_v2_constant.sol diff --git a/tests/tools/check-upgradeability/proxy.sol b/tests/tools/check_upgradeability/proxy.sol similarity index 100% rename from tests/tools/check-upgradeability/proxy.sol rename to tests/tools/check_upgradeability/proxy.sol diff --git a/tests/tools/check-upgradeability/test_1.txt b/tests/tools/check_upgradeability/test_1.txt similarity index 100% rename from tests/tools/check-upgradeability/test_1.txt rename to tests/tools/check_upgradeability/test_1.txt diff --git a/tests/tools/check-upgradeability/test_10.txt b/tests/tools/check_upgradeability/test_10.txt similarity index 100% rename from tests/tools/check-upgradeability/test_10.txt rename to tests/tools/check_upgradeability/test_10.txt diff --git a/tests/tools/check-upgradeability/test_11.txt b/tests/tools/check_upgradeability/test_11.txt similarity index 100% rename from tests/tools/check-upgradeability/test_11.txt rename to tests/tools/check_upgradeability/test_11.txt diff --git a/tests/tools/check-upgradeability/test_12.txt b/tests/tools/check_upgradeability/test_12.txt similarity index 100% rename from tests/tools/check-upgradeability/test_12.txt rename to tests/tools/check_upgradeability/test_12.txt diff --git a/tests/tools/check-upgradeability/test_13.txt b/tests/tools/check_upgradeability/test_13.txt similarity index 100% rename from tests/tools/check-upgradeability/test_13.txt rename to tests/tools/check_upgradeability/test_13.txt diff --git a/tests/tools/check-upgradeability/test_2.txt b/tests/tools/check_upgradeability/test_2.txt similarity index 100% rename from tests/tools/check-upgradeability/test_2.txt rename to tests/tools/check_upgradeability/test_2.txt diff --git a/tests/tools/check-upgradeability/test_3.txt b/tests/tools/check_upgradeability/test_3.txt similarity index 100% rename from tests/tools/check-upgradeability/test_3.txt rename to tests/tools/check_upgradeability/test_3.txt diff --git a/tests/tools/check-upgradeability/test_4.txt b/tests/tools/check_upgradeability/test_4.txt similarity index 100% rename from tests/tools/check-upgradeability/test_4.txt rename to tests/tools/check_upgradeability/test_4.txt diff --git a/tests/tools/check-upgradeability/test_5.txt b/tests/tools/check_upgradeability/test_5.txt similarity index 100% rename from tests/tools/check-upgradeability/test_5.txt rename to tests/tools/check_upgradeability/test_5.txt diff --git a/tests/tools/check-upgradeability/test_6.txt b/tests/tools/check_upgradeability/test_6.txt similarity index 100% rename from tests/tools/check-upgradeability/test_6.txt rename to tests/tools/check_upgradeability/test_6.txt diff --git a/tests/tools/check-upgradeability/test_7.txt b/tests/tools/check_upgradeability/test_7.txt similarity index 100% rename from tests/tools/check-upgradeability/test_7.txt rename to tests/tools/check_upgradeability/test_7.txt diff --git a/tests/tools/check-upgradeability/test_8.txt b/tests/tools/check_upgradeability/test_8.txt similarity index 100% rename from tests/tools/check-upgradeability/test_8.txt rename to tests/tools/check_upgradeability/test_8.txt diff --git a/tests/tools/check-upgradeability/test_9.txt b/tests/tools/check_upgradeability/test_9.txt similarity index 100% rename from tests/tools/check-upgradeability/test_9.txt rename to tests/tools/check_upgradeability/test_9.txt diff --git a/tests/tools/read-storage/conftest.py b/tests/tools/read-storage/conftest.py new file mode 100644 index 000000000..3e2cfb400 --- /dev/null +++ b/tests/tools/read-storage/conftest.py @@ -0,0 +1,56 @@ +""" +Testing utilities for the read-storage tool +""" + +import shutil +import subprocess +from time import sleep +from typing import Generator +from dataclasses import dataclass +from web3 import Web3 +import pytest + + +@dataclass +class GanacheInstance: + def __init__(self, provider: str, eth_address: str, eth_privkey: str): + self.provider = provider + self.eth_address = eth_address + self.eth_privkey = eth_privkey + + +@pytest.fixture(scope="module", name="ganache") +def fixture_ganache() -> Generator[GanacheInstance, None, None]: + """Fixture that runs ganache""" + if not shutil.which("ganache"): + raise Exception( + "ganache was not found in PATH, you can install it with `npm install -g ganache`" + ) + + # Address #1 when ganache is run with `--wallet.seed test`, it starts with 1000 ETH + eth_address = "0xae17D2dD99e07CA3bF2571CCAcEAA9e2Aefc2Dc6" + eth_privkey = "0xe48ba530a63326818e116be262fd39ae6dcddd89da4b1f578be8afd4e8894b8d" + eth = int(1e18 * 1e6) + port = 8545 + with subprocess.Popen( + f"""ganache + --port {port} + --chain.networkId 1 + --chain.chainId 1 + --account {eth_privkey},{eth} + """.replace( + "\n", " " + ), + shell=True, + ) as p: + + sleep(3) + yield GanacheInstance(f"http://127.0.0.1:{port}", eth_address, eth_privkey) + p.kill() + p.wait() + + +@pytest.fixture(scope="module", name="web3") +def fixture_web3(ganache: GanacheInstance): + w3 = Web3(Web3.HTTPProvider(ganache.provider, request_kwargs={"timeout": 30})) + return w3 diff --git a/tests/tools/read-storage/test_data/storage_layout-0.8.10.sol b/tests/tools/read-storage/test_data/StorageLayout.sol similarity index 95% rename from tests/tools/read-storage/test_data/storage_layout-0.8.10.sol rename to tests/tools/read-storage/test_data/StorageLayout.sol index 28d1428eb..0940b6769 100644 --- a/tests/tools/read-storage/test_data/storage_layout-0.8.10.sol +++ b/tests/tools/read-storage/test_data/StorageLayout.sol @@ -1,5 +1,6 @@ +pragma solidity 0.8.10; // overwrite abi and bin: -// solc tests/storage-layout/storage_layout-0.8.10.sol --abi --bin -o tests/storage-layout --overwrite +// solc StorageLayout.sol --abi --bin --overwrite contract StorageLayout { uint248 packedUint = 1; bool packedBool = true; diff --git a/tests/tools/read-storage/test_data/TEST_unstructured_storage.json b/tests/tools/read-storage/test_data/TEST_unstructured_storage.json new file mode 100644 index 000000000..6dbdbddca --- /dev/null +++ b/tests/tools/read-storage/test_data/TEST_unstructured_storage.json @@ -0,0 +1,56 @@ +{ + "masterCopy": { + "name": "masterCopy", + "type_string": "address", + "slot": 0, + "size": 160, + "offset": 0, + "value": "0x0000000000000000000000000000000000000000", + "elems": {} + }, + "ADMIN_SLOT": { + "name": "ADMIN_SLOT", + "type_string": "address", + "slot": 7616251639890160809447714111544359812065171195189364993079081710756264753419, + "size": 160, + "offset": 0, + "value": "0xae17D2dD99e07CA3bF2571CCAcEAA9e2Aefc2Dc6", + "elems": {} + }, + "IMPLEMENTATION_SLOT": { + "name": "IMPLEMENTATION_SLOT", + "type_string": "address", + "slot": 24440054405305269366569402256811496959409073762505157381672968839269610695612, + "size": 160, + "offset": 0, + "value": "0x54006763154c764da4AF42a8c3cfc25Ea29765D5", + "elems": {} + }, + "ROLLBACK_SLOT": { + "name": "ROLLBACK_SLOT", + "type_string": "bool", + "slot": 33048860383849004559742813297059419343339852917517107368639918720169455489347, + "size": 1, + "offset": 0, + "value": true, + "elems": {} + }, + "BEACON_SLOT": { + "name": "BEACON_SLOT", + "type_string": "address", + "slot": 74152234768234802001998023604048924213078445070507226371336425913862612794704, + "size": 160, + "offset": 0, + "value": "0x54006763154c764da4AF42a8c3cfc25Ea29765D5", + "elems": {} + }, + "fallback_sload_hardcoded": { + "name": "fallback_sload_hardcoded", + "type_string": "address", + "slot": 89532207833283453166981358064394884954800891875771469636219037672473505217783, + "size": 160, + "offset": 0, + "value": "0x54006763154c764da4AF42a8c3cfc25Ea29765D5", + "elems": {} + } +} \ No newline at end of file diff --git a/tests/tools/read-storage/test_data/UnstructuredStorageLayout.abi b/tests/tools/read-storage/test_data/UnstructuredStorageLayout.abi new file mode 100644 index 000000000..9b579f254 --- /dev/null +++ b/tests/tools/read-storage/test_data/UnstructuredStorageLayout.abi @@ -0,0 +1 @@ +[{"stateMutability":"nonpayable","type":"fallback"},{"inputs":[],"name":"store","outputs":[],"stateMutability":"nonpayable","type":"function"}] \ No newline at end of file diff --git a/tests/tools/read-storage/test_data/UnstructuredStorageLayout.bin b/tests/tools/read-storage/test_data/UnstructuredStorageLayout.bin new file mode 100644 index 000000000..9f20de74c --- /dev/null +++ b/tests/tools/read-storage/test_data/UnstructuredStorageLayout.bin @@ -0,0 +1 @@ +608060405234801561001057600080fd5b5061030b806100206000396000f3fe608060405234801561001057600080fd5b506004361061002f5760003560e01c8063975057e71461009757610030565b5b600180035473ffffffffffffffffffffffffffffffffffffffff600054167fc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7543660008037600080366000845af43d806000803e816000811461009257816000f35b816000fd5b61009f6100a1565b005b60006100ab6101a8565b9050600073ffffffffffffffffffffffffffffffffffffffff168173ffffffffffffffffffffffffffffffffffffffff16146100e657600080fd5b60007f10d6a54a4754c8869d6886b5f5d7fbfa5b4522237ea5c60d11bc4e7a1ff9390b9050600033905080825560007f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b905060007354006763154c764da4af42a8c3cfc25ea29765d59050808255807fc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf75561018460016101d6565b6101a17354006763154c764da4af42a8c3cfc25ea29765d5610220565b5050505050565b6000807f10d6a54a4754c8869d6886b5f5d7fbfa5b4522237ea5c60d11bc4e7a1ff9390b9050805491505090565b806102037f4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd914360001b61025e565b60000160006101000a81548160ff02191690831515021790555050565b600060017fa3f0ad74e5423aebfd80d3ef4346578335a9a72aeaee59ff6cb3582b35133d5160001c61025291906102a1565b60001b90508181555050565b6000819050919050565b6000819050919050565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052601160045260246000fd5b60006102ac82610268565b91506102b783610268565b9250828210156102ca576102c9610272565b5b82820390509291505056fea2646970667358221220f079473c1b94744ac2818f521ccef06187a433d996633e61e51a86dfb60cc6ff64736f6c634300080a0033 \ No newline at end of file diff --git a/tests/tools/read-storage/test_data/UnstructuredStorageLayout.sol b/tests/tools/read-storage/test_data/UnstructuredStorageLayout.sol new file mode 100644 index 000000000..81b14a119 --- /dev/null +++ b/tests/tools/read-storage/test_data/UnstructuredStorageLayout.sol @@ -0,0 +1,141 @@ +pragma solidity 0.8.10; +// overwrite abi and bin: +// solc UnstructuredStorageLayout.sol --abi --bin --overwrite + +library StorageSlot { + struct AddressSlot { + address value; + } + + struct BooleanSlot { + bool value; + } + + struct Bytes32Slot { + bytes32 value; + } + + struct Uint256Slot { + uint256 value; + } + + /** + * @dev Returns an `AddressSlot` with member `value` located at `slot`. + */ + function getAddressSlot(bytes32 slot) internal pure returns (AddressSlot storage r) { + /// @solidity memory-safe-assembly + assembly { + r.slot := slot + } + } + + /** + * @dev Returns an `BooleanSlot` with member `value` located at `slot`. + */ + function getBooleanSlot(bytes32 slot) internal pure returns (BooleanSlot storage r) { + /// @solidity memory-safe-assembly + assembly { + r.slot := slot + } + } + + /** + * @dev Returns an `Bytes32Slot` with member `value` located at `slot`. + */ + function getBytes32Slot(bytes32 slot) internal pure returns (Bytes32Slot storage r) { + /// @solidity memory-safe-assembly + assembly { + r.slot := slot + } + } + + /** + * @dev Returns an `Uint256Slot` with member `value` located at `slot`. + */ + function getUint256Slot(bytes32 slot) internal pure returns (Uint256Slot storage r) { + /// @solidity memory-safe-assembly + assembly { + r.slot := slot + } + } +} + +contract UnstructuredStorageLayout { + + bytes32 constant ADMIN_SLOT = keccak256("org.zeppelinos.proxy.admin"); + // This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1. + bytes32 internal constant IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + // This is the keccak-256 hash of "eip1967.proxy.rollback" subtracted by 1 + bytes32 private constant ROLLBACK_SLOT = 0x4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd9143; + bytes32 constant BEACON_SLOT = bytes32(uint256(keccak256('eip1967.proxy.beacon')) - 1); + + address internal masterCopy; + + function _admin() internal view returns (address admin) { + bytes32 slot = ADMIN_SLOT; + assembly { + admin := sload(slot) + } + } + + function _implementation() internal view returns (address) { + address _impl; + bytes32 slot = IMPLEMENTATION_SLOT; + assembly { + _impl := sload(slot) + } + return _impl; + } + + function _set_rollback(bool _rollback) internal { + StorageSlot.getBooleanSlot(ROLLBACK_SLOT).value = _rollback; + } + + function _set_beacon(address _beacon) internal { + bytes32 slot = bytes32(uint256(keccak256('eip1967.proxy.beacon')) - 1); + assembly { + sstore(slot, _beacon) + } + } + + function store() external { + address admin = _admin(); + require(admin == address(0)); + + bytes32 admin_slot = ADMIN_SLOT; + address sender = msg.sender; + assembly { + sstore(admin_slot, sender) + } + + bytes32 impl_slot = IMPLEMENTATION_SLOT; + address _impl = address(0x0054006763154c764da4af42a8c3cfc25ea29765d5); + assembly { + sstore(impl_slot, _impl) + sstore(0xc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7, _impl) + } + + _set_rollback(true); + _set_beacon(address(0x0054006763154c764da4af42a8c3cfc25ea29765d5)); + } + + // Code position in storage is keccak256("PROXIABLE") = "0xc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7" + fallback() external { + assembly { // solium-disable-line + let nonsense := sload(sub(1,1)) + let _masterCopy := and(sload(0), 0xffffffffffffffffffffffffffffffffffffffff) + let contractLogic := sload(0xc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7) + calldatacopy(0x0, 0x0, calldatasize()) + let success := delegatecall(gas(), contractLogic, 0x0, calldatasize(), 0, 0) + let retSz := returndatasize() + returndatacopy(0, 0, retSz) + switch success + case 0 { + revert(0, retSz) + } + default { + return(0, retSz) + } + } + } +} diff --git a/tests/tools/read-storage/test_read_storage.py b/tests/tools/read-storage/test_read_storage.py index 6d2ab007d..b056ad056 100644 --- a/tests/tools/read-storage/test_read_storage.py +++ b/tests/tools/read-storage/test_read_storage.py @@ -1,65 +1,16 @@ -import json import re -import shutil -import subprocess -from time import sleep +import json from pathlib import Path -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 +from slither.tools.read_storage import SlitherReadStorage, RpcInfo TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" -# pylint: disable=too-few-public-methods -class GanacheInstance: - def __init__(self, provider: str, eth_address: str, eth_privkey: str): - self.provider = provider - self.eth_address = eth_address - self.eth_privkey = eth_privkey - - -@pytest.fixture(scope="module", name="web3") -def fixture_web3(ganache: GanacheInstance): - w3 = Web3(Web3.HTTPProvider(ganache.provider, request_kwargs={"timeout": 30})) - return w3 - - -@pytest.fixture(scope="module", name="ganache") -def fixture_ganache() -> Generator[GanacheInstance, None, None]: - """Fixture that runs ganache""" - if not shutil.which("ganache"): - raise Exception( - "ganache was not found in PATH, you can install it with `npm install -g ganache`" - ) - - # Address #1 when ganache is run with `--wallet.seed test`, it starts with 1000 ETH - eth_address = "0xae17D2dD99e07CA3bF2571CCAcEAA9e2Aefc2Dc6" - eth_privkey = "0xe48ba530a63326818e116be262fd39ae6dcddd89da4b1f578be8afd4e8894b8d" - eth = int(1e18 * 1e6) - port = 8545 - with subprocess.Popen( - f"""ganache - --port {port} - --chain.networkId 1 - --chain.chainId 1 - --account {eth_privkey},{eth} - """.replace( - "\n", " " - ), - shell=True, - ) as p: - - sleep(3) - yield GanacheInstance(f"http://127.0.0.1:{port}", eth_address, eth_privkey) - p.kill() - p.wait() - def get_source_file(file_path) -> str: with open(file_path, "r", encoding="utf8") as f: @@ -89,24 +40,29 @@ def deploy_contract(w3, ganache, contract_bin, contract_abi) -> Contract: # pylint: disable=too-many-locals +@pytest.mark.parametrize( + "test_contract, storage_file", + [("StorageLayout", "storage_layout"), ("UnstructuredStorageLayout", "unstructured_storage")], +) @pytest.mark.usefixtures("web3", "ganache") -def test_read_storage(web3, ganache, solc_binary_path) -> None: +def test_read_storage(test_contract, storage_file, web3, ganache, solc_binary_path) -> None: solc_path = solc_binary_path(version="0.8.10") assert web3.is_connected() - bin_path = Path(TEST_DATA_DIR, "StorageLayout.bin").as_posix() - abi_path = Path(TEST_DATA_DIR, "StorageLayout.abi").as_posix() + bin_path = Path(TEST_DATA_DIR, f"{test_contract}.bin").as_posix() + abi_path = Path(TEST_DATA_DIR, f"{test_contract}.abi").as_posix() bytecode = get_source_file(bin_path) abi = get_source_file(abi_path) contract = deploy_contract(web3, ganache, bytecode, abi) contract.functions.store().transact({"from": ganache.eth_address}) address = contract.address - sl = Slither(Path(TEST_DATA_DIR, "storage_layout-0.8.10.sol").as_posix(), solc=solc_path) + sl = Slither(Path(TEST_DATA_DIR, f"{test_contract}.sol").as_posix(), solc=solc_path) contracts = sl.contracts - srs = SlitherReadStorage(contracts, 100) - srs.rpc = ganache.provider + rpc_info: RpcInfo = RpcInfo(ganache.provider) + srs = SlitherReadStorage(contracts, 100, rpc_info) + srs.unstructured = True srs.storage_address = address srs.get_all_storage_variables() srs.get_storage_layout() @@ -116,7 +72,7 @@ def test_read_storage(web3, ganache, solc_binary_path) -> None: slot_infos_json = srs.to_json() json.dump(slot_infos_json, file, indent=4) - expected_file = Path(TEST_DATA_DIR, "TEST_storage_layout.json").as_posix() + expected_file = Path(TEST_DATA_DIR, f"TEST_{storage_file}.json").as_posix() with open(expected_file, "r", encoding="utf8") as f: expected = json.load(f) diff --git a/tests/unit/core/test_arithmetic.py b/tests/unit/core/test_arithmetic.py index 6de63d767..760b6d397 100644 --- a/tests/unit/core/test_arithmetic.py +++ b/tests/unit/core/test_arithmetic.py @@ -2,7 +2,8 @@ from pathlib import Path from slither import Slither from slither.utils.arithmetic import unchecked_arithemtic_usage - +from slither.slithir.operations import Binary, Unary, Assignment +from slither.slithir.variables.temporary import TemporaryVariable TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "arithmetic_usage" @@ -14,3 +15,21 @@ def test_arithmetic_usage(solc_binary_path) -> None: assert { f.source_mapping.content_hash for f in unchecked_arithemtic_usage(slither.contracts[0]) } == {"2b4bc73cf59d486dd9043e840b5028b679354dd9", "e4ecd4d0fda7e762d29aceb8425f2c5d4d0bf962"} + + +def test_scope_is_checked(solc_binary_path) -> None: + solc_path = solc_binary_path("0.8.15") + slither = Slither(Path(TEST_DATA_DIR, "unchecked_scope.sol").as_posix(), solc=solc_path) + func = slither.get_contract_from_name("TestScope")[0].get_function_from_full_name( + "scope(uint256)" + ) + bin_op_is_checked = {} + for node in func.nodes: + for op in node.irs: + if isinstance(op, (Binary, Unary)): + bin_op_is_checked[op.lvalue] = op.node.scope.is_checked + if isinstance(op, Assignment) and isinstance(op.rvalue, TemporaryVariable): + if op.lvalue.name.startswith("checked"): + assert bin_op_is_checked[op.rvalue] is True + else: + assert bin_op_is_checked[op.rvalue] is False diff --git a/tests/unit/core/test_constant_folding.py b/tests/unit/core/test_constant_folding.py index 489b4e0ec..6c0cc8295 100644 --- a/tests/unit/core/test_constant_folding.py +++ b/tests/unit/core/test_constant_folding.py @@ -21,39 +21,40 @@ def test_constant_folding_rational(solc_binary_path): variable_a = contract.get_state_variable_from_name("a") assert str(variable_a.type) == "uint256" - assert str(ConstantFolding(variable_a.expression, "uint256").result()) == "10" + assert ConstantFolding(variable_a.expression, "uint256").result().value == 10 variable_b = contract.get_state_variable_from_name("b") assert str(variable_b.type) == "int128" - assert str(ConstantFolding(variable_b.expression, "int128").result()) == "2" + assert ConstantFolding(variable_b.expression, "int128").result().value == 2 variable_c = contract.get_state_variable_from_name("c") assert str(variable_c.type) == "int64" - assert str(ConstantFolding(variable_c.expression, "int64").result()) == "3" + assert ConstantFolding(variable_c.expression, "int64").result().value == 3 variable_d = contract.get_state_variable_from_name("d") assert str(variable_d.type) == "int256" - assert str(ConstantFolding(variable_d.expression, "int256").result()) == "1500" + assert ConstantFolding(variable_d.expression, "int256").result().value == 1500 variable_e = contract.get_state_variable_from_name("e") assert str(variable_e.type) == "uint256" assert ( - str(ConstantFolding(variable_e.expression, "uint256").result()) - == "57896044618658097711785492504343953926634992332820282019728792003956564819968" + ConstantFolding(variable_e.expression, "uint256").result().value + == 57896044618658097711785492504343953926634992332820282019728792003956564819968 ) variable_f = contract.get_state_variable_from_name("f") assert str(variable_f.type) == "uint256" assert ( - str(ConstantFolding(variable_f.expression, "uint256").result()) - == "115792089237316195423570985008687907853269984665640564039457584007913129639935" + ConstantFolding(variable_f.expression, "uint256").result().value + == 115792089237316195423570985008687907853269984665640564039457584007913129639935 ) variable_g = contract.get_state_variable_from_name("g") assert str(variable_g.type) == "int64" - assert str(ConstantFolding(variable_g.expression, "int64").result()) == "-7" + assert ConstantFolding(variable_g.expression, "int64").result().value == -7 +# pylint: disable=too-many-locals def test_constant_folding_binary_expressions(solc_binary_path): sl = Slither( Path(CONSTANT_FOLDING_TEST_ROOT, "constant_folding_binop.sol").as_posix(), @@ -63,51 +64,65 @@ def test_constant_folding_binary_expressions(solc_binary_path): variable_a = contract.get_state_variable_from_name("a") assert str(variable_a.type) == "uint256" - assert str(ConstantFolding(variable_a.expression, "uint256").result()) == "0" + assert ConstantFolding(variable_a.expression, "uint256").result().value == 0 variable_b = contract.get_state_variable_from_name("b") assert str(variable_b.type) == "uint256" - assert str(ConstantFolding(variable_b.expression, "uint256").result()) == "3" + assert ConstantFolding(variable_b.expression, "uint256").result().value == 3 variable_c = contract.get_state_variable_from_name("c") assert str(variable_c.type) == "uint256" - assert str(ConstantFolding(variable_c.expression, "uint256").result()) == "3" + assert ConstantFolding(variable_c.expression, "uint256").result().value == 3 variable_d = contract.get_state_variable_from_name("d") assert str(variable_d.type) == "bool" - assert str(ConstantFolding(variable_d.expression, "bool").result()) == "False" + assert ConstantFolding(variable_d.expression, "bool").result().value is False variable_e = contract.get_state_variable_from_name("e") assert str(variable_e.type) == "bool" - assert str(ConstantFolding(variable_e.expression, "bool").result()) == "False" + assert ConstantFolding(variable_e.expression, "bool").result().value is False variable_f = contract.get_state_variable_from_name("f") assert str(variable_f.type) == "bool" - assert str(ConstantFolding(variable_f.expression, "bool").result()) == "True" + assert ConstantFolding(variable_f.expression, "bool").result().value is True variable_g = contract.get_state_variable_from_name("g") assert str(variable_g.type) == "bool" - assert str(ConstantFolding(variable_g.expression, "bool").result()) == "False" + assert ConstantFolding(variable_g.expression, "bool").result().value is False variable_h = contract.get_state_variable_from_name("h") assert str(variable_h.type) == "bool" - assert str(ConstantFolding(variable_h.expression, "bool").result()) == "False" + assert ConstantFolding(variable_h.expression, "bool").result().value is False variable_i = contract.get_state_variable_from_name("i") assert str(variable_i.type) == "bool" - assert str(ConstantFolding(variable_i.expression, "bool").result()) == "True" + assert ConstantFolding(variable_i.expression, "bool").result().value is True variable_j = contract.get_state_variable_from_name("j") assert str(variable_j.type) == "bool" - assert str(ConstantFolding(variable_j.expression, "bool").result()) == "False" + assert ConstantFolding(variable_j.expression, "bool").result().value is False variable_k = contract.get_state_variable_from_name("k") assert str(variable_k.type) == "bool" - assert str(ConstantFolding(variable_k.expression, "bool").result()) == "True" + assert ConstantFolding(variable_k.expression, "bool").result().value is True variable_l = contract.get_state_variable_from_name("l") assert str(variable_l.type) == "uint256" assert ( - str(ConstantFolding(variable_l.expression, "uint256").result()) - == "115792089237316195423570985008687907853269984665640564039457584007913129639935" + ConstantFolding(variable_l.expression, "uint256").result().value + == 115792089237316195423570985008687907853269984665640564039457584007913129639935 ) + + IMPLEMENTATION_SLOT = contract.get_state_variable_from_name("IMPLEMENTATION_SLOT") + assert str(IMPLEMENTATION_SLOT.type) == "bytes32" + assert ( + int.from_bytes( + ConstantFolding(IMPLEMENTATION_SLOT.expression, "bytes32").result().value, + byteorder="big", + ) + == 24440054405305269366569402256811496959409073762505157381672968839269610695612 + ) + + variable_m = contract.get_state_variable_from_name("m") + assert str(variable_m.type) == "bytes2" + assert ConstantFolding(variable_m.expression, "bytes2").result().value == "ab" diff --git a/tests/unit/core/test_data/arithmetic_usage/unchecked_scope.sol b/tests/unit/core/test_data/arithmetic_usage/unchecked_scope.sol new file mode 100644 index 000000000..f0cee1f10 --- /dev/null +++ b/tests/unit/core/test_data/arithmetic_usage/unchecked_scope.sol @@ -0,0 +1,17 @@ +contract TestScope { + function scope(uint256 x) public { + uint checked1 = x - x; + unchecked { + uint unchecked1 = x - x; + if (true) { + uint unchecked2 = x - x; + + } + for (uint i = 0; i < 10; i++) { + uint unchecked3 = x - x; + + } + } + uint checked2 = x - x; + } +} \ No newline at end of file diff --git a/tests/unit/core/test_data/constant_folding/constant_folding_binop.sol b/tests/unit/core/test_data/constant_folding/constant_folding_binop.sol index 923418ce7..3935585b8 100644 --- a/tests/unit/core/test_data/constant_folding/constant_folding_binop.sol +++ b/tests/unit/core/test_data/constant_folding/constant_folding_binop.sol @@ -11,4 +11,6 @@ contract BinOp { bool j = true && false; bool k = true || false; uint l = uint(1) - uint(2); + bytes32 IMPLEMENTATION_SLOT = bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1); + bytes2 m = "ab"; } \ No newline at end of file diff --git a/tests/unit/core/test_data/inheritance_resolution_error/contract_with_duplicate_names.sol b/tests/unit/core/test_data/inheritance_resolution_error/contract_with_duplicate_names.sol new file mode 100644 index 000000000..d5c6816e2 --- /dev/null +++ b/tests/unit/core/test_data/inheritance_resolution_error/contract_with_duplicate_names.sol @@ -0,0 +1,2 @@ +import {ERC20 as ERC20_1} from "./import.sol"; +contract ERC20 is ERC20_1 {} \ No newline at end of file diff --git a/tests/unit/core/test_data/inheritance_resolution_error/import.sol b/tests/unit/core/test_data/inheritance_resolution_error/import.sol new file mode 100644 index 000000000..e3221165a --- /dev/null +++ b/tests/unit/core/test_data/inheritance_resolution_error/import.sol @@ -0,0 +1 @@ +contract ERC20 {} \ No newline at end of file diff --git a/tests/unit/core/test_error_messages.py b/tests/unit/core/test_error_messages.py new file mode 100644 index 000000000..d0d915d56 --- /dev/null +++ b/tests/unit/core/test_error_messages.py @@ -0,0 +1,18 @@ +from pathlib import Path +import pytest + + +from slither import Slither +from slither.solc_parsing.slither_compilation_unit_solc import InheritanceResolutionError + +TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" +INHERITANCE_ERROR_ROOT = Path(TEST_DATA_DIR, "inheritance_resolution_error") + + +def test_inheritance_resolution_error(solc_binary_path) -> None: + with pytest.raises(InheritanceResolutionError): + solc_path = solc_binary_path("0.8.0") + Slither( + Path(INHERITANCE_ERROR_ROOT, "contract_with_duplicate_names.sol").as_posix(), + solc=solc_path, + ) diff --git a/tests/unit/slithir/test_ssa_generation.py b/tests/unit/slithir/test_ssa_generation.py index 62218e41a..3c7e84973 100644 --- a/tests/unit/slithir/test_ssa_generation.py +++ b/tests/unit/slithir/test_ssa_generation.py @@ -1,15 +1,17 @@ # # pylint: disable=too-many-lines import pathlib -from collections import defaultdict from argparse import ArgumentTypeError +from collections import defaultdict from inspect import getsourcefile from typing import Union, List, Dict, Callable import pytest from solc_select.solc_select import valid_version as solc_valid_version + from slither import Slither from slither.core.cfg.node import Node, NodeType from slither.core.declarations import Function, Contract +from slither.core.solidity_types import ArrayType from slither.core.variables.local_variable import LocalVariable from slither.core.variables.state_variable import StateVariable from slither.slithir.operations import ( @@ -1050,6 +1052,34 @@ def test_issue_1748(slither_from_source): assert isinstance(assign_op, InitArray) +def test_issue_1776(slither_from_source): + source = """ + contract Contract { + function foo() public returns (uint) { + uint[5][10][] memory arr = new uint[5][10][](2); + return 0; + } + } + """ + with slither_from_source(source) as slither: + c = slither.get_contract_from_name("Contract")[0] + f = c.functions[0] + operations = f.slithir_operations + new_op = operations[0] + lvalue = new_op.lvalue + lvalue_type = lvalue.type + assert isinstance(lvalue_type, ArrayType) + assert lvalue_type.is_dynamic + lvalue_type1 = lvalue_type.type + assert isinstance(lvalue_type1, ArrayType) + assert not lvalue_type1.is_dynamic + assert lvalue_type1.length_value.value == "10" + lvalue_type2 = lvalue_type1.type + assert isinstance(lvalue_type2, ArrayType) + assert not lvalue_type2.is_dynamic + assert lvalue_type2.length_value.value == "5" + + def test_issue_1846_ternary_in_if(slither_from_source): source = """ contract Contract { diff --git a/tests/unit/utils/test_data/upgradeability_util/src/ContractV2.sol b/tests/unit/utils/test_data/upgradeability_util/src/ContractV2.sol index 9b102f3e9..9c508caf3 100644 --- a/tests/unit/utils/test_data/upgradeability_util/src/ContractV2.sol +++ b/tests/unit/utils/test_data/upgradeability_util/src/ContractV2.sol @@ -1,6 +1,7 @@ pragma solidity ^0.8.2; import "./ProxyStorage.sol"; +import "./ERC20.sol"; contract ContractV2 is ProxyStorage { uint private stateA = 0; @@ -38,4 +39,8 @@ contract ContractV2 is ProxyStorage { function checkB() internal returns (bool) { return stateB == 32; } + + function erc20Transfer(address erc20, address to, uint256 amount) public returns (bool) { + return ERC20(erc20).transfer(to, amount); + } } diff --git a/tests/unit/utils/test_data/upgradeability_util/src/ERC20.sol b/tests/unit/utils/test_data/upgradeability_util/src/ERC20.sol new file mode 100644 index 000000000..6c7801581 --- /dev/null +++ b/tests/unit/utils/test_data/upgradeability_util/src/ERC20.sol @@ -0,0 +1,376 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v4.8.0) (token/ERC20/ERC20.sol) + +pragma solidity ^0.8.0; + +/** + * @dev Implementation of the {IERC20} interface. + * + * This implementation is agnostic to the way tokens are created. This means + * that a supply mechanism has to be added in a derived contract using {_mint}. + * For a generic mechanism see {ERC20PresetMinterPauser}. + * + * TIP: For a detailed writeup see our guide + * https://forum.openzeppelin.com/t/how-to-implement-erc20-supply-mechanisms/226[How + * to implement supply mechanisms]. + * + * We have followed general OpenZeppelin Contracts guidelines: functions revert + * instead returning `false` on failure. This behavior is nonetheless + * conventional and does not conflict with the expectations of ERC20 + * applications. + * + * Additionally, an {Approval} event is emitted on calls to {transferFrom}. + * This allows applications to reconstruct the allowance for all accounts just + * by listening to said events. Other implementations of the EIP may not emit + * these events, as it isn't required by the specification. + * + * Finally, the non-standard {decreaseAllowance} and {increaseAllowance} + * functions have been added to mitigate the well-known issues around setting + * allowances. See {IERC20-approve}. + */ +contract ERC20 { + mapping(address => uint256) private _balances; + + mapping(address => mapping(address => uint256)) private _allowances; + + uint256 private _totalSupply; + + string private _name; + string private _symbol; + + /** + * @dev Sets the values for {name} and {symbol}. + * + * The default value of {decimals} is 18. To select a different value for + * {decimals} you should overload it. + * + * All two of these values are immutable: they can only be set once during + * construction. + */ + constructor(string memory name_, string memory symbol_) { + _name = name_; + _symbol = symbol_; + } + + /** + * @dev Returns the name of the token. + */ + function name() public view virtual returns (string memory) { + return _name; + } + + /** + * @dev Returns the symbol of the token, usually a shorter version of the + * name. + */ + function symbol() public view virtual returns (string memory) { + return _symbol; + } + + /** + * @dev Returns the number of decimals used to get its user representation. + * For example, if `decimals` equals `2`, a balance of `505` tokens should + * be displayed to a user as `5.05` (`505 / 10 ** 2`). + * + * Tokens usually opt for a value of 18, imitating the relationship between + * Ether and Wei. This is the value {ERC20} uses, unless this function is + * overridden; + * + * NOTE: This information is only used for _display_ purposes: it in + * no way affects any of the arithmetic of the contract, including + * {IERC20-balanceOf} and {IERC20-transfer}. + */ + function decimals() public view virtual returns (uint8) { + return 18; + } + + /** + * @dev See {IERC20-totalSupply}. + */ + function totalSupply() public view virtual returns (uint256) { + return _totalSupply; + } + + /** + * @dev See {IERC20-balanceOf}. + */ + function balanceOf(address account) public view virtual returns (uint256) { + return _balances[account]; + } + + /** + * @dev See {IERC20-transfer}. + * + * Requirements: + * + * - `to` cannot be the zero address. + * - the caller must have a balance of at least `amount`. + */ + function transfer(address to, uint256 amount) public virtual returns (bool) { + address owner = msg.sender; + _transfer(owner, to, amount); + return true; + } + + /** + * @dev See {IERC20-allowance}. + */ + function allowance(address owner, address spender) public view virtual returns (uint256) { + return _allowances[owner][spender]; + } + + /** + * @dev See {IERC20-approve}. + * + * NOTE: If `amount` is the maximum `uint256`, the allowance is not updated on + * `transferFrom`. This is semantically equivalent to an infinite approval. + * + * Requirements: + * + * - `spender` cannot be the zero address. + */ + function approve(address spender, uint256 amount) public virtual returns (bool) { + address owner = msg.sender; + _approve(owner, spender, amount); + return true; + } + + /** + * @dev See {IERC20-transferFrom}. + * + * Emits an {Approval} event indicating the updated allowance. This is not + * required by the EIP. See the note at the beginning of {ERC20}. + * + * NOTE: Does not update the allowance if the current allowance + * is the maximum `uint256`. + * + * Requirements: + * + * - `from` and `to` cannot be the zero address. + * - `from` must have a balance of at least `amount`. + * - the caller must have allowance for ``from``'s tokens of at least + * `amount`. + */ + function transferFrom( + address from, + address to, + uint256 amount + ) public virtual returns (bool) { + address spender = msg.sender; + _spendAllowance(from, spender, amount); + _transfer(from, to, amount); + return true; + } + + /** + * @dev Atomically increases the allowance granted to `spender` by the caller. + * + * This is an alternative to {approve} that can be used as a mitigation for + * problems described in {IERC20-approve}. + * + * Emits an {Approval} event indicating the updated allowance. + * + * Requirements: + * + * - `spender` cannot be the zero address. + */ + function increaseAllowance(address spender, uint256 addedValue) public virtual returns (bool) { + address owner = msg.sender; + _approve(owner, spender, allowance(owner, spender) + addedValue); + return true; + } + + /** + * @dev Atomically decreases the allowance granted to `spender` by the caller. + * + * This is an alternative to {approve} that can be used as a mitigation for + * problems described in {IERC20-approve}. + * + * Emits an {Approval} event indicating the updated allowance. + * + * Requirements: + * + * - `spender` cannot be the zero address. + * - `spender` must have allowance for the caller of at least + * `subtractedValue`. + */ + function decreaseAllowance(address spender, uint256 subtractedValue) public virtual returns (bool) { + address owner = msg.sender; + uint256 currentAllowance = allowance(owner, spender); + require(currentAllowance >= subtractedValue, "ERC20: decreased allowance below zero"); + unchecked { + _approve(owner, spender, currentAllowance - subtractedValue); + } + + return true; + } + + /** + * @dev Moves `amount` of tokens from `from` to `to`. + * + * This internal function is equivalent to {transfer}, and can be used to + * e.g. implement automatic token fees, slashing mechanisms, etc. + * + * Emits a {Transfer} event. + * + * Requirements: + * + * - `from` cannot be the zero address. + * - `to` cannot be the zero address. + * - `from` must have a balance of at least `amount`. + */ + function _transfer( + address from, + address to, + uint256 amount + ) internal virtual { + require(from != address(0), "ERC20: transfer from the zero address"); + require(to != address(0), "ERC20: transfer to the zero address"); + + _beforeTokenTransfer(from, to, amount); + + uint256 fromBalance = _balances[from]; + require(fromBalance >= amount, "ERC20: transfer amount exceeds balance"); + unchecked { + _balances[from] = fromBalance - amount; + // Overflow not possible: the sum of all balances is capped by totalSupply, and the sum is preserved by + // decrementing then incrementing. + _balances[to] += amount; + } + _afterTokenTransfer(from, to, amount); + } + + /** @dev Creates `amount` tokens and assigns them to `account`, increasing + * the total supply. + * + * Emits a {Transfer} event with `from` set to the zero address. + * + * Requirements: + * + * - `account` cannot be the zero address. + */ + function _mint(address account, uint256 amount) internal virtual { + require(account != address(0), "ERC20: mint to the zero address"); + + _beforeTokenTransfer(address(0), account, amount); + + _totalSupply += amount; + unchecked { + // Overflow not possible: balance + amount is at most totalSupply + amount, which is checked above. + _balances[account] += amount; + } + _afterTokenTransfer(address(0), account, amount); + } + + /** + * @dev Destroys `amount` tokens from `account`, reducing the + * total supply. + * + * Emits a {Transfer} event with `to` set to the zero address. + * + * Requirements: + * + * - `account` cannot be the zero address. + * - `account` must have at least `amount` tokens. + */ + function _burn(address account, uint256 amount) internal virtual { + require(account != address(0), "ERC20: burn from the zero address"); + + _beforeTokenTransfer(account, address(0), amount); + + uint256 accountBalance = _balances[account]; + require(accountBalance >= amount, "ERC20: burn amount exceeds balance"); + unchecked { + _balances[account] = accountBalance - amount; + // Overflow not possible: amount <= accountBalance <= totalSupply. + _totalSupply -= amount; + } + _afterTokenTransfer(account, address(0), amount); + } + + /** + * @dev Sets `amount` as the allowance of `spender` over the `owner` s tokens. + * + * This internal function is equivalent to `approve`, and can be used to + * e.g. set automatic allowances for certain subsystems, etc. + * + * Emits an {Approval} event. + * + * Requirements: + * + * - `owner` cannot be the zero address. + * - `spender` cannot be the zero address. + */ + function _approve( + address owner, + address spender, + uint256 amount + ) internal virtual { + require(owner != address(0), "ERC20: approve from the zero address"); + require(spender != address(0), "ERC20: approve to the zero address"); + + _allowances[owner][spender] = amount; + } + + /** + * @dev Updates `owner` s allowance for `spender` based on spent `amount`. + * + * Does not update the allowance amount in case of infinite allowance. + * Revert if not enough allowance is available. + * + * Might emit an {Approval} event. + */ + function _spendAllowance( + address owner, + address spender, + uint256 amount + ) internal virtual { + uint256 currentAllowance = allowance(owner, spender); + if (currentAllowance != type(uint256).max) { + require(currentAllowance >= amount, "ERC20: insufficient allowance"); + unchecked { + _approve(owner, spender, currentAllowance - amount); + } + } + } + + /** + * @dev Hook that is called before any transfer of tokens. This includes + * minting and burning. + * + * Calling conditions: + * + * - when `from` and `to` are both non-zero, `amount` of ``from``'s tokens + * will be transferred to `to`. + * - when `from` is zero, `amount` tokens will be minted for `to`. + * - when `to` is zero, `amount` of ``from``'s tokens will be burned. + * - `from` and `to` are never both zero. + * + * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks]. + */ + function _beforeTokenTransfer( + address from, + address to, + uint256 amount + ) internal virtual {} + + /** + * @dev Hook that is called after any transfer of tokens. This includes + * minting and burning. + * + * Calling conditions: + * + * - when `from` and `to` are both non-zero, `amount` of ``from``'s tokens + * has been transferred to `to`. + * - when `from` is zero, `amount` tokens have been minted for `to`. + * - when `to` is zero, `amount` of ``from``'s tokens have been burned. + * - `from` and `to` are never both zero. + * + * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks]. + */ + function _afterTokenTransfer( + address from, + address to, + uint256 amount + ) internal virtual {} +} diff --git a/tests/unit/utils/test_upgradeability_util.py b/tests/unit/utils/test_upgradeability_util.py index 1563d3117..e17d87cd3 100644 --- a/tests/unit/utils/test_upgradeability_util.py +++ b/tests/unit/utils/test_upgradeability_util.py @@ -20,19 +20,41 @@ def test_upgrades_compare(solc_binary_path) -> None: sl = Slither(os.path.join(TEST_DATA_DIR, "TestUpgrades-0.8.2.sol"), solc=solc_path) v1 = sl.get_contract_from_name("ContractV1")[0] v2 = sl.get_contract_from_name("ContractV2")[0] - missing_vars, new_vars, tainted_vars, new_funcs, modified_funcs, tainted_funcs = compare(v1, v2) + ( + missing_vars, + new_vars, + tainted_vars, + new_funcs, + modified_funcs, + tainted_funcs, + tainted_contracts, + ) = compare(v1, v2, include_external=True) assert len(missing_vars) == 0 assert new_vars == [v2.get_state_variable_from_name("stateC")] assert tainted_vars == [ - v2.get_state_variable_from_name("stateB"), v2.get_state_variable_from_name("bug"), ] - assert new_funcs == [v2.get_function_from_signature("i()")] + assert new_funcs == [ + v2.get_function_from_signature("i()"), + v2.get_function_from_signature("erc20Transfer(address,address,uint256)"), + ] assert modified_funcs == [v2.get_function_from_signature("checkB()")] assert tainted_funcs == [ - v2.get_function_from_signature("g(uint256)"), v2.get_function_from_signature("h()"), ] + erc20 = sl.get_contract_from_name("ERC20")[0] + assert len(tainted_contracts) == 1 + assert tainted_contracts[0].contract == erc20 + assert set(tainted_contracts[0].tainted_functions) == { + erc20.get_function_from_signature("transfer(address,uint256)"), + erc20.get_function_from_signature("_transfer(address,address,uint256)"), + erc20.get_function_from_signature("_burn(address,uint256)"), + erc20.get_function_from_signature("balanceOf(address)"), + erc20.get_function_from_signature("_mint(address,uint256)"), + } + assert tainted_contracts[0].tainted_variables == [ + erc20.get_state_variable_from_name("_balances") + ] def test_upgrades_implementation_var(solc_binary_path) -> None: