slither icon indicating copy to clipboard operation
slither copied to clipboard

Fix the dependency issue when using SSA variables

Open IzaiahSun opened this issue 7 months ago • 9 comments

What is happening in the previous version

function onERC721Received(
      address,
      address from,
      uint256,
      bytes calldata
  ) external returns (bytes4) {
      require(_opened, "Already yet open.");
      require(msg.sender == address(erc721Contract), "Unauthorized token");
      _safeMint(from, ERC721_RATIO); // Adjust minting based on the ERC721_RATIO
      return this.onERC721Received.selector;
}

If I'm using Slither to analyze the above code, the SSA variables will be generated for the parameter from in slither/slithir/utils/ssa.py line 127-135 and 162-172:

init_definition = {}
    for v in function.parameters:
        if v.name:
            init_definition[v.name] = (v, function.entry_point)
            function.entry_point.add_ssa_ir(Phi(LocalIRVariable(v), set()))

    for v in function.returns:
        if v.name:
            init_definition[v.name] = (v, function.entry_point)
for v in function.parameters:
    if v.name:
        new_var = LocalIRVariable(v)
        function.add_parameter_ssa(new_var)
        if new_var.is_storage:
            fake_variable = LocalIRVariable(v)
            fake_variable.name = "STORAGE_" + fake_variable.name
            fake_variable.set_location("reference_to_storage")
            new_var.refers_to = {fake_variable}
            init_local_variables_instances[fake_variable.name] = fake_variable
        init_local_variables_instances[v.name] = new_var

In this case, even a parameter is never written in a function, it will have two SSA variable, one is for the parameter (in the given case is from_0), and another for the usage (from_1).

When building the data dependency of SSA variables in slither/analyses/data_dependency/data_dependency.py, function compute_dependency_function, it will not consider such dependency. So from_1 will not have dependency with from_0, which is wrong.

How I solved this

I added some code into slither/analyses/data_dependency/data_dependency.py, function compute_dependency_function. When a function is analyzed, I will record all the SSA variables that are never written (used as lvalue) but used (as rvalue). If these variables' non-ssa version is the same with parameters, I will add a dependency edge between them.

The new test case is provided a proof-of-concept.

IzaiahSun avatar May 12 '25 13:05 IzaiahSun

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 12 '25 13:05 CLAassistant

Dear maintainers, this linter error seems to be the issue of GitHub's authentication, not the code. Could you help check this? image

IzaiahSun avatar May 24 '25 05:05 IzaiahSun

Hi, it should be due to the warnings you see in the screenshot. If you run pylint slither --rcfile pyproject.toml locally you should get them.

smonicas avatar May 27 '25 18:05 smonicas

Hi, it should be due to the warnings you see in the screenshot. If you run pylint slither --rcfile pyproject.toml locally you should get them.

Hi, I have run this locally, I have the following output, and no such warnings are shown.

(.venv) ➜  slither git:(dev) pylint slither --rcfile pyproject.toml
************* Module slither.tools.upgradeability.__main__
slither/tools/upgradeability/__main__.py:175:16: W0719: Raising too general exception: Exception (broad-exception-raised)
************* Module slither.tools.upgradeability.checks.constant
slither/tools/upgradeability/checks/constant.py:59:12: W0719: Raising too general exception: Exception (broad-exception-raised)
slither/tools/upgradeability/checks/constant.py:148:12: W0719: Raising too general exception: Exception (broad-exception-raised)
************* Module slither.tools.flattening.flattening
slither/tools/flattening/flattening.py:51:4: R0917: Too many positional arguments (8/5) (too-many-positional-arguments)
slither/tools/flattening/flattening.py:272:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/tools/flattening/flattening.py:456:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.tools.documentation.__main__
slither/tools/documentation/__main__.py:141:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
slither/tools/documentation/__main__.py:213:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.tools.slither_format.slither_format
slither/tools/slither_format/slither_format.py:113:16: W0719: Raising too general exception: Exception (broad-exception-raised)
************* Module slither.tools.properties.platforms.truffle
slither/tools/properties/platforms/truffle.py:71:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.tools.mutator.utils.patch
slither/tools/mutator/utils/patch.py:6:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.tools.mutator.utils.testing_generated_mutant
slither/tools/mutator/utils/testing_generated_mutant.py:85:0: R0917: Too many positional arguments (8/5) (too-many-positional-arguments)
************* Module slither.tools.mutator.mutators.abstract_mutator
slither/tools/mutator/mutators/abstract_mutator.py:24:4: R0917: Too many positional arguments (12/5) (too-many-positional-arguments)
************* Module slither.printers.summary.when_not_paused
slither/printers/summary/when_not_paused.py:15:11: E1121: Too many positional arguments for function call (too-many-function-args)
************* Module slither.printers.call.call_graph
slither/printers/call/call_graph.py:50:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/printers/call/call_graph.py:113:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/printers/call/call_graph.py:144:0: R0917: Too many positional arguments (8/5) (too-many-positional-arguments)
************* Module slither.slithir.operations.low_level_call
slither/slithir/operations/low_level_call.py:23:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.slithir.operations.internal_call
slither/slithir/operations/internal_call.py:15:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.slithir.operations.high_level_call
slither/slithir/operations/high_level_call.py:24:4: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.slithir.utils.ssa
slither/slithir/utils/ssa.py:216:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
slither/slithir/utils/ssa.py:416:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/slithir/utils/ssa.py:486:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/slithir/utils/ssa.py:589:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.slithir.tmp_operations.tmp_call
slither/slithir/tmp_operations/tmp_call.py:22:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.utils.code_generation
slither/utils/code_generation.py:27:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.formatters.naming_convention.naming_convention
slither/formatters/naming_convention/naming_convention.py:348:0: R0917: Too many positional arguments (8/5) (too-many-positional-arguments)
slither/formatters/naming_convention/naming_convention.py:449:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.formatters.utils.patches
slither/formatters/utils/patches.py:9:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.formatters.variables.unchanged_state_variables
slither/formatters/variables/unchanged_state_variables.py:31:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.formatters.attributes.constant_pragma
slither/formatters/attributes/constant_pragma.py:71:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.formatters.attributes.incorrect_solc
slither/formatters/attributes/incorrect_solc.py:61:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.solc_parsing.solidity_types.type_parsing
slither/solc_parsing/solidity_types/type_parsing.py:44:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.solc_parsing.declarations.contract
slither/solc_parsing/declarations/contract.py:489:4: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
slither/solc_parsing/declarations/contract.py:531:4: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.analyses.data_dependency.data_dependency
slither/analyses/data_dependency/data_dependency.py:436:0: R0912: Too many branches (19/12) (too-many-branches)
slither/analyses/data_dependency/data_dependency.py:465:4: R1702: Too many nested blocks (8/5) (too-many-nested-blocks)

-----------------------------------
Your code has been rated at 9.99/10

Only the last several lines are related to my changed code and they should not cause the workflow to fail.

IzaiahSun avatar May 28 '25 06:05 IzaiahSun

Hi! These are the lint warnings that cause the workflow to fail. Once these get fixed the CI will be green 👍

Screenshot 2025-05-28 at 09 01 18

elopez avatar May 28 '25 12:05 elopez

I have done the refactor on the code, and there should be no more linter issues.

IzaiahSun avatar Jun 18 '25 12:06 IzaiahSun

Hi @smonicas, can we proceed to the CI/CD workflows? I have fixed the previous problems and it should work now.

IzaiahSun avatar Aug 06 '25 06:08 IzaiahSun

Sorry for the previous errors. I have made test locally and in my own repo's CI/CD to make sure that they work.

image image image image

IzaiahSun avatar Aug 06 '25 15:08 IzaiahSun

Hi Maintainers,

It seems that all the checks are passed. Is it possible to merge the PR or I need to make further changes?

IzaiahSun avatar Aug 13 '25 07:08 IzaiahSun