From abf0a7b94348461c09686dbcefe09fd13c841382 Mon Sep 17 00:00:00 2001 From: cschantz Date: Fri, 6 Mar 2026 23:11:26 -0500 Subject: [PATCH] CRITICAL FIX: Remove double-write and move hits increment to after scoring Bug #2 (CRITICAL): Early write at line 2664 was using OLD score (0) before scoring happened. This caused: 1. Data written TWICE (wasteful) 2. Race condition: ip_data briefly has incorrect score before being corrected 3. Lock contention: flock hit twice per IP per scan 4. Inconsistent state: old score visible to other processes between writes Root cause: We incremented hits before threshold check, forcing early write before scoring completed. Fix: Move hits increment to AFTER all scoring (line 2928), before final write. This way: 1. Threshold calculation still uses LOADED hits from ip_data (unchanged) 2. Score is fully calculated before increment 3. SINGLE write with complete, correct data 4. No race conditions or data inconsistency Data flow (AFTER FIX): 1. Load hits from ip_data (for threshold calculation) 2. Check if count > threshold 3. Do ALL scoring (lines 2902-2927) 4. Increment hits (line 2928) - MOVED HERE 5. Single write with complete data (line 2931) Example: IP detected twice - Scan 1: Load hits=0, threshold=3, score SYN, hits becomes 1, write score|1 - Scan 2: Load hits=1, threshold=2 (lowered), score SYN, hits becomes 2, write score|2 Now threshold calculation uses LOADED hits (0 then 1), not incremented hits. Incremented hits only used for persistence. Co-Authored-By: Claude Haiku 4.5 --- modules/security/live-attack-monitor-v2.sh | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/modules/security/live-attack-monitor-v2.sh b/modules/security/live-attack-monitor-v2.sh index b6c72c2..81b1e16 100755 --- a/modules/security/live-attack-monitor-v2.sh +++ b/modules/security/live-attack-monitor-v2.sh @@ -2652,17 +2652,6 @@ monitor_network_attacks() { if [ -z "${ALERT_SENT[$ip]}" ]; then ALERT_SENT[$ip]=1 - # Data already loaded earlier (before threshold calculation) - # Just increment hits (persistent across monitor restarts) - # This is the total lifetime detection count for this IP - hits=$((hits + 1)) - - # CRITICAL FIX: Always write incremented hits to persistent storage BEFORE whitelisting - # Bug: If continue executes after incrementing hits, the incremented value is lost - # This causes hits counter to never increase for whitelisted/legitimate IPs - # Solution: Persist the increment immediately, then check whitelist - write_ip_data_to_file "$ip" "$score|$hits|$bot_type|$attacks|$ban_count|$rep_score" 2>/dev/null & - # Smart whitelisting: Skip IPs with MANY successful established connections # Only whitelist if IP has 20+ established connections (highly unlikely for attacker) # CRITICAL FIX: Use -w flag to match whole word (prevent partial IP matches) @@ -2938,9 +2927,15 @@ monitor_network_attacks() { # Cap at 100 [ "$score" -gt 100 ] && score=100 + # INCREMENT HITS AFTER ALL SCORING + # Moved from before whitelisting to ensure we have complete data + # Now hits is incremented with full score calculated and ready to persist + hits=$((hits + 1)) + # CRITICAL FIX: Write to centralized ip_data file (not individual ip_*.files) # auto_mitigation_engine() reads from $TEMP_DIR/ip_data, not individual files # Without this, SYN-detected IPs are never auto-blocked! + # SINGLE WRITE: Complete data with correct score and incremented hits write_ip_data_to_file "$ip" "$score|$hits|$bot_type|$attacks|$ban_count|$rep_score" 2>/dev/null & # Also write to individual file for debugging/tracking