Detector: Constant Function changing state (#661)

aderyn_core/src/context/browser/mod.rs

@@ -10,6 +10,7 @@ mod peek_over;
 mod peek_under;
 mod siblings;
 mod sort_nodes;
+mod storage_vars;
 pub use ancestral_line::*;
 pub use closest_ancestor::*;
 pub use extractor::*;
@@ -21,3 +22,4 @@ pub use peek_over::*;
 pub use peek_under::*;
 pub use siblings::*;
 pub use sort_nodes::*;
+pub use storage_vars::*;

aderyn_core/src/detect/detector.rs

@@ -86,6 +86,7 @@ pub fn get_all_issue_detectors() -> Vec<Box<dyn IssueDetector>> {
         Box::<ReturnBombDetector>::default(),
         Box::<OutOfOrderRetryableDetector>::default(),
         Box::<FunctionInitializingStateDetector>::default(),
+        Box::<ConstantFunctionChangingStateDetector>::default(),
         Box::<BuiltinSymbolShadowDetector>::default(),
     ]
 }
@@ -98,6 +99,7 @@ pub fn get_all_detectors_names() -> Vec<String> {
 #[derive(Debug, PartialEq, EnumString, Display)]
 #[strum(serialize_all = "kebab-case")]
 pub(crate) enum IssueDetectorNamePool {
+    ConstantFunctionChangingState,
     BuiltinSymbolShadow,
     IncorrectERC721Interface,
     FunctionInitializingState,
@@ -178,6 +180,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::ConstantFunctionChangingState => {
+            Some(Box::<ConstantFunctionChangingStateDetector>::default())
+        }
         IssueDetectorNamePool::BuiltinSymbolShadow => {
             Some(Box::<BuiltinSymbolShadowDetector>::default())
         }

aderyn_core/src/detect/high/const_func_change_state.rs

@@ -0,0 +1,168 @@
+use std::collections::BTreeMap;
+use std::error::Error;
+
+use crate::ast::{NodeID, StateMutability};
+
+use crate::capture;
+use crate::context::browser::ApproximateStorageChangeFinder;
+use crate::context::graph::{CallGraph, CallGraphDirection, CallGraphVisitor};
+use crate::detect::detector::IssueDetectorNamePool;
+use crate::detect::helpers;
+use crate::{
+    context::workspace_context::WorkspaceContext,
+    detect::detector::{IssueDetector, IssueSeverity},
+};
+use eyre::Result;
+
+#[derive(Default)]
+pub struct ConstantFunctionChangingStateDetector {
+    // 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 ConstantFunctionChangingStateDetector {
+    fn detect(&mut self, context: &WorkspaceContext) -> Result<bool, Box<dyn Error>> {
+        for func in helpers::get_implemented_external_and_public_functions(context) {
+            // Rule applies to only view functions, so ignore the rest
+            if func.state_mutability() != &StateMutability::View {
+                continue;
+            }
+            // Check if this func is compilable for solc < 0.5.0. If not, move on to the next
+            if !func.compiles_for_solc_below_0_5_0(context) {
+                continue;
+            }
+            // Now, investigate the function to see if there is scope for any state variable changes
+            let mut tracker = StateVariableChangeTracker {
+                state_var_has_changed: false,
+                context,
+            };
+
+            let investigator =
+                CallGraph::new(context, &[&(func.into())], CallGraphDirection::Inward)?;
+            investigator.accept(context, &mut tracker)?;
+
+            if tracker.state_var_has_changed {
+                capture!(self, context, func);
+            }
+        }
+
+        Ok(!self.found_instances.is_empty())
+    }
+
+    fn severity(&self) -> IssueSeverity {
+        IssueSeverity::High
+    }
+
+    fn title(&self) -> String {
+        String::from("Constant functions changing state")
+    }
+
+    fn description(&self) -> String {
+        String::from("Function is declared constant/view but it changes state. Ensure that the attributes of contract compiled prior to 0.5 are correct.")
+    }
+
+    fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> {
+        self.found_instances.clone()
+    }
+
+    fn name(&self) -> String {
+        IssueDetectorNamePool::ConstantFunctionChangingState.to_string()
+    }
+}
+
+struct StateVariableChangeTracker<'a> {
+    state_var_has_changed: bool,
+    context: &'a WorkspaceContext,
+}
+
+impl<'a> CallGraphVisitor for StateVariableChangeTracker<'a> {
+    fn visit_any(&mut self, node: &crate::ast::ASTNode) -> eyre::Result<()> {
+        if self.state_var_has_changed {
+            return Ok(());
+        }
+        // Check for state variable changes
+        let finder = ApproximateStorageChangeFinder::from(self.context, node);
+        if finder.state_variables_have_been_manipulated() {
+            self.state_var_has_changed = true;
+        }
+        Ok(())
+    }
+}
+
+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_0(&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_0(&pragma_semver) {
+                            return true;
+                        }
+                    }
+                }
+            }
+            false
+        }
+    }
+
+    fn version_req_allows_below_0_5_0(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;
+            }
+        }
+
+        // Else, return false
+        false
+    }
+}
+
+#[cfg(test)]
+mod constant_func_changing_state {
+    use serial_test::serial;
+
+    use crate::detect::{
+        detector::IssueDetector,
+        high::const_func_change_state::ConstantFunctionChangingStateDetector,
+    };
+
+    #[test]
+    #[serial]
+    fn test_constant_function_changing_state() {
+        let context = crate::detect::test_utils::load_solidity_source_unit(
+            "../tests/contract-playground/src/ConstFuncChangeState.sol",
+        );
+
+        let mut detector = ConstantFunctionChangingStateDetector::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
+        );
+    }
+}

aderyn_core/src/detect/high/mod.rs

@@ -1,6 +1,7 @@
 pub(crate) mod arbitrary_transfer_from;
 pub(crate) mod avoid_abi_encode_packed;
 pub(crate) mod block_timestamp_deadline;
+pub(crate) mod const_func_change_state;
 pub(crate) mod contract_locks_ether;
 pub(crate) mod dangerous_strict_equality_balance;
 pub(crate) mod dangerous_unary_operator;
@@ -41,6 +42,7 @@ pub(crate) mod yul_return;
 pub use arbitrary_transfer_from::ArbitraryTransferFromDetector;
 pub use avoid_abi_encode_packed::AvoidAbiEncodePackedDetector;
 pub use block_timestamp_deadline::BlockTimestampDeadlineDetector;
+pub use const_func_change_state::ConstantFunctionChangingStateDetector;
 pub use contract_locks_ether::ContractLocksEtherDetector;
 pub use dangerous_strict_equality_balance::DangerousStrictEqualityOnBalanceDetector;
 pub use dangerous_unary_operator::DangerousUnaryOperatorDetector;

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

@@ -200,6 +200,7 @@
     "contract-locks-ether",
     "incorrect-erc721-interface",
     "incorrect-erc20-interface",
-    "out-of-order-retryable"
+    "out-of-order-retryable",
+    "constant-function-changing-state"
   ]
 }