Detector: Function Pointers in constructors

aderyn_core/src/detect/detector.rs

@@ -93,6 +93,7 @@ pub fn get_all_issue_detectors() -> Vec<Box<dyn IssueDetector>> {
         Box::<ConstantFunctionChangingStateDetector>::default(),
         Box::<BuiltinSymbolShadowDetector>::default(),
         Box::<FunctionSelectorCollisionDetector>::default(),
+        Box::<FucntionPointerInConstructorDetector>::default(),
     ]
 }
 
@@ -104,6 +105,7 @@ pub fn get_all_detectors_names() -> Vec<String> {
 #[derive(Debug, PartialEq, EnumString, Display)]
 #[strum(serialize_all = "kebab-case")]
 pub(crate) enum IssueDetectorNamePool {
+    FunctionPointerInConstructor,
     DeadCode,
     FunctionSelectorCollision,
     CacheArrayLength,
@@ -191,6 +193,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option<Box<dyn Iss
     // Expects a valid detector_name
     let detector_name = IssueDetectorNamePool::from_str(detector_name).ok()?;
     match detector_name {
+        IssueDetectorNamePool::FunctionPointerInConstructor => {
+            Some(Box::<FucntionPointerInConstructorDetector>::default())
+        }
         IssueDetectorNamePool::DeadCode => Some(Box::<DeadCodeDetector>::default()),
         IssueDetectorNamePool::FunctionSelectorCollision => {
             Some(Box::<FunctionSelectorCollisionDetector>::default())

aderyn_core/src/detect/low/function_pointer_in_constructor.rs

@@ -0,0 +1,154 @@
+use std::collections::BTreeMap;
+use std::error::Error;
+
+use crate::ast::{FunctionKind, NodeID};
+
+use crate::capture;
+use crate::context::browser::ExtractVariableDeclarations;
+use crate::detect::detector::IssueDetectorNamePool;
+use crate::{
+    context::workspace_context::WorkspaceContext,
+    detect::detector::{IssueDetector, IssueSeverity},
+};
+use eyre::Result;
+
+#[derive(Default)]
+pub struct FucntionPointerInConstructorDetector {
+    // 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 FucntionPointerInConstructorDetector {
+    fn detect(&mut self, context: &WorkspaceContext) -> Result<bool, Box<dyn Error>> {
+        // PLAN:
+        // Catch all the function pointers in constructors that compile below 0.5.9
+
+        for func in context
+            .function_definitions()
+            .into_iter()
+            .filter(|f| f.kind() == &FunctionKind::Constructor)
+            .filter(|f| f.compiles_for_solc_below_0_5_9(context))
+        {
+            let variable_declarations = ExtractVariableDeclarations::from(func).extracted;
+
+            for variable_declaration in variable_declarations {
+                if variable_declaration
+                    .type_descriptions
+                    .type_string
+                    .as_ref()
+                    .is_some_and(|type_string| type_string.starts_with("function "))
+                {
+                    capture!(self, context, variable_declaration);
+                }
+            }
+        }
+
+        Ok(!self.found_instances.is_empty())
+    }
+
+    fn severity(&self) -> IssueSeverity {
+        IssueSeverity::Low
+    }
+
+    fn title(&self) -> String {
+        String::from("Function pointers used in constructors.")
+    }
+
+    fn description(&self) -> String {
+        String::from("solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior when calling uninitialized function pointers in constructors. It is recommended to not use function pointers in constructors.")
+    }
+
+    fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> {
+        self.found_instances.clone()
+    }
+
+    fn name(&self) -> String {
+        format!("{}", IssueDetectorNamePool::FunctionPointerInConstructor)
+    }
+}
+
+mod func_compilation_solc_pragma_helper {
+    use std::str::FromStr;
+
+    use semver::{Version, VersionReq};
+
+    use crate::{
+        ast::{FunctionDefinition, NodeType},
+        context::{
+            browser::{ExtractPragmaDirectives, GetClosestAncestorOfTypeX},
+            workspace_context::WorkspaceContext,
+        },
+        detect::helpers,
+    };
+
+    impl FunctionDefinition {
+        pub fn compiles_for_solc_below_0_5_9(&self, context: &WorkspaceContext) -> bool {
+            if let Some(source_unit) = self.closest_ancestor_of_type(context, NodeType::SourceUnit)
+            {
+                let pragma_directives = ExtractPragmaDirectives::from(source_unit).extracted;
+
+                if let Some(pragma_directive) = pragma_directives.first() {
+                    if let Ok(pragma_semver) = helpers::pragma_directive_to_semver(pragma_directive)
+                    {
+                        if version_req_allows_below_0_5_9(&pragma_semver) {
+                            return true;
+                        }
+                    }
+                }
+            }
+            false
+        }
+    }
+
+    fn version_req_allows_below_0_5_9(version_req: &VersionReq) -> bool {
+        // If it matches any 0.4.0 to 0.4.26, return true
+        for i in 0..=26 {
+            let version = Version::from_str(&format!("0.4.{}", i)).unwrap();
+            if version_req.matches(&version) {
+                return true;
+            }
+        }
+
+        // If it matches any 0.5.0 to 0.5.8, return true
+        for i in 0..=8 {
+            let version = Version::from_str(&format!("0.5.{}", i)).unwrap();
+            if version_req.matches(&version) {
+                return true;
+            }
+        }
+
+        // Else, return false
+        false
+    }
+}
+
+#[cfg(test)]
+mod function_pointers_tests {
+    use serial_test::serial;
+
+    use crate::detect::{
+        detector::IssueDetector,
+        low::function_pointer_in_constructor::FucntionPointerInConstructorDetector,
+    };
+
+    #[test]
+    #[serial]
+    fn test_function_pointers() {
+        let context = crate::detect::test_utils::load_solidity_source_unit(
+            "../tests/contract-playground/src/FunctionPointers.sol",
+        );
+
+        let mut detector = FucntionPointerInConstructorDetector::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 low
+        assert_eq!(
+            detector.severity(),
+            crate::detect::detector::IssueSeverity::Low
+        );
+    }
+}

aderyn_core/src/detect/low/mod.rs

@@ -13,6 +13,7 @@ pub(crate) mod division_before_multiplication;
 pub(crate) mod ecrecover;
 pub(crate) mod empty_blocks;
 pub(crate) mod function_init_state_vars;
+pub(crate) mod function_pointer_in_constructor;
 pub(crate) mod inconsistent_type_names;
 pub(crate) mod large_literal_value;
 pub(crate) mod non_reentrant_before_others;
@@ -50,6 +51,7 @@ pub use division_before_multiplication::DivisionBeforeMultiplicationDetector;
 pub use ecrecover::EcrecoverDetector;
 pub use empty_blocks::EmptyBlockDetector;
 pub use function_init_state_vars::FunctionInitializingStateDetector;
+pub use function_pointer_in_constructor::FucntionPointerInConstructorDetector;
 pub use inconsistent_type_names::InconsistentTypeNamesDetector;
 pub use large_literal_value::LargeLiteralValueDetector;
 pub use non_reentrant_before_others::NonReentrantBeforeOthersDetector;

reports/report.json

@@ -1,7 +1,7 @@
 {
   "files_summary": {
-    "total_source_units": 95,
-    "total_sloc": 3336
+    "total_source_units": 96,
+    "total_sloc": 3346
   },
   "files_details": {
     "files_details": [
@@ -137,6 +137,10 @@
         "file_path": "src/FunctionInitializingState.sol",
         "n_sloc": 49
       },
+      {
+        "file_path": "src/FunctionPointers.sol",
+        "n_sloc": 10
+      },
       {
         "file_path": "src/FunctionSignatureCollision.sol",
         "n_sloc": 9
@@ -389,7 +393,7 @@
   },
   "issue_count": {
     "high": 41,
-    "low": 36
+    "low": 37
   },
   "high_issues": {
     "issues": [
@@ -5623,6 +5627,19 @@
             "src_char": "414:15"
           }
         ]
+      },
+      {
+        "title": "Function pointers used in constructors.",
+        "description": "solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior when calling uninitialized function pointers in constructors. It is recommended to not use function pointers in constructors.",
+        "detector_name": "function-pointer-in-constructor",
+        "instances": [
+          {
+            "contract_path": "src/FunctionPointers.sol",
+            "line_no": 13,
+            "src": "330:50",
+            "src_char": "330:50"
+          }
+        ]
       }
     ]
   },
@@ -5703,6 +5720,7 @@
     "costly-operations-inside-loops",
     "constant-function-changing-state",
     "builtin-symbol-shadow",
-    "function-selector-collision"
+    "function-selector-collision",
+    "function-pointer-in-constructor"
   ]
 }

reports/report.md

@@ -86,6 +86,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
   - [L-34: Incorrect use of `assert()`](#l-34-incorrect-use-of-assert)
   - [L-35: Costly operations inside loops.](#l-35-costly-operations-inside-loops)
   - [L-36: Builtin Symbol Shadowing](#l-36-builtin-symbol-shadowing)
+  - [L-37: Function pointers used in constructors.](#l-37-function-pointers-used-in-constructors)
 
 
 # Summary
@@ -94,8 +95,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 
 | Key | Value |
 | --- | --- |
-| .sol Files | 95 |
-| Total nSLOC | 3336 |
+| .sol Files | 96 |
+| Total nSLOC | 3346 |
 
 
 ## Files Details
@@ -135,6 +136,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 | src/EnumerableSetIteration.sol | 55 |
 | src/ExperimentalEncoder.sol | 4 |
 | src/FunctionInitializingState.sol | 49 |
+| src/FunctionPointers.sol | 10 |
 | src/FunctionSignatureCollision.sol | 9 |
 | src/HugeConstants.sol | 36 |
 | src/InconsistentUints.sol | 17 |
@@ -197,15 +199,15 @@ 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** | **3336** |
+| **Total** | **3346** |
 
 
 ## Issue Summary
 
 | Category | No. of Issues |
 | --- | --- |
 | High | 41 |
-| Low | 36 |
+| Low | 37 |
 
 
 # High Issues
@@ -5731,3 +5733,20 @@ Name clashes with a built-in-symbol. Consider renaming it.
 
 
 
+## L-37: Function pointers used in constructors.
+
+solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior when calling uninitialized function pointers in constructors. It is recommended to not use function pointers in constructors.
+
+<details><summary>1 Found Instances</summary>
+
+
+- Found in src/FunctionPointers.sol [Line: 13](../tests/contract-playground/src/FunctionPointers.sol#L13)
+
+	```solidity
+	        function(uint, uint) pure returns (uint) operation;
+	```
+
+</details>
+
+
+

reports/report.sarif

@@ -9293,6 +9293,26 @@
             "text": "Name clashes with a built-in-symbol. Consider renaming it."
           },
           "ruleId": "builtin-symbol-shadow"
+        },
+        {
+          "level": "note",
+          "locations": [
+            {
+              "physicalLocation": {
+                "artifactLocation": {
+                  "uri": "src/FunctionPointers.sol"
+                },
+                "region": {
+                  "byteLength": 50,
+                  "byteOffset": 330
+                }
+              }
+            }
+          ],
+          "message": {
+            "text": "solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior when calling uninitialized function pointers in constructors. It is recommended to not use function pointers in constructors."
+          },
+          "ruleId": "function-pointer-in-constructor"
         }
       ],
       "tool": {

tests/contract-playground/src/FunctionPointers.sol

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.5.8;
+
+contract FunctionPointerExample {
+
+    // A simple add function
+    function add(uint a, uint b) public pure returns (uint) {
+        return a + b;
+    }
+
+    constructor() public {
+        // Declare a function type that takes two uint arguments and returns a uint
+        function(uint, uint) pure returns (uint) operation;
+
+        // Assign the add function to the operation variable
+        operation = add;
+    }
+
+}