From 7e6eee51af0a44af9e9baa941934311ea2c38660 Mon Sep 17 00:00:00 2001 From: bart1e Date: Tue, 27 Dec 2022 16:48:44 +0100 Subject: [PATCH 1/2] Uninitialised local vars bug fixed --- slither/solc_parsing/declarations/function.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index 9671d9bbe2..b2b8566310 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -894,7 +894,7 @@ def _parse_variable_definition_init_tuple( local_var_parser = LocalVariableInitFromTupleSolc(local_var, statement, index) self._add_local_variable(local_var_parser) - + statement["declarations"][0]["name"] = local_var.name new_node = self._new_node(NodeType.VARIABLE, statement["src"], node.underlying_node.scope) new_node.underlying_node.add_variable_declaration(local_var) link_underlying_nodes(node, new_node) From 2790e76fa2948c4bf66f64e4ed199d6a989d357a Mon Sep 17 00:00:00 2001 From: bart1e Date: Wed, 15 Mar 2023 19:25:06 +0100 Subject: [PATCH 2/2] Tests and comment added --- slither/solc_parsing/declarations/function.py | 7 ++++++ .../local_variables_initialised_in_tuple.sol | 22 +++++++++++++++++++ ...ple.sol.0.5.16.UninitializedLocalVars.json | 3 +++ .../local_variables_initialised_in_tuple.sol | 22 +++++++++++++++++++ ...ple.sol.0.6.11.UninitializedLocalVars.json | 3 +++ .../local_variables_initialised_in_tuple.sol | 22 +++++++++++++++++++ ...uple.sol.0.7.6.UninitializedLocalVars.json | 3 +++ tests/slithir/test_var_renaming_tuple.py | 21 ++++++++++++++++++ tests/slithir/var_renaming_tuple.sol | 19 ++++++++++++++++ tests/test_detectors.py | 15 +++++++++++++ 10 files changed, 137 insertions(+) create mode 100644 tests/detectors/uninitialized-local/0.5.16/local_variables_initialised_in_tuple.sol create mode 100644 tests/detectors/uninitialized-local/0.5.16/local_variables_initialised_in_tuple.sol.0.5.16.UninitializedLocalVars.json create mode 100644 tests/detectors/uninitialized-local/0.6.11/local_variables_initialised_in_tuple.sol create mode 100644 tests/detectors/uninitialized-local/0.6.11/local_variables_initialised_in_tuple.sol.0.6.11.UninitializedLocalVars.json create mode 100644 tests/detectors/uninitialized-local/0.7.6/local_variables_initialised_in_tuple.sol create mode 100644 tests/detectors/uninitialized-local/0.7.6/local_variables_initialised_in_tuple.sol.0.7.6.UninitializedLocalVars.json create mode 100644 tests/slithir/test_var_renaming_tuple.py create mode 100644 tests/slithir/var_renaming_tuple.sol diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index b2b8566310..566219758d 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -894,7 +894,14 @@ def _parse_variable_definition_init_tuple( local_var_parser = LocalVariableInitFromTupleSolc(local_var, statement, index) self._add_local_variable(local_var_parser) + + # the following instruction is necessary, because normally, when slither encounters vars with the same name in + # different scopes, it renames them, but while handling expressions like `(type1 a, type2 b) = f()`, an + # additional expression is created in `_parse_variable_definition` - it is initialised with a statement + # `"name": v["name"],`, so it uses the original name instead of using the newly created name for a variable + # see https://github.com/crytic/slither/pull/1533 for more information statement["declarations"][0]["name"] = local_var.name + new_node = self._new_node(NodeType.VARIABLE, statement["src"], node.underlying_node.scope) new_node.underlying_node.add_variable_declaration(local_var) link_underlying_nodes(node, new_node) diff --git a/tests/detectors/uninitialized-local/0.5.16/local_variables_initialised_in_tuple.sol b/tests/detectors/uninitialized-local/0.5.16/local_variables_initialised_in_tuple.sol new file mode 100644 index 0000000000..150a1fb73a --- /dev/null +++ b/tests/detectors/uninitialized-local/0.5.16/local_variables_initialised_in_tuple.sol @@ -0,0 +1,22 @@ +pragma solidity 0.5.16; + +// slither shouldn't throw "local variable never initialized" error on this file +// see https://github.com/crytic/slither/pull/1533 + +contract Test +{ + function f1() public view returns (int, string memory) { + return (0,"hello"); + } + + function f2(bool a) public view returns (string memory) { + if (a) { + (int x, string memory z) = f1(); + return z; + } else { + (int x, string memory z) = f1(); + return z; + } + + } +} diff --git a/tests/detectors/uninitialized-local/0.5.16/local_variables_initialised_in_tuple.sol.0.5.16.UninitializedLocalVars.json b/tests/detectors/uninitialized-local/0.5.16/local_variables_initialised_in_tuple.sol.0.5.16.UninitializedLocalVars.json new file mode 100644 index 0000000000..5825bcacc6 --- /dev/null +++ b/tests/detectors/uninitialized-local/0.5.16/local_variables_initialised_in_tuple.sol.0.5.16.UninitializedLocalVars.json @@ -0,0 +1,3 @@ +[ + [] +] \ No newline at end of file diff --git a/tests/detectors/uninitialized-local/0.6.11/local_variables_initialised_in_tuple.sol b/tests/detectors/uninitialized-local/0.6.11/local_variables_initialised_in_tuple.sol new file mode 100644 index 0000000000..62475372fb --- /dev/null +++ b/tests/detectors/uninitialized-local/0.6.11/local_variables_initialised_in_tuple.sol @@ -0,0 +1,22 @@ +pragma solidity 0.6.11; + +// slither shouldn't throw "local variable never initialized" error on this file +// see https://github.com/crytic/slither/pull/1533 + +contract Test +{ + function f1() public view returns (int, string memory) { + return (0,"hello"); + } + + function f2(bool a) public view returns (string memory) { + if (a) { + (int x, string memory z) = f1(); + return z; + } else { + (int x, string memory z) = f1(); + return z; + } + + } +} diff --git a/tests/detectors/uninitialized-local/0.6.11/local_variables_initialised_in_tuple.sol.0.6.11.UninitializedLocalVars.json b/tests/detectors/uninitialized-local/0.6.11/local_variables_initialised_in_tuple.sol.0.6.11.UninitializedLocalVars.json new file mode 100644 index 0000000000..5825bcacc6 --- /dev/null +++ b/tests/detectors/uninitialized-local/0.6.11/local_variables_initialised_in_tuple.sol.0.6.11.UninitializedLocalVars.json @@ -0,0 +1,3 @@ +[ + [] +] \ No newline at end of file diff --git a/tests/detectors/uninitialized-local/0.7.6/local_variables_initialised_in_tuple.sol b/tests/detectors/uninitialized-local/0.7.6/local_variables_initialised_in_tuple.sol new file mode 100644 index 0000000000..e5ca1813d5 --- /dev/null +++ b/tests/detectors/uninitialized-local/0.7.6/local_variables_initialised_in_tuple.sol @@ -0,0 +1,22 @@ +pragma solidity 0.7.6; + +// slither shouldn't throw "local variable never initialized" error on this file +// see https://github.com/crytic/slither/pull/1533 + +contract Test +{ + function f1() public view returns (int, string memory) { + return (0,"hello"); + } + + function f2(bool a) public view returns (string memory) { + if (a) { + (int x, string memory z) = f1(); + return z; + } else { + (int x, string memory z) = f1(); + return z; + } + + } +} diff --git a/tests/detectors/uninitialized-local/0.7.6/local_variables_initialised_in_tuple.sol.0.7.6.UninitializedLocalVars.json b/tests/detectors/uninitialized-local/0.7.6/local_variables_initialised_in_tuple.sol.0.7.6.UninitializedLocalVars.json new file mode 100644 index 0000000000..5825bcacc6 --- /dev/null +++ b/tests/detectors/uninitialized-local/0.7.6/local_variables_initialised_in_tuple.sol.0.7.6.UninitializedLocalVars.json @@ -0,0 +1,3 @@ +[ + [] +] \ No newline at end of file diff --git a/tests/slithir/test_var_renaming_tuple.py b/tests/slithir/test_var_renaming_tuple.py new file mode 100644 index 0000000000..8418ebde00 --- /dev/null +++ b/tests/slithir/test_var_renaming_tuple.py @@ -0,0 +1,21 @@ +from typing import Set + +from slither import Slither +from slither.slithir.operations import Unpack + + +# test if slither renames correctly local variables when they are initialised in tuple +def test_var_renaming_tuple() -> None: + slither = Slither("./tests/slithir/var_renaming_tuple.sol") + contract = slither.contracts[0] + f2 = contract.functions[0] if contract.functions[0].name == "f2" else contract.functions[1] + var_names: Set[str] = set() + for op in f2.slithir_operations: + if isinstance(op, Unpack): + var_names.add(op.lvalue.name) + # check if each variable has a different name + assert len(var_names) == 4 + + +if __name__ == "__main__": + test_var_renaming_tuple() diff --git a/tests/slithir/var_renaming_tuple.sol b/tests/slithir/var_renaming_tuple.sol new file mode 100644 index 0000000000..1899704600 --- /dev/null +++ b/tests/slithir/var_renaming_tuple.sol @@ -0,0 +1,19 @@ +pragma solidity 0.8.18; + +contract Test +{ + function f1() public view returns (int, string memory) { + return (0,"hello"); + } + + function f2(bool a) public view returns (string memory) { + if (a) { + (int x, string memory z) = f1(); + return z; + } else { + (int x, string memory z) = f1(); + return z; + } + + } +} \ No newline at end of file diff --git a/tests/test_detectors.py b/tests/test_detectors.py index 9d82afd2cf..31cd472038 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -674,6 +674,21 @@ def id_test(test_item: Test): "uninitialized_local_variable.sol", "0.7.6", ), + Test( + all_detectors.UninitializedLocalVars, + "local_variables_initialised_in_tuple.sol", + "0.5.16", + ), + Test( + all_detectors.UninitializedLocalVars, + "local_variables_initialised_in_tuple.sol", + "0.6.11", + ), + Test( + all_detectors.UninitializedLocalVars, + "local_variables_initialised_in_tuple.sol", + "0.7.6", + ), Test(all_detectors.ConstantFunctionsAsm, "constant.sol", "0.4.25"), Test( all_detectors.ConstantFunctionsState,