Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve mapping-deletion detector for nested mappings #2084

Merged
merged 2 commits into from
Sep 8, 2023
Merged

Improve mapping-deletion detector for nested mappings #2084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7881f7c
Fix for nested mappings
smonicas Aug 11, 2023
2c196a1
Add test
smonicas Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions slither/detectors/statements/mapping_deletion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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

Expand Down
7 changes: 5 additions & 2 deletions .../snapshots/detectors__detector_MappingDeletionDetection_0_4_25_MappingDeletion_sol__0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)

7 changes: 5 additions & 2 deletions .../snapshots/detectors__detector_MappingDeletionDetection_0_5_16_MappingDeletion_sol__0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)

7 changes: 5 additions & 2 deletions .../snapshots/detectors__detector_MappingDeletionDetection_0_6_11_MappingDeletion_sol__0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)

7 changes: 5 additions & 2 deletions ...s/snapshots/detectors__detector_MappingDeletionDetection_0_7_6_MappingDeletion_sol__0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
19 changes: 15 additions & 4 deletions tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand All @@ -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);
Expand Down
Binary file modified tests/e2e/detectors/test_data/mapping-deletion/0.4.25/MappingDeletion.sol-0.4.25.zip
View file
Open in desktop
Binary file not shown.
12 changes: 11 additions & 1 deletion tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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);
Expand Down
Binary file modified tests/e2e/detectors/test_data/mapping-deletion/0.5.16/MappingDeletion.sol-0.5.16.zip
View file
Open in desktop
Binary file not shown.
12 changes: 11 additions & 1 deletion tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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);
Expand Down
Binary file modified tests/e2e/detectors/test_data/mapping-deletion/0.6.11/MappingDeletion.sol-0.6.11.zip
View file
Open in desktop
Binary file not shown.
12 changes: 11 additions & 1 deletion tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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);
Expand Down
Binary file modified tests/e2e/detectors/test_data/mapping-deletion/0.7.6/MappingDeletion.sol-0.7.6.zip
View file
Open in desktop
Binary file not shown.