diff --git a/slither/detectors/statements/mapping_deletion.py b/slither/detectors/statements/mapping_deletion.py index 4cdac72400..0940d5a07b 100644 --- a/slither/detectors/statements/mapping_deletion.py +++ b/slither/detectors/statements/mapping_deletion.py @@ -6,6 +6,7 @@ from slither.core.cfg.node import Node from slither.core.declarations import Structure from slither.core.declarations.contract import Contract +from slither.core.variables.variable import Variable from slither.core.declarations.function_contract import FunctionContract from slither.core.solidity_types import MappingType, UserDefinedType from slither.detectors.abstract_detector import ( @@ -69,14 +70,25 @@ def detect_mapping_deletion( for ir in node.irs: if isinstance(ir, Delete): value = ir.variable - if isinstance(value.type, UserDefinedType) and isinstance( - value.type.type, Structure - ): - st = value.type.type - if any(isinstance(e.type, MappingType) for e in st.elems.values()): - ret.append((f, st, node)) + MappingDeletionDetection.check_if_mapping(value, ret, f, node) + return ret + @staticmethod + def check_if_mapping( + value: Variable, + ret: List[Tuple[FunctionContract, Structure, Node]], + f: FunctionContract, + node: Node, + ): + if isinstance(value.type, UserDefinedType) and isinstance(value.type.type, Structure): + st = value.type.type + if any(isinstance(e.type, MappingType) for e in st.elems.values()): + ret.append((f, st, node)) + return + for e in st.elems.values(): + MappingDeletionDetection.check_if_mapping(e, ret, f, node) + def _detect(self) -> List[Output]: """Detect mapping deletion diff --git a/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_4_25_MappingDeletion_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_4_25_MappingDeletion_sol__0.txt index 902f966688..4d47bb5709 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_4_25_MappingDeletion_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_4_25_MappingDeletion_sol__0.txt @@ -1,6 +1,9 @@ +Balances.deleteNestedBalance() (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#40-42) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#17-20) which contains a mapping: + -delete nestedStackBalance (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#41) + Lib.deleteSt(Lib.MyStruct[1]) (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#9-11) deletes Lib.MyStruct (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#5-7) which contains a mapping: -delete st[0] (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#10) -Balances.deleteBalance(uint256) (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#28-31) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#17-20) which contains a mapping: - -delete stackBalance[idx] (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#30) +Balances.deleteBalance(uint256) (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#35-38) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#17-20) which contains a mapping: + -delete stackBalance[idx] (tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol#37) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_5_16_MappingDeletion_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_5_16_MappingDeletion_sol__0.txt index fec236e1c2..88e4ac554f 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_5_16_MappingDeletion_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_5_16_MappingDeletion_sol__0.txt @@ -1,6 +1,9 @@ Lib.deleteSt(Lib.MyStruct[1]) (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#9-11) deletes Lib.MyStruct (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#5-7) which contains a mapping: -delete st[0] (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#10) -Balances.deleteBalance(uint256) (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#29-32) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#17-20) which contains a mapping: - -delete stackBalance[idx] (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#31) +Balances.deleteBalance(uint256) (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#35-38) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#17-20) which contains a mapping: + -delete stackBalance[idx] (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#37) + +Balances.deleteNestedBalance() (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#40-42) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#17-20) which contains a mapping: + -delete nestedStackBalance (tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol#41) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_6_11_MappingDeletion_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_6_11_MappingDeletion_sol__0.txt index 7f0372c36d..4270f0d86c 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_6_11_MappingDeletion_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_6_11_MappingDeletion_sol__0.txt @@ -1,6 +1,9 @@ -Balances.deleteBalance(uint256) (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#29-32) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#17-20) which contains a mapping: - -delete stackBalance[idx] (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#31) +Balances.deleteNestedBalance() (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#40-42) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#17-20) which contains a mapping: + -delete nestedStackBalance (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#41) Lib.deleteSt(Lib.MyStruct[1]) (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#9-11) deletes Lib.MyStruct (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#5-7) which contains a mapping: -delete st[0] (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#10) +Balances.deleteBalance(uint256) (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#35-38) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#17-20) which contains a mapping: + -delete stackBalance[idx] (tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol#37) + diff --git a/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_7_6_MappingDeletion_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_7_6_MappingDeletion_sol__0.txt index f519a046f1..ea6ed2dd6a 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_7_6_MappingDeletion_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_MappingDeletionDetection_0_7_6_MappingDeletion_sol__0.txt @@ -1,5 +1,8 @@ -Balances.deleteBalance(uint256) (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#29-32) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#17-20) which contains a mapping: - -delete stackBalance[idx] (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#31) +Balances.deleteNestedBalance() (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#40-42) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#17-20) which contains a mapping: + -delete nestedStackBalance (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#41) + +Balances.deleteBalance(uint256) (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#35-38) deletes Balances.BalancesStruct (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#17-20) which contains a mapping: + -delete stackBalance[idx] (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#37) Lib.deleteSt(Lib.MyStruct[1]) (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#9-11) deletes Lib.MyStruct (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#5-7) which contains a mapping: -delete st[0] (tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol#10) diff --git a/tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol b/tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol index bedbb64a8f..bcbc86c9d1 100644 --- a/tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol +++ b/tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol @@ -6,7 +6,7 @@ library Lib{ mapping(address => uint) maps; } - function deleteSt(MyStruct[1] storage st){ + function deleteSt(MyStruct[1] storage st) internal { delete st[0]; } @@ -17,18 +17,29 @@ contract Balances { struct BalancesStruct{ address owner; mapping(address => uint) balances; - } + } + + struct NestedBalanceStruct { + BalancesStruct balanceStruct; + } mapping(uint => BalancesStruct) public stackBalance; + NestedBalanceStruct internal nestedStackBalance; + function createBalance(uint idx) public { - require(stackBalance[idx].owner == 0); - stackBalance[idx] = BalancesStruct(msg.sender); + require(stackBalance[idx].owner == address(0)); + BalancesStruct storage str = stackBalance[idx]; + str.owner = msg.sender; } function deleteBalance(uint idx) public { require(stackBalance[idx].owner == msg.sender); delete stackBalance[idx]; } + + function deleteNestedBalance() public { + delete nestedStackBalance; + } function setBalance(uint idx, address addr, uint val) public { require(stackBalance[idx].owner == msg.sender); diff --git a/tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol-0.4.25.zip b/tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol-0.4.25.zip index be3e13807a..5885936995 100644 Binary files a/tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol-0.4.25.zip and b/tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol-0.4.25.zip differ diff --git a/tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol b/tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol index e1b608a463..bcbc86c9d1 100644 --- a/tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol +++ b/tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol @@ -17,9 +17,15 @@ contract Balances { struct BalancesStruct{ address owner; mapping(address => uint) balances; - } + } + + struct NestedBalanceStruct { + BalancesStruct balanceStruct; + } mapping(uint => BalancesStruct) public stackBalance; + NestedBalanceStruct internal nestedStackBalance; + function createBalance(uint idx) public { require(stackBalance[idx].owner == address(0)); BalancesStruct storage str = stackBalance[idx]; @@ -30,6 +36,10 @@ contract Balances { require(stackBalance[idx].owner == msg.sender); delete stackBalance[idx]; } + + function deleteNestedBalance() public { + delete nestedStackBalance; + } function setBalance(uint idx, address addr, uint val) public { require(stackBalance[idx].owner == msg.sender); diff --git a/tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol-0.5.16.zip b/tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol-0.5.16.zip index 0ad84a5889..2e57890461 100644 Binary files a/tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol-0.5.16.zip and b/tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol-0.5.16.zip differ diff --git a/tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol b/tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol index e1b608a463..bcbc86c9d1 100644 --- a/tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol +++ b/tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol @@ -17,9 +17,15 @@ contract Balances { struct BalancesStruct{ address owner; mapping(address => uint) balances; - } + } + + struct NestedBalanceStruct { + BalancesStruct balanceStruct; + } mapping(uint => BalancesStruct) public stackBalance; + NestedBalanceStruct internal nestedStackBalance; + function createBalance(uint idx) public { require(stackBalance[idx].owner == address(0)); BalancesStruct storage str = stackBalance[idx]; @@ -30,6 +36,10 @@ contract Balances { require(stackBalance[idx].owner == msg.sender); delete stackBalance[idx]; } + + function deleteNestedBalance() public { + delete nestedStackBalance; + } function setBalance(uint idx, address addr, uint val) public { require(stackBalance[idx].owner == msg.sender); diff --git a/tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol-0.6.11.zip b/tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol-0.6.11.zip index 5f66da0617..8f532ea669 100644 Binary files a/tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol-0.6.11.zip and b/tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol-0.6.11.zip differ diff --git a/tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol b/tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol index e1b608a463..bcbc86c9d1 100644 --- a/tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol +++ b/tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol @@ -17,9 +17,15 @@ contract Balances { struct BalancesStruct{ address owner; mapping(address => uint) balances; - } + } + + struct NestedBalanceStruct { + BalancesStruct balanceStruct; + } mapping(uint => BalancesStruct) public stackBalance; + NestedBalanceStruct internal nestedStackBalance; + function createBalance(uint idx) public { require(stackBalance[idx].owner == address(0)); BalancesStruct storage str = stackBalance[idx]; @@ -30,6 +36,10 @@ contract Balances { require(stackBalance[idx].owner == msg.sender); delete stackBalance[idx]; } + + function deleteNestedBalance() public { + delete nestedStackBalance; + } function setBalance(uint idx, address addr, uint val) public { require(stackBalance[idx].owner == msg.sender); diff --git a/tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol-0.7.6.zip b/tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol-0.7.6.zip index 5888e0e534..c685454ce6 100644 Binary files a/tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol-0.7.6.zip and b/tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol-0.7.6.zip differ