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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user