From 9acddc3259fe7db22b79e9b9b557633793d58cad Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 8 Mar 2023 08:58:20 -0600 Subject: [PATCH 1/2] include return variables in uninit ptr detector --- .../uninitialized_storage_variables.py | 3 +- .../0.8.19/uninitialized_storage_pointer.sol | 12 +++ ...r.sol.0.8.19.UninitializedStorageVars.json | 85 +++++++++++++++++++ tests/test_detectors.py | 5 ++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol create mode 100644 tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json diff --git a/slither/detectors/variables/uninitialized_storage_variables.py b/slither/detectors/variables/uninitialized_storage_variables.py index 422996646..b1a0d956b 100644 --- a/slither/detectors/variables/uninitialized_storage_variables.py +++ b/slither/detectors/variables/uninitialized_storage_variables.py @@ -104,8 +104,9 @@ Bob calls `func`. As a result, `owner` is overridden to `0`. for function in contract.functions: if function.is_implemented and function.entry_point: uninitialized_storage_variables = [ - v for v in function.local_variables if v.is_storage and v.uninitialized + v for v in function.variables if v.is_storage and v.uninitialized ] + function.entry_point.context[self.key] = uninitialized_storage_variables self._detect_uninitialized(function, function.entry_point, []) diff --git a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol new file mode 100644 index 000000000..cac727ef5 --- /dev/null +++ b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol @@ -0,0 +1,12 @@ +contract Uninitialized{ + + struct St{ + uint a; + } + + function test() internal returns (St storage ret){ + ret = ret; + ret.a += 1; + } + +} diff --git a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json new file mode 100644 index 000000000..9c32fe16e --- /dev/null +++ b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json @@ -0,0 +1,85 @@ +[ + [ + { + "elements": [ + { + "type": "variable", + "name": "ret", + "source_mapping": { + "start": 101, + "length": 14, + "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "is_dependency": false, + "lines": [ + 7 + ], + "starting_column": 39, + "ending_column": 53 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "test", + "source_mapping": { + "start": 67, + "length": 96, + "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Uninitialized", + "source_mapping": { + "start": 0, + "length": 170, + "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "is_dependency": false, + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "test()" + } + } + } + } + ], + "description": "Uninitialized.test().ret (tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#7) is a storage variable never initialized\n", + "markdown": "[Uninitialized.test().ret](tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7) is a storage variable never initialized\n", + "first_markdown_element": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7", + "id": "cc7efd7ba9d430d21b17f18d15e561d2e188b59a3d62b9c5e76eeec765626cbd", + "check": "uninitialized-storage", + "impact": "High", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/test_detectors.py b/tests/test_detectors.py index 3d3cc7474..9d82afd2c 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -371,6 +371,11 @@ ALL_TEST_OBJECTS = [ "uninitialized_storage_pointer.sol", "0.4.25", ), + Test( + all_detectors.UninitializedStorageVars, + "uninitialized_storage_pointer.sol", + "0.8.19", + ), Test(all_detectors.TxOrigin, "tx_origin.sol", "0.4.25"), Test(all_detectors.TxOrigin, "tx_origin.sol", "0.5.16"), Test(all_detectors.TxOrigin, "tx_origin.sol", "0.6.11"), From e26a520766441c9a22ca99b2985ffe4fefa9578f Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 8 Mar 2023 10:45:45 -0600 Subject: [PATCH 2/2] exclude storage params --- .../uninitialized_storage_variables.py | 3 ++- .../0.8.19/uninitialized_storage_pointer.sol | 7 ++++- ...r.sol.0.8.19.UninitializedStorageVars.json | 27 +++++++++++-------- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/slither/detectors/variables/uninitialized_storage_variables.py b/slither/detectors/variables/uninitialized_storage_variables.py index b1a0d956b..9caa5b88f 100644 --- a/slither/detectors/variables/uninitialized_storage_variables.py +++ b/slither/detectors/variables/uninitialized_storage_variables.py @@ -103,8 +103,9 @@ Bob calls `func`. As a result, `owner` is overridden to `0`. for contract in self.compilation_unit.contracts: for function in contract.functions: if function.is_implemented and function.entry_point: + locals_except_params = set(function.variables) - set(function.parameters) uninitialized_storage_variables = [ - v for v in function.variables if v.is_storage and v.uninitialized + v for v in locals_except_params if v.is_storage and v.uninitialized ] function.entry_point.context[self.key] = uninitialized_storage_variables diff --git a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol index cac727ef5..a2fb6a77c 100644 --- a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol +++ b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol @@ -4,9 +4,14 @@ contract Uninitialized{ uint a; } - function test() internal returns (St storage ret){ + function bad() internal returns (St storage ret){ ret = ret; ret.a += 1; } + function ok(St storage ret) internal { + ret = ret; + ret.a += 1; + } + } diff --git a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json index 9c32fe16e..e7fab681d 100644 --- a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json +++ b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json @@ -6,7 +6,7 @@ "type": "variable", "name": "ret", "source_mapping": { - "start": 101, + "start": 100, "length": 14, "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", "filename_absolute": "/GENERIC_PATH", @@ -15,16 +15,16 @@ "lines": [ 7 ], - "starting_column": 39, - "ending_column": 53 + "starting_column": 38, + "ending_column": 52 }, "type_specific_fields": { "parent": { "type": "function", - "name": "test", + "name": "bad", "source_mapping": { "start": 67, - "length": 96, + "length": 95, "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", "filename_absolute": "/GENERIC_PATH", "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", @@ -44,7 +44,7 @@ "name": "Uninitialized", "source_mapping": { "start": 0, - "length": 170, + "length": 262, "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", "filename_absolute": "/GENERIC_PATH", "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", @@ -61,22 +61,27 @@ 9, 10, 11, - 12 + 12, + 13, + 14, + 15, + 16, + 17 ], "starting_column": 1, "ending_column": 2 } }, - "signature": "test()" + "signature": "bad()" } } } } ], - "description": "Uninitialized.test().ret (tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#7) is a storage variable never initialized\n", - "markdown": "[Uninitialized.test().ret](tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7) is a storage variable never initialized\n", + "description": "Uninitialized.bad().ret (tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#7) is a storage variable never initialized\n", + "markdown": "[Uninitialized.bad().ret](tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7) is a storage variable never initialized\n", "first_markdown_element": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7", - "id": "cc7efd7ba9d430d21b17f18d15e561d2e188b59a3d62b9c5e76eeec765626cbd", + "id": "979d28e501693ed7ece0d429e7c30266f8e9d6a2e2eedc87006c4bad63e78706", "check": "uninitialized-storage", "impact": "High", "confidence": "High"