is_guarded_history_utf8.txt

commit 9e9ec903746e7ea9054d1d9928238856e98f7f8a
Author: ObaHacker <dynateolabs@gmail.com>
Date:   Wed Mar 25 08:30:29 2026 +0100

    fixed issue#456

diff --git a/src/analyzer/security.rs b/src/analyzer/security.rs
index 3fd6a63..463317c 100644
--- a/src/analyzer/security.rs
+++ b/src/analyzer/security.rs
@@ -1,6 +1,6 @@
 use crate::runtime::executor::ContractExecutor;
 use crate::server::protocol::{ DynamicTraceEvent, DynamicTraceEventKind };
-use crate::utils::wasm::{ parse_instructions, WasmInstruction };
+use crate::utils::wasm::{ analyze_arithmetic_ops };
 use crate::Result;
 use serde::{ Deserialize, Serialize };
 use std::collections::{ HashMap, HashSet };
@@ -234,65 +234,35 @@ impl SecurityRule for ArithmeticCheckRule {
     }
 
     fn analyze_static(&self, wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
-        let mut findings = Vec::new();
-        let instructions = parse_instructions(wasm_bytes);
-
-        for (i, instr) in instructions.iter().enumerate() {
-            if Self::is_arithmetic(instr) && !Self::is_guarded(&instructions, i) {
-                findings.push(SecurityFinding {
-                    rule_id: self.name().to_string(),
-                    severity: Severity::Medium,
-                    location: format!("Instruction {}", i),
-                    description: format!("Unchecked arithmetic operation detected: {:?}", instr),
-                    remediation: "Ensure arithmetic operations are guarded with proper bounds checks or overflow handling.".to_string(),
-                    confidence: None,
-                    rationale: None,
-                });
-            }
-        }
-
-        Ok(findings)
-    }
-}
-
-impl ArithmeticCheckRule {
-    fn is_arithmetic(instr: &WasmInstruction) -> bool {
-        matches!(
-            instr,
-            WasmInstruction::I32Add |
-                WasmInstruction::I32Sub |
-                WasmInstruction::I32Mul |
-                WasmInstruction::I64Add |
-                WasmInstruction::I64Sub |
-                WasmInstruction::I64Mul
+        Ok(
+            analyze_arithmetic_ops(wasm_bytes)?
+                .into_iter()
+                .map(|analysis| {
+                    let confidence_label = analysis.confidence.label();
+                    let rationale = analysis.rationale;
+                    SecurityFinding {
+                        rule_id: self.name().to_string(),
+                        severity: Severity::Medium,
+                        location: format!(
+                            "Function {} instruction {} (offset {})",
+                            analysis.function_index,
+                            analysis.instruction_index,
+                            analysis.offset
+                        ),
+                        description: format!(
+                            "Potential unchecked arithmetic operation detected: {:?}. Confidence: {}. {}",
+                            analysis.instruction,
+                            confidence_label,
+                            rationale
+                        ),
+                        remediation: "Ensure arithmetic operations are guarded with proper bounds checks or overflow handling.".to_string(),
+                        confidence: Some(analysis.confidence.score()),
+                        rationale: Some(rationale),
+                    }
+                })
+                .collect(),
         )
     }
-
-    fn is_guarded(instructions: &[WasmInstruction], idx: usize) -> bool {
-        // A guard must appear *after* the arithmetic instruction.
-        //
-        // Rationale:
-        //   ΓÇó Instructions before `idx` execute before the result is on the
-        //     stack, so they cannot be checking that result.
-        //   ΓÇó `Call` is intentionally excluded: an unrelated nearby call (a
-        //     logger, a helper, etc.) is not a bounds check and must not
-        //     suppress the finding.
-        //
-        // A legitimate overflow guard looks like:
-        //   i32.add          ← idx
-        //   <optional cmp>
-        //   br_if / if       ← this is the guard
-        //
-        // We allow up to 3 instructions of "compare setup" between the
-        // arithmetic and the conditional branch before giving up.
-        let end = (idx + 4).min(instructions.len());
-        for instr in &instructions[idx + 1..end] {
-            if matches!(instr, WasmInstruction::If | WasmInstruction::BrIf) {
-                return true;
-            }
-        }
-        false
-    }
 }
 
 struct AuthorizationCheckRule;
@@ -1052,106 +1022,6 @@ mod tests {
         assert!(findings[0].description.contains(&valid_addr));
     }
 
-    // -----------------------------------------------------------------------
-    // ArithmeticCheckRule / is_guarded ΓÇö fixture tests
-    // -----------------------------------------------------------------------
-
-    /// Bare arithmetic with no surrounding instructions must be flagged.
-    #[test]
-    fn is_guarded_false_for_isolated_arithmetic() {
-        let instrs = vec![WasmInstruction::I32Add];
-        assert!(!ArithmeticCheckRule::is_guarded(&instrs, 0));
-    }
-
-    /// A `BrIf` immediately *after* the arithmetic is a valid guard.
-    #[test]
-    fn is_guarded_true_for_brif_after_arithmetic() {
-        let instrs = vec![WasmInstruction::I32Add, WasmInstruction::BrIf];
-        assert!(ArithmeticCheckRule::is_guarded(&instrs, 0));
-    }
-
-    /// An `If` immediately *after* the arithmetic is a valid guard.
-    #[test]
-    fn is_guarded_true_for_if_after_arithmetic() {
-        let instrs = vec![WasmInstruction::I32Add, WasmInstruction::If];
-        assert!(ArithmeticCheckRule::is_guarded(&instrs, 0));
-    }
-
-    /// A `BrIf` within the 3-instruction lookahead window (with one
-    /// intermediate instruction between) is still a valid guard.
-    #[test]
-    fn is_guarded_true_for_brif_within_lookahead_window() {
-        // e.g.: i32.add  ->  i32.const (compare setup)  ->  br_if
-        let instrs = vec![
-            WasmInstruction::I32Add,
-            WasmInstruction::Unknown(0x41),
-            WasmInstruction::BrIf,
-        ];
-        assert!(ArithmeticCheckRule::is_guarded(&instrs, 0));
-    }
-
-    /// A `BrIf` that falls *outside* the 3-instruction lookahead must NOT
-    /// suppress the finding ΓÇö the guard is too far away to be meaningful.
-    #[test]
-    fn is_guarded_false_when_brif_beyond_lookahead() {
-        // idx=0, window covers idx+1..idx+4 (indices 1, 2, 3).
-        // BrIf is at index 4, which is outside the window.
-        let instrs = vec![
-            WasmInstruction::I32Add,   // idx 0
-            WasmInstruction::Unknown(0x41), // idx 1
-            WasmInstruction::Unknown(0x41), // idx 2
-            WasmInstruction::Unknown(0x41), // idx 3
-            WasmInstruction::BrIf,     // idx 4 ΓÇö outside window
-        ];
-        assert!(!ArithmeticCheckRule::is_guarded(&instrs, 0));
-    }
-
-    /// **Key regression** ΓÇö a `BrIf` that appears *before* the arithmetic
-    /// (guarding something else entirely) must NOT suppress the finding.
-    ///
-    /// The old code used `idx.saturating_sub(2)` as the start, so a BrIf
-    /// two slots before the arithmetic would incorrectly return true.
-    #[test]
-    fn is_guarded_false_for_brif_only_before_arithmetic() {
-        let instrs = vec![WasmInstruction::BrIf, WasmInstruction::I32Add];
-        assert!(!ArithmeticCheckRule::is_guarded(&instrs, 1));
-    }
-
-    /// **Key regression** ΓÇö a `Call` anywhere near the arithmetic must NOT
-    /// suppress the finding.  An unrelated call (logger, helper, etc.) is not
-    /// a bounds check.
-    #[test]
-    fn is_guarded_false_for_nearby_unrelated_call() {
-        // Call before:
-        let before = vec![WasmInstruction::Call, WasmInstruction::I32Add];
-        assert!(!ArithmeticCheckRule::is_guarded(&before, 1));
-
-        // Call after:
-        let after = vec![WasmInstruction::I32Add, WasmInstruction::Call];
-        assert!(!ArithmeticCheckRule::is_guarded(&after, 0));
-
-        // Call on both sides:
-        let both = vec![WasmInstruction::Call, WasmInstruction::I32Mul, WasmInstruction::Call];
-        assert!(!ArithmeticCheckRule::is_guarded(&both, 1));
-    }
-
-    /// A `Call` between the arithmetic and a `BrIf` must not block the guard
-    /// from being recognised ΓÇö only the presence of If/BrIf matters.
-    #[test]
-    fn is_guarded_true_when_brif_follows_call_after_arithmetic() {
-        // i32.add  ->  call (side-effect)  ->  br_if (checks result)
-        let instrs = vec![WasmInstruction::I32Add, WasmInstruction::Call, WasmInstruction::BrIf];
-        assert!(ArithmeticCheckRule::is_guarded(&instrs, 0));
-    }
-
-    /// Arithmetic at the very last position of the slice must not panic and
-    /// must be reported as unguarded (no instructions ahead to look at).
-    #[test]
-    fn is_guarded_false_at_end_of_slice() {
-        let instrs = vec![WasmInstruction::Unknown(0x41), WasmInstruction::I64Add];
-        assert!(!ArithmeticCheckRule::is_guarded(&instrs, 1));
-    }
-
     // Pre-existing tests (unchanged)
 
     #[test]

commit 5cd9d2acd4c6eeecb842acdf08c204748a9f95a2
Author: Juwonlo <owoeyejuju@gmail.com>
Date:   Tue Mar 24 04:58:07 2026 +0100

    fix: Issue #398

diff --git a/src/analyzer/security.rs b/src/analyzer/security.rs
index 192b78d..1985e17 100644
--- a/src/analyzer/security.rs
+++ b/src/analyzer/security.rs
@@ -265,18 +265,28 @@ impl ArithmeticCheckRule {
     }
 
     fn is_guarded(instructions: &[WasmInstruction], idx: usize) -> bool {
-        let start = idx.saturating_sub(2);
-        let end = (idx + 3).min(instructions.len());
-
-        for instr in &instructions[start..end] {
-            if matches!(
-                instr,
-                WasmInstruction::If | WasmInstruction::BrIf | WasmInstruction::Call
-            ) {
+        // A guard must appear *after* the arithmetic instruction.
+        //
+        // Rationale:
+        //   ΓÇó Instructions before `idx` execute before the result is on the
+        //     stack, so they cannot be checking that result.
+        //   ΓÇó `Call` is intentionally excluded: an unrelated nearby call (a
+        //     logger, a helper, etc.) is not a bounds check and must not
+        //     suppress the finding.
+        //
+        // A legitimate overflow guard looks like:
+        //   i32.add          ← idx
+        //   <optional cmp>
+        //   br_if / if       ← this is the guard
+        //
+        // We allow up to 3 instructions of "compare setup" between the
+        // arithmetic and the conditional branch before giving up.
+        let end = (idx + 4).min(instructions.len());
+        for instr in &instructions[idx + 1..end] {
+            if matches!(instr, WasmInstruction::If | WasmInstruction::BrIf) {
                 return true;
             }
         }
-
         false
     }
 }
@@ -854,6 +864,114 @@ mod tests {
         assert!(findings[0].description.contains(&valid_addr));
     }
 
+    // -----------------------------------------------------------------------
+    // ArithmeticCheckRule / is_guarded ΓÇö fixture tests
+    // -----------------------------------------------------------------------
+
+    /// Bare arithmetic with no surrounding instructions must be flagged.
+    #[test]
+    fn is_guarded_false_for_isolated_arithmetic() {
+        let instrs = vec![WasmInstruction::I32Add];
+        assert!(!ArithmeticCheckRule::is_guarded(&instrs, 0));
+    }
+
+    /// A `BrIf` immediately *after* the arithmetic is a valid guard.
+    #[test]
+    fn is_guarded_true_for_brif_after_arithmetic() {
+        let instrs = vec![WasmInstruction::I32Add, WasmInstruction::BrIf];
+        assert!(ArithmeticCheckRule::is_guarded(&instrs, 0));
+    }
+
+    /// An `If` immediately *after* the arithmetic is a valid guard.
+    #[test]
+    fn is_guarded_true_for_if_after_arithmetic() {
+        let instrs = vec![WasmInstruction::I32Add, WasmInstruction::If];
+        assert!(ArithmeticCheckRule::is_guarded(&instrs, 0));
+    }
+
+    /// A `BrIf` within the 3-instruction lookahead window (with one
+    /// intermediate compare instruction between) is still a valid guard.
+    #[test]
+    fn is_guarded_true_for_brif_within_lookahead_window() {
+        // e.g.: i32.add  ->  i32.const (compare setup)  ->  br_if
+        let instrs = vec![
+            WasmInstruction::I32Add,
+            WasmInstruction::I32Const,
+            WasmInstruction::BrIf,
+        ];
+        assert!(ArithmeticCheckRule::is_guarded(&instrs, 0));
+    }
+
+    /// A `BrIf` that falls *outside* the 3-instruction lookahead must NOT
+    /// suppress the finding ΓÇö the guard is too far away to be meaningful.
+    #[test]
+    fn is_guarded_false_when_brif_beyond_lookahead() {
+        // idx=0, window covers idx+1..idx+4 (indices 1, 2, 3).
+        // BrIf is at index 4, which is outside the window.
+        let instrs = vec![
+            WasmInstruction::I32Add,   // idx 0
+            WasmInstruction::I32Const, // idx 1
+            WasmInstruction::I32Const, // idx 2
+            WasmInstruction::I32Const, // idx 3
+            WasmInstruction::BrIf,     // idx 4 ΓÇö outside window
+        ];
+        assert!(!ArithmeticCheckRule::is_guarded(&instrs, 0));
+    }
+
+    /// **Key regression** ΓÇö a `BrIf` that appears *before* the arithmetic
+    /// (guarding something else entirely) must NOT suppress the finding.
+    ///
+    /// The old code used `idx.saturating_sub(2)` as the start, so a BrIf
+    /// two slots before the arithmetic would incorrectly return true.
+    #[test]
+    fn is_guarded_false_for_brif_only_before_arithmetic() {
+        let instrs = vec![WasmInstruction::BrIf, WasmInstruction::I32Add];
+        assert!(!ArithmeticCheckRule::is_guarded(&instrs, 1));
+    }
+
+    /// **Key regression** ΓÇö a `Call` anywhere near the arithmetic must NOT
+    /// suppress the finding.  An unrelated call (logger, helper, etc.) is not
+    /// a bounds check.
+    #[test]
+    fn is_guarded_false_for_nearby_unrelated_call() {
+        // Call before:
+        let before = vec![WasmInstruction::Call, WasmInstruction::I32Add];
+        assert!(!ArithmeticCheckRule::is_guarded(&before, 1));
+
+        // Call after:
+        let after = vec![WasmInstruction::I32Add, WasmInstruction::Call];
+        assert!(!ArithmeticCheckRule::is_guarded(&after, 0));
+
+        // Call on both sides:
+        let both = vec![
+            WasmInstruction::Call,
+            WasmInstruction::I32Mul,
+            WasmInstruction::Call,
+        ];
+        assert!(!ArithmeticCheckRule::is_guarded(&both, 1));
+    }
+
+    /// A `Call` between the arithmetic and a `BrIf` must not block the guard
+    /// from being recognised ΓÇö only the presence of If/BrIf matters.
+    #[test]
+    fn is_guarded_true_when_brif_follows_call_after_arithmetic() {
+        // i32.add  ->  call (side-effect)  ->  br_if (checks result)
+        let instrs = vec![
+            WasmInstruction::I32Add,
+            WasmInstruction::Call,
+            WasmInstruction::BrIf,
+        ];
+        assert!(ArithmeticCheckRule::is_guarded(&instrs, 0));
+    }
+
+    /// Arithmetic at the very last position of the slice must not panic and
+    /// must be reported as unguarded (no instructions ahead to look at).
+    #[test]
+    fn is_guarded_false_at_end_of_slice() {
+        let instrs = vec![WasmInstruction::I32Const, WasmInstruction::I64Add];
+        assert!(!ArithmeticCheckRule::is_guarded(&instrs, 1));
+    }
+
     // -----------------------------------------------------------------------
     // Pre-existing tests (unchanged)
     // -----------------------------------------------------------------------

commit 69ce9efc8f5259cf8b147713575d29b98d254816
Author: Depo.dev <depolonedev@outlook.com>
Date:   Mon Mar 23 20:44:00 2026 +0100

    feat: implement unchecked arithmetic analysis rule
    
    Replace placeholder ArithmeticCheckRule with deterministic WASM instruction-level heuristic to detect unsafe arithmetic operations. Implements single-pass linear scan that identifies arithmetic opcodes and checks for nearby control-flow guards (if, br_if, call).
    
    - Add WasmInstruction enum with opcodes for i32/i64 arithmetic and control flow
    - Add parse_instructions() for single-pass instruction decoding
    - Replace no-op analyze_static() with real detection logic
    - Add is_arithmetic() and is_guarded() helper methods
    - Add comprehensive test suite covering positive detection, guarding, and noise rejection

diff --git a/src/analyzer/security.rs b/src/analyzer/security.rs
index 512d95f..cc1dd33 100644
--- a/src/analyzer/security.rs
+++ b/src/analyzer/security.rs
@@ -1,272 +1,318 @@
-use crate::runtime::executor::ContractExecutor;
-use crate::Result;
-use serde::{Deserialize, Serialize};
-use wasmparser::{Parser, Payload};
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub enum Severity {
-    Low,
-    Medium,
-    High,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct SecurityFinding {
-    pub rule_id: String,
-    pub severity: Severity,
-    pub location: String,
-    pub description: String,
-    pub remediation: String,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize, Default)]
-pub struct SecurityReport {
-    pub findings: Vec<SecurityFinding>,
-}
-
-pub trait SecurityRule {
-    fn name(&self) -> &str;
-    fn description(&self) -> &str;
-    fn analyze_static(&self, _wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
-        Ok(vec![])
-    }
-    fn analyze_dynamic(
-        &self,
-        _executor: &ContractExecutor,
-        _trace: &[String],
-    ) -> Result<Vec<SecurityFinding>> {
-        Ok(vec![])
-    }
-}
-
-pub struct SecurityAnalyzer {
-    rules: Vec<Box<dyn SecurityRule>>,
-}
-
-impl SecurityAnalyzer {
-    pub fn new() -> Self {
-        Self {
-            rules: vec![
-                Box::new(HardcodedAddressRule),
-                Box::new(ArithmeticCheckRule),
-                Box::new(AuthorizationCheckRule),
-                Box::new(ReentrancyPatternRule),
-                Box::new(UnboundedIterationRule),
-            ],
-        }
-    }
-
-    pub fn analyze(
-        &self,
-        wasm_bytes: &[u8],
-        executor: Option<&ContractExecutor>,
-        trace: Option<&[String]>,
-    ) -> Result<SecurityReport> {
-        let mut report = SecurityReport::default();
-
-        for rule in &self.rules {
-            // Static analysis
-            let static_findings = rule.analyze_static(wasm_bytes)?;
-            report.findings.extend(static_findings);
-
-            // Dynamic analysis
-            if let (Some(exec), Some(tr)) = (executor, trace) {
-                let dynamic_findings = rule.analyze_dynamic(exec, tr)?;
-                report.findings.extend(dynamic_findings);
-            }
-        }
-
-        Ok(report)
-    }
-}
-
-impl Default for SecurityAnalyzer {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
-// --- Rules ---
-
-struct HardcodedAddressRule;
-impl SecurityRule for HardcodedAddressRule {
-    fn name(&self) -> &str {
-        "hardcoded-address"
-    }
-    fn description(&self) -> &str {
-        "Detects hardcoded addresses in WASM bytes."
-    }
-
-    fn analyze_static(&self, wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
-        let mut findings = Vec::new();
-        // Simple heuristic: look for G... or C... strings of appropriate length
-        // This is a basic implementation.
-        let parser = Parser::new(0);
-        for payload in parser.parse_all(wasm_bytes).flatten() {
-            if let Payload::DataSection(reader) = payload {
-                for data in reader.into_iter().flatten() {
-                    let content = String::from_utf8_lossy(data.data);
-                    // Check for Stellar address patterns (G... or C...)
-                    // Standard Stellar addresses are 56 chars.
-                    for word in content.split(|c: char| !c.is_alphanumeric()) {
-                        if (word.starts_with('G') || word.starts_with('C')) && word.len() == 56 {
-                            findings.push(SecurityFinding {
-                                rule_id: self.name().to_string(),
-                                severity: Severity::Medium,
-                                location: "Data Section".to_string(),
-                                description: format!("Found potential hardcoded address: {}", word),
-                                remediation: "Use Address::from_str from a configuration or argument instead of hardcoding.".to_string(),
-                            });
-                        }
-                    }
-                }
-            }
-        }
-        Ok(findings)
-    }
-}
-
-struct ArithmeticCheckRule;
-impl SecurityRule for ArithmeticCheckRule {
-    fn name(&self) -> &str {
-        "arithmetic-overflow"
-    }
-    fn description(&self) -> &str {
-        "Detects potential for unchecked arithmetic overflow."
-    }
-
-    fn analyze_static(&self, _wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
-        // In WASM, arithmetic is generally "unchecked" (wraps or traps depending on type).
-        // Soroban SDK usually uses checked arithmetic by default, but developers might use raw primitives.
-        // This is hard to detect statically without DWARF info.
-        // For now, we flag use of basic i32/i64 arithmetic opcodes if they seem frequent?
-        // Actually, let's keep it as a placeholder or look for lack of "panic" branches after adds.
-        Ok(vec![])
-    }
-}
-
-struct AuthorizationCheckRule;
-impl SecurityRule for AuthorizationCheckRule {
-    fn name(&self) -> &str {
-        "missing-auth"
-    }
-    fn description(&self) -> &str {
-        "Detects sensitive functions that might be missing authorization checks."
-    }
-
-    fn analyze_dynamic(
-        &self,
-        _executor: &ContractExecutor,
-        _trace: &[String],
-    ) -> Result<Vec<SecurityFinding>> {
-        let mut findings = Vec::new();
-        // Heuristic: If a function writes to storage but no 'require_auth' was seen in the trace.
-        // This requires parsing the diagnostic events / traces.
-        // For now, let's assume 'trace' contains event names.
-        let mut auth_seen = false;
-        let mut storage_write_seen = false;
-
-        for entry in _trace {
-            if entry.contains("require_auth") || entry.contains("authorized") {
-                auth_seen = true;
-            }
-            if entry.contains("contract_storage_put") || entry.contains("contract_storage_update") {
-                storage_write_seen = true;
-            }
-        }
-
-        if storage_write_seen && !auth_seen {
-            findings.push(SecurityFinding {
-                rule_id: self.name().to_string(),
-                severity: Severity::High,
-                location: "Execution Trace".to_string(),
-                description: "Storage mutation detected without preceding authorization check."
-                    .to_string(),
-                remediation: "Ensure all sensitive functions call `address.require_auth()`."
-                    .to_string(),
-            });
-        }
-
-        Ok(findings)
-    }
-}
-
-struct ReentrancyPatternRule;
-impl SecurityRule for ReentrancyPatternRule {
-    fn name(&self) -> &str {
-        "reentrancy-pattern"
-    }
-    fn description(&self) -> &str {
-        "Detects cross-contract calls followed by storage writes."
-    }
-
-    fn analyze_dynamic(
-        &self,
-        _executor: &ContractExecutor,
-        _trace: &[String],
-    ) -> Result<Vec<SecurityFinding>> {
-        let mut findings = Vec::new();
-        let mut cross_call_seen = false;
-
-        for (i, entry) in _trace.iter().enumerate() {
-            if entry.contains("call_contract") || entry.contains("invoke_contract") {
-                cross_call_seen = true;
-            }
-            if cross_call_seen
-                && (entry.contains("contract_storage_put")
-                    || entry.contains("contract_storage_update"))
-            {
-                findings.push(SecurityFinding {
-                    rule_id: self.name().to_string(),
-                    severity: Severity::Medium,
-                    location: format!("Trace line {}", i),
-                    description: "Storage write detected after an external contract call. Possible reentrancy risk.".to_string(),
-                    remediation: "Follow the checks-effects-interactions pattern: update state before making external calls.".to_string(),
-                });
-                // Reset to avoid duplicate flags for the same sequence if desired, or keep flagging.
-            }
-        }
-        Ok(findings)
-    }
-}
-
-struct UnboundedIterationRule;
-impl SecurityRule for UnboundedIterationRule {
-    fn name(&self) -> &str {
-        "unbounded-iteration"
-    }
-    fn description(&self) -> &str {
-        "Detects storage iterations that might be unbounded."
-    }
-
-    fn analyze_static(&self, _wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
-        // Look for loops that call storage get/has in a way that suggests iteration.
-        // Again, hard without control flow graph.
-        Ok(vec![])
-    }
-
-    fn analyze_dynamic(
-        &self,
-        _executor: &ContractExecutor,
-        _trace: &[String],
-    ) -> Result<Vec<SecurityFinding>> {
-        let mut findings = Vec::new();
-        let mut storage_read_count = 0;
-        for entry in _trace {
-            if entry.contains("contract_storage_get") || entry.contains("contract_storage_has") {
-                storage_read_count += 1;
-            }
-        }
-
-        if storage_read_count > 50 {
-            findings.push(SecurityFinding {
-                rule_id: self.name().to_string(),
-                severity: Severity::Low,
-                location: "Execution Trace".to_string(),
-                description: format!("High number of storage reads ({}) detected. Could lead to out-of-gas for large datasets.", storage_read_count),
-                remediation: "Avoid unbounded iteration over storage. Use pagination or mapping where possible.".to_string(),
-            });
-        }
-        Ok(findings)
-    }
-}
+use crate::runtime::executor::ContractExecutor;
+use crate::utils::wasm::{parse_instructions, WasmInstruction};
+use crate::Result;
+use serde::{Deserialize, Serialize};
+use wasmparser::{Parser, Payload};
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub enum Severity {
+    Low,
+    Medium,
+    High,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct SecurityFinding {
+    pub rule_id: String,
+    pub severity: Severity,
+    pub location: String,
+    pub description: String,
+    pub remediation: String,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
+pub struct SecurityReport {
+    pub findings: Vec<SecurityFinding>,
+}
+
+pub trait SecurityRule {
+    fn name(&self) -> &str;
+    fn description(&self) -> &str;
+    fn analyze_static(&self, _wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
+        Ok(vec![])
+    }
+    fn analyze_dynamic(
+        &self,
+        _executor: &ContractExecutor,
+        _trace: &[String],
+    ) -> Result<Vec<SecurityFinding>> {
+        Ok(vec![])
+    }
+}
+
+pub struct SecurityAnalyzer {
+    rules: Vec<Box<dyn SecurityRule>>,
+}
+
+impl SecurityAnalyzer {
+    pub fn new() -> Self {
+        Self {
+            rules: vec![
+                Box::new(HardcodedAddressRule),
+                Box::new(ArithmeticCheckRule),
+                Box::new(AuthorizationCheckRule),
+                Box::new(ReentrancyPatternRule),
+                Box::new(UnboundedIterationRule),
+            ],
+        }
+    }
+
+    pub fn analyze(
+        &self,
+        wasm_bytes: &[u8],
+        executor: Option<&ContractExecutor>,
+        trace: Option<&[String]>,
+    ) -> Result<SecurityReport> {
+        let mut report = SecurityReport::default();
+
+        for rule in &self.rules {
+            // Static analysis
+            let static_findings = rule.analyze_static(wasm_bytes)?;
+            report.findings.extend(static_findings);
+
+            // Dynamic analysis
+            if let (Some(exec), Some(tr)) = (executor, trace) {
+                let dynamic_findings = rule.analyze_dynamic(exec, tr)?;
+                report.findings.extend(dynamic_findings);
+            }
+        }
+
+        Ok(report)
+    }
+}
+
+impl Default for SecurityAnalyzer {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+// --- Rules ---
+
+struct HardcodedAddressRule;
+impl SecurityRule for HardcodedAddressRule {
+    fn name(&self) -> &str {
+        "hardcoded-address"
+    }
+    fn description(&self) -> &str {
+        "Detects hardcoded addresses in WASM bytes."
+    }
+
+    fn analyze_static(&self, wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
+        let mut findings = Vec::new();
+        // Simple heuristic: look for G... or C... strings of appropriate length
+        // This is a basic implementation.
+        let parser = Parser::new(0);
+        for payload in parser.parse_all(wasm_bytes).flatten() {
+            if let Payload::DataSection(reader) = payload {
+                for data in reader.into_iter().flatten() {
+                    let content = String::from_utf8_lossy(data.data);
+                    // Check for Stellar address patterns (G... or C...)
+                    // Standard Stellar addresses are 56 chars.
+                    for word in content.split(|c: char| !c.is_alphanumeric()) {
+                        if (word.starts_with('G') || word.starts_with('C')) && word.len() == 56 {
+                            findings.push(SecurityFinding {
+                                rule_id: self.name().to_string(),
+                                severity: Severity::Medium,
+                                location: "Data Section".to_string(),
+                                description: format!("Found potential hardcoded address: {}", word),
+                                remediation: "Use Address::from_str from a configuration or argument instead of hardcoding.".to_string(),
+                            });
+                        }
+                    }
+                }
+            }
+        }
+        Ok(findings)
+    }
+}
+
+struct ArithmeticCheckRule;
+impl SecurityRule for ArithmeticCheckRule {
+    fn name(&self) -> &str {
+        "arithmetic-overflow"
+    }
+    fn description(&self) -> &str {
+        "Detects potential for unchecked arithmetic overflow."
+    }
+
+    fn analyze_static(&self, wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
+        let mut findings = Vec::new();
+        let instructions = parse_instructions(wasm_bytes);
+
+        for (i, instr) in instructions.iter().enumerate() {
+            if Self::is_arithmetic(instr) && !Self::is_guarded(&instructions, i) {
+                findings.push(SecurityFinding {
+                    rule_id: self.name().to_string(),
+                    severity: Severity::Medium,
+                    location: format!("Instruction {}", i),
+                    description: format!(
+                        "Unchecked arithmetic operation detected: {:?}",
+                        instr
+                    ),
+                    remediation: "Ensure arithmetic operations are guarded with proper bounds checks or overflow handling.".to_string(),
+                });
+            }
+        }
+
+        Ok(findings)
+    }
+}
+
+impl ArithmeticCheckRule {
+    /// Check if an instruction is an arithmetic operation.
+    fn is_arithmetic(instr: &WasmInstruction) -> bool {
+        matches!(
+            instr,
+            WasmInstruction::I32Add
+                | WasmInstruction::I32Sub
+                | WasmInstruction::I32Mul
+                | WasmInstruction::I64Add
+                | WasmInstruction::I64Sub
+                | WasmInstruction::I64Mul
+        )
+    }
+
+    /// Check if an arithmetic operation is guarded by control flow or external function call.
+    fn is_guarded(instructions: &[WasmInstruction], idx: usize) -> bool {
+        let start = idx.saturating_sub(2);
+        let end = (idx + 3).min(instructions.len());
+
+        for i in start..end {
+            match instructions[i] {
+                WasmInstruction::If | WasmInstruction::BrIf | WasmInstruction::Call => {
+                    return true
+                }
+                _ => {}
+            }
+        }
+
+        false
+    }
+}
+
+struct AuthorizationCheckRule;
+impl SecurityRule for AuthorizationCheckRule {
+    fn name(&self) -> &str {
+        "missing-auth"
+    }
+    fn description(&self) -> &str {
+        "Detects sensitive functions that might be missing authorization checks."
+    }
+
+    fn analyze_dynamic(
+        &self,
+        _executor: &ContractExecutor,
+        _trace: &[String],
+    ) -> Result<Vec<SecurityFinding>> {
+        let mut findings = Vec::new();
+        // Heuristic: If a function writes to storage but no 'require_auth' was seen in the trace.
+        // This requires parsing the diagnostic events / traces.
+        // For now, let's assume 'trace' contains event names.
+        let mut auth_seen = false;
+        let mut storage_write_seen = false;
+
+        for entry in _trace {
+            if entry.contains("require_auth") || entry.contains("authorized") {
+                auth_seen = true;
+            }
+            if entry.contains("contract_storage_put") || entry.contains("contract_storage_update") {
+                storage_write_seen = true;
+            }
+        }
+
+        if storage_write_seen && !auth_seen {
+            findings.push(SecurityFinding {
+                rule_id: self.name().to_string(),
+                severity: Severity::High,
+                location: "Execution Trace".to_string(),
+                description: "Storage mutation detected without preceding authorization check."
+                    .to_string(),
+                remediation: "Ensure all sensitive functions call `address.require_auth()`."
+                    .to_string(),
+            });
+        }
+
+        Ok(findings)
+    }
+}
+
+struct ReentrancyPatternRule;
+impl SecurityRule for ReentrancyPatternRule {
+    fn name(&self) -> &str {
+        "reentrancy-pattern"
+    }
+    fn description(&self) -> &str {
+        "Detects cross-contract calls followed by storage writes."
+    }
+
+    fn analyze_dynamic(
+        &self,
+        _executor: &ContractExecutor,
+        _trace: &[String],
+    ) -> Result<Vec<SecurityFinding>> {
+        let mut findings = Vec::new();
+        let mut cross_call_seen = false;
+
+        for (i, entry) in _trace.iter().enumerate() {
+            if entry.contains("call_contract") || entry.contains("invoke_contract") {
+                cross_call_seen = true;
+            }
+            if cross_call_seen
+                && (entry.contains("contract_storage_put")
+                    || entry.contains("contract_storage_update"))
+            {
+                findings.push(SecurityFinding {
+                    rule_id: self.name().to_string(),
+                    severity: Severity::Medium,
+                    location: format!("Trace line {}", i),
+                    description: "Storage write detected after an external contract call. Possible reentrancy risk.".to_string(),
+                    remediation: "Follow the checks-effects-interactions pattern: update state before making external calls.".to_string(),
+                });
+                // Reset to avoid duplicate flags for the same sequence if desired, or keep flagging.
+            }
+        }
+        Ok(findings)
+    }
+}
+
+struct UnboundedIterationRule;
+impl SecurityRule for UnboundedIterationRule {
+    fn name(&self) -> &str {
+        "unbounded-iteration"
+    }
+    fn description(&self) -> &str {
+        "Detects storage iterations that might be unbounded."
+    }
+
+    fn analyze_static(&self, _wasm_bytes: &[u8]) -> Result<Vec<SecurityFinding>> {
+        // Look for loops that call storage get/has in a way that suggests iteration.
+        // Again, hard without control flow graph.
+        Ok(vec![])
+    }
+
+    fn analyze_dynamic(
+        &self,
+        _executor: &ContractExecutor,
+        _trace: &[String],
+    ) -> Result<Vec<SecurityFinding>> {
+        let mut findings = Vec::new();
+        let mut storage_read_count = 0;
+        for entry in _trace {
+            if entry.contains("contract_storage_get") || entry.contains("contract_storage_has") {
+                storage_read_count += 1;
+            }
+        }
+
+        if storage_read_count > 50 {
+            findings.push(SecurityFinding {
+                rule_id: self.name().to_string(),
+                severity: Severity::Low,
+                location: "Execution Trace".to_string(),
+                description: format!("High number of storage reads ({}) detected. Could lead to out-of-gas for large datasets.", storage_read_count),
+                remediation: "Avoid unbounded iteration over storage. Use pagination or mapping where possible.".to_string(),
+            });
+        }
+        Ok(findings)
+    }
+}