Detector: Deletion of nested mapping

aderyn_core/src/detect/detector.rs

@@ -65,6 +65,7 @@ pub fn get_all_issue_detectors() -> Vec<Box<dyn IssueDetector>> {
         Box::<RTLODetector>::default(),
         Box::<UncheckedReturnDetector>::default(),
         Box::<DangerousUnaryOperatorDetector>::default(),
+        Box::<DeletionNestedMappingDetector>::default(),
     ]
 }
 
@@ -126,6 +127,7 @@ pub(crate) enum IssueDetectorNamePool {
     RTLO,
     UncheckedReturn,
     DangerousUnaryOperator,
+    DeleteNestedMapping,
     // NOTE: `Undecided` will be the default name (for new bots).
     // If it's accepted, a new variant will be added to this enum before normalizing it in aderyn
     Undecided,
@@ -261,6 +263,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option<Box<dyn Iss
         IssueDetectorNamePool::DangerousUnaryOperator => {
             Some(Box::<DangerousUnaryOperatorDetector>::default())
         }
+        IssueDetectorNamePool::DeleteNestedMapping => {
+            Some(Box::<DeletionNestedMappingDetector>::default())
+        }
         IssueDetectorNamePool::Undecided => None,
     }
 }

aderyn_core/src/detect/high/deletion_nested_mapping.rs

@@ -0,0 +1,135 @@
+use std::collections::BTreeMap;
+use std::error::Error;
+
+use crate::ast::{
+    ASTNode, Expression, Identifier, IndexAccess, Mapping, NodeID, TypeName, UserDefinedTypeName,
+    VariableDeclaration,
+};
+
+use crate::capture;
+use crate::detect::detector::IssueDetectorNamePool;
+use crate::{
+    context::workspace_context::WorkspaceContext,
+    detect::detector::{IssueDetector, IssueSeverity},
+};
+use eyre::Result;
+
+#[derive(Default)]
+pub struct DeletionNestedMappingDetector {
+    // Keys are: [0] source file name, [1] line number, [2] character location of node.
+    // Do not add items manually, use `capture!` to add nodes to this BTreeMap.
+    found_instances: BTreeMap<(String, usize, String), NodeID>,
+}
+
+impl IssueDetector for DeletionNestedMappingDetector {
+    fn detect(&mut self, context: &WorkspaceContext) -> Result<bool, Box<dyn Error>> {
+        for delete_operation in context
+            .unary_operations()
+            .into_iter()
+            .filter(|op| op.operator == "delete")
+        {
+            if let Expression::IndexAccess(IndexAccess {
+                base_expression, ..
+            }) = delete_operation.sub_expression.as_ref()
+            {
+                if let Expression::Identifier(Identifier {
+                    referenced_declaration: Some(referenced_id),
+                    type_descriptions,
+                    ..
+                }) = base_expression.as_ref()
+                {
+                    // Check if we're deleting a value from mapping
+                    if type_descriptions
+                        .type_string
+                        .as_ref()
+                        .is_some_and(|type_string| type_string.starts_with("mapping"))
+                    {
+                        // Check if the value in the mapping is of type struct that has a member which is also a mapping
+                        if let Some(ASTNode::VariableDeclaration(VariableDeclaration {
+                            type_name: Some(TypeName::Mapping(Mapping { value_type, .. })),
+                            ..
+                        })) = context.nodes.get(referenced_id)
+                        {
+                            if let TypeName::UserDefinedTypeName(UserDefinedTypeName {
+                                referenced_declaration,
+                                ..
+                            }) = value_type.as_ref()
+                            {
+                                if let Some(ASTNode::StructDefinition(structure)) =
+                                    context.nodes.get(referenced_declaration)
+                                {
+                                    // Check that a member of a struct is of type mapping
+                                    if structure.members.iter().any(|member| {
+                                        member.type_descriptions.type_string.as_ref().is_some_and(
+                                            |type_string| type_string.starts_with("mapping"),
+                                        )
+                                    }) {
+                                        capture!(self, context, delete_operation);
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        Ok(!self.found_instances.is_empty())
+    }
+
+    fn severity(&self) -> IssueSeverity {
+        IssueSeverity::High
+    }
+
+    fn title(&self) -> String {
+        String::from("Deletion from a nested mappping.")
+    }
+
+    fn description(&self) -> String {
+        String::from("A deletion in a structure containing a mapping will not delete the mapping. The remaining data may be used to compromise the contract.")
+    }
+
+    fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> {
+        self.found_instances.clone()
+    }
+
+    fn name(&self) -> String {
+        IssueDetectorNamePool::DeleteNestedMapping.to_string()
+    }
+}
+
+#[cfg(test)]
+mod deletion_nested_mapping_tests {
+    use crate::detect::{
+        detector::IssueDetector, high::deletion_nested_mapping::DeletionNestedMappingDetector,
+    };
+
+    #[test]
+    fn test_deletion_nested_mapping() {
+        let context = crate::detect::test_utils::load_solidity_source_unit(
+            "../tests/contract-playground/src/DeletionNestedMappingStructureContract.sol",
+        );
+
+        let mut detector = DeletionNestedMappingDetector::default();
+        let found = detector.detect(&context).unwrap();
+        // assert that the detector found an issue
+        assert!(found);
+        // assert that the detector found the correct number of instances
+        assert_eq!(detector.instances().len(), 1);
+        // assert the severity is high
+        assert_eq!(
+            detector.severity(),
+            crate::detect::detector::IssueSeverity::High
+        );
+        // assert the title is correct
+        assert_eq!(
+            detector.title(),
+            String::from("Deletion from a nested mappping.")
+        );
+        // assert the description is correct
+        assert_eq!(
+            detector.description(),
+            String::from("A deletion in a structure containing a mapping will not delete the mapping. The remaining data may be used to compromise the contract.")
+        );
+    }
+}

aderyn_core/src/detect/high/mod.rs

@@ -4,6 +4,7 @@ pub(crate) mod block_timestamp_deadline;
 pub(crate) mod dangerous_unary_operator;
 pub(crate) mod delegate_call_in_loop;
 pub(crate) mod delegate_call_no_address_check;
+pub(crate) mod deletion_nested_mapping;
 pub(crate) mod dynamic_array_length_assignment;
 pub(crate) mod enumerable_loop_removal;
 pub(crate) mod experimental_encoder;
@@ -31,6 +32,7 @@ pub use block_timestamp_deadline::BlockTimestampDeadlineDetector;
 pub use dangerous_unary_operator::DangerousUnaryOperatorDetector;
 pub use delegate_call_in_loop::DelegateCallInLoopDetector;
 pub use delegate_call_no_address_check::DelegateCallOnUncheckedAddressDetector;
+pub use deletion_nested_mapping::DeletionNestedMappingDetector;
 pub use dynamic_array_length_assignment::DynamicArrayLengthAssignmentDetector;
 pub use enumerable_loop_removal::EnumerableLoopRemovalDetector;
 pub use experimental_encoder::ExperimentalEncoderDetector;

reports/adhoc-sol-files-highs-only-report.json

@@ -187,6 +187,7 @@
     "tautological-compare",
     "rtlo",
     "unchecked-return",
-    "dangerous-unary-operator"
+    "dangerous-unary-operator",
+    "delete-nested-mapping"
   ]
 }

reports/report.json

@@ -1,7 +1,7 @@
 {
   "files_summary": {
-    "total_source_units": 64,
-    "total_sloc": 1825
+    "total_source_units": 65,
+    "total_sloc": 1836
   },
   "files_details": {
     "files_details": [
@@ -53,6 +53,10 @@
         "file_path": "src/DelegateCallWithoutAddressCheck.sol",
         "n_sloc": 31
       },
+      {
+        "file_path": "src/DeletionNestedMappingStructureContract.sol",
+        "n_sloc": 11
+      },
       {
         "file_path": "src/DeprecatedOZFunctions.sol",
         "n_sloc": 32
@@ -264,7 +268,7 @@
     ]
   },
   "issue_count": {
-    "high": 26,
+    "high": 27,
     "low": 23
   },
   "high_issues": {
@@ -1578,6 +1582,19 @@
             "src_char": "247:10"
           }
         ]
+      },
+      {
+        "title": "Deletion from a nested mappping.",
+        "description": "A deletion in a structure containing a mapping will not delete the mapping. The remaining data may be used to compromise the contract.",
+        "detector_name": "delete-nested-mapping",
+        "instances": [
+          {
+            "contract_path": "src/DeletionNestedMappingStructureContract.sol",
+            "line_no": 17,
+            "src": "426:25",
+            "src_char": "426:25"
+          }
+        ]
       }
     ]
   },
@@ -1821,6 +1838,12 @@
             "src": "32:21",
             "src_char": "32:21"
           },
+          {
+            "contract_path": "src/DeletionNestedMappingStructureContract.sol",
+            "line_no": 2,
+            "src": "32:23",
+            "src_char": "32:23"
+          },
           {
             "contract_path": "src/InconsistentUints.sol",
             "line_no": 1,
@@ -2477,6 +2500,12 @@
             "src": "32:21",
             "src_char": "32:21"
           },
+          {
+            "contract_path": "src/DeletionNestedMappingStructureContract.sol",
+            "line_no": 2,
+            "src": "32:23",
+            "src_char": "32:23"
+          },
           {
             "contract_path": "src/DeprecatedOZFunctions.sol",
             "line_no": 2,
@@ -3319,6 +3348,7 @@
     "tautological-compare",
     "rtlo",
     "unchecked-return",
-    "dangerous-unary-operator"
+    "dangerous-unary-operator",
+    "delete-nested-mapping"
   ]
 }

reports/report.md

@@ -34,6 +34,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
   - [H-24: RTLO character detected in file. \u{202e}](#h-24-rtlo-character-detected-in-file-u202e)
   - [H-25: Return value of the function call is not checked.](#h-25-return-value-of-the-function-call-is-not-checked)
   - [H-26: Dangerous unary operator found in assignment.](#h-26-dangerous-unary-operator-found-in-assignment)
+  - [H-27: Deletion from a nested mappping.](#h-27-deletion-from-a-nested-mappping)
 - [Low Issues](#low-issues)
   - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners)
   - [L-2: Solmate's SafeTransferLib does not check for token contract's existence](#l-2-solmates-safetransferlib-does-not-check-for-token-contracts-existence)
@@ -66,8 +67,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 
 | Key | Value |
 | --- | --- |
-| .sol Files | 64 |
-| Total nSLOC | 1825 |
+| .sol Files | 65 |
+| Total nSLOC | 1836 |
 
 
 ## Files Details
@@ -86,6 +87,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 | src/CrazyPragma.sol | 4 |
 | src/DangerousUnaryOperator.sol | 13 |
 | src/DelegateCallWithoutAddressCheck.sol | 31 |
+| src/DeletionNestedMappingStructureContract.sol | 11 |
 | src/DeprecatedOZFunctions.sol | 32 |
 | src/DivisionBeforeMultiplication.sol | 22 |
 | src/DynamicArrayLengthAssignment.sol | 16 |
@@ -138,14 +140,14 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 | src/reused_contract_name/ContractB.sol | 7 |
 | src/uniswap/UniswapV2Swapper.sol | 50 |
 | src/uniswap/UniswapV3Swapper.sol | 150 |
-| **Total** | **1825** |
+| **Total** | **1836** |
 
 
 ## Issue Summary
 
 | Category | No. of Issues |
 | --- | --- |
-| High | 26 |
+| High | 27 |
 | Low | 23 |
 
 
@@ -1567,6 +1569,23 @@ Potentially mistakened `=+` for `+=` or `=-` for `-=`. Please include a space in
 
 
 
+## H-27: Deletion from a nested mappping.
+
+A deletion in a structure containing a mapping will not delete the mapping. The remaining data may be used to compromise the contract.
+
+<details><summary>1 Found Instances</summary>
+
+
+- Found in src/DeletionNestedMappingStructureContract.sol [Line: 17](../tests/contract-playground/src/DeletionNestedMappingStructureContract.sol#L17)
+
+	```solidity
+	        delete people[msg.sender];
+	```
+
+</details>
+
+
+
 # Low Issues
 
 ## L-1: Centralization Risk for trusted owners
@@ -1799,7 +1818,7 @@ ERC20 functions may not behave as expected. For example: return values are not a
 
 Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of `pragma solidity ^0.8.0;`, use `pragma solidity 0.8.0;`
 
-<details><summary>13 Found Instances</summary>
+<details><summary>14 Found Instances</summary>
 
 
 - Found in src/ContractWithTodo.sol [Line: 2](../tests/contract-playground/src/ContractWithTodo.sol#L2)
@@ -1832,6 +1851,12 @@ Consider using a specific version of Solidity in your contracts instead of a wid
 	pragma solidity ^0.8;
 	```
 
+- Found in src/DeletionNestedMappingStructureContract.sol [Line: 2](../tests/contract-playground/src/DeletionNestedMappingStructureContract.sol#L2)
+
+	```solidity
+	pragma solidity ^0.8.0;
+	```
+
 - Found in src/InconsistentUints.sol [Line: 1](../tests/contract-playground/src/InconsistentUints.sol#L1)
 
 	```solidity
@@ -2475,7 +2500,7 @@ Using `ERC721::_mint()` can mint ERC721 tokens to addresses which don't support
 
 Solc compiler version 0.8.20 switches the default target EVM version to Shanghai, which means that the generated bytecode will include PUSH0 opcodes. Be sure to select the appropriate EVM version in case you intend to deploy on a chain other than mainnet like L2 chains that may not support PUSH0, otherwise deployment of your contracts will fail.
 
-<details><summary>25 Found Instances</summary>
+<details><summary>26 Found Instances</summary>
 
 
 - Found in src/AdminContract.sol [Line: 2](../tests/contract-playground/src/AdminContract.sol#L2)
@@ -2508,6 +2533,12 @@ Solc compiler version 0.8.20 switches the default target EVM version to Shanghai
 	pragma solidity ^0.8;
 	```
 
+- Found in src/DeletionNestedMappingStructureContract.sol [Line: 2](../tests/contract-playground/src/DeletionNestedMappingStructureContract.sol#L2)
+
+	```solidity
+	pragma solidity ^0.8.0;
+	```
+
 - Found in src/DeprecatedOZFunctions.sol [Line: 2](../tests/contract-playground/src/DeprecatedOZFunctions.sol#L2)
 
 	```solidity