From 6f94676ccd5c6756d31f18027be80a82b4deaf40 Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Mon, 23 Mar 2026 22:33:51 +0800 Subject: [PATCH 1/8] fix: align HIGHEST_SEVERITY with ModSecurity behavior Default HIGHEST_SEVERITY to 255 (no severity set) instead of 0 (emergency). Fix comparison to use < instead of > since lower numbers represent higher severity. Use strconv.Atoi instead of ParseRuleSeverity which cannot parse the sentinel value 255. Fixes #1567 Signed-off-by: majiayu000 <1835304752@qq.com> --- internal/corazawaf/transaction.go | 4 ++-- internal/corazawaf/waf.go | 2 +- testing/engine/rulemetadata.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index d8705b902..6933b2791 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -556,8 +556,8 @@ func (tx *Transaction) MatchRule(r *Rule, mds []types.MatchData) { // set highest_severity hs := tx.variables.highestSeverity - maxSeverity, _ := types.ParseRuleSeverity(hs.Get()) - if r.Severity_ > maxSeverity { + currentVal, _ := strconv.Atoi(hs.Get()) + if r.Severity_.Int() < currentVal { hs.Set(strconv.Itoa(r.Severity_.Int())) } diff --git a/internal/corazawaf/waf.go b/internal/corazawaf/waf.go index 54e28eaac..f5a6867dd 100644 --- a/internal/corazawaf/waf.go +++ b/internal/corazawaf/waf.go @@ -259,7 +259,7 @@ func (w *WAF) newTransaction(opts Options) *Transaction { tx.variables.reqbodyProcessorError.Set("0") tx.variables.requestBodyLength.Set("0") tx.variables.duration.Set("0") - tx.variables.highestSeverity.Set("0") + tx.variables.highestSeverity.Set("255") tx.variables.uniqueID.Set(tx.id) tx.setTimeVariables() diff --git a/testing/engine/rulemetadata.go b/testing/engine/rulemetadata.go index 8ba4f6d3d..3ebbc73e1 100644 --- a/testing/engine/rulemetadata.go +++ b/testing/engine/rulemetadata.go @@ -33,3 +33,31 @@ SecAction "id:1, log, severity:5" SecRule HIGHEST_SEVERITY "@eq 5" "id:2, log" `, }) + +var _ = profile.RegisterProfile(profile.Profile{ + Meta: profile.Meta{ + Author: "majiayu000", + Description: "Test HIGHEST_SEVERITY with multiple severities keeps the lowest number", + Enabled: true, + Name: "rulemetadata_highest_severity.yaml", + }, + Tests: []profile.Test{ + { + Title: "highest_severity_multiple", + Stages: []profile.Stage{ + { + Stage: profile.SubStage{ + Output: profile.ExpectedOutput{ + TriggeredRules: []int{1, 2, 3}, + }, + }, + }, + }, + }, + }, + Rules: ` +SecAction "id:1, log, severity:5" +SecAction "id:2, log, severity:2" +SecRule HIGHEST_SEVERITY "@eq 2" "id:3, log" +`, +}) From 698bfd8a06ad4677fadd111fdeabbf7a3ac40344 Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Mon, 23 Mar 2026 22:40:10 +0800 Subject: [PATCH 2/8] fix: add default value test and document severity edge case Add test verifying HIGHEST_SEVERITY defaults to 255 (ModSecurity sentinel) when no rules with severity have fired. Add TODO comment documenting that rules without an explicit severity action have Severity_=0 (Go zero value = Emergency), which may incorrectly update HIGHEST_SEVERITY. Signed-off-by: majiayu000 <1835304752@qq.com> --- internal/corazawaf/transaction.go | 5 +++++ testing/engine/rulemetadata.go | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index 6933b2791..2082c57eb 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -555,6 +555,11 @@ func (tx *Transaction) MatchRule(r *Rule, mds []types.MatchData) { } // set highest_severity + // TODO: Rules without an explicit `severity` action have Severity_=0 + // (Go zero value = RuleSeverityEmergency), which is indistinguishable from + // an explicitly set Emergency severity. This may incorrectly update + // HIGHEST_SEVERITY (e.g., 5→0). A sentinel value or HasSeverity flag on + // RuleMetadata may be needed to distinguish "not set" from "Emergency". hs := tx.variables.highestSeverity currentVal, _ := strconv.Atoi(hs.Get()) if r.Severity_.Int() < currentVal { diff --git a/testing/engine/rulemetadata.go b/testing/engine/rulemetadata.go index 3ebbc73e1..dab431d5c 100644 --- a/testing/engine/rulemetadata.go +++ b/testing/engine/rulemetadata.go @@ -34,6 +34,32 @@ SecRule HIGHEST_SEVERITY "@eq 5" "id:2, log" `, }) +var _ = profile.RegisterProfile(profile.Profile{ + Meta: profile.Meta{ + Author: "majiayu000", + Description: "Test HIGHEST_SEVERITY defaults to 255 when no rules with severity fire", + Enabled: true, + Name: "rulemetadata_default_severity.yaml", + }, + Tests: []profile.Test{ + { + Title: "highest_severity_default", + Stages: []profile.Stage{ + { + Stage: profile.SubStage{ + Output: profile.ExpectedOutput{ + TriggeredRules: []int{1}, + }, + }, + }, + }, + }, + }, + Rules: ` +SecRule HIGHEST_SEVERITY "@eq 255" "id:1, log" +`, +}) + var _ = profile.RegisterProfile(profile.Profile{ Meta: profile.Meta{ Author: "majiayu000", From 08e5bd471cbca9694221d0635faed8a2456eb475 Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Tue, 24 Mar 2026 10:12:37 +0800 Subject: [PATCH 3/8] fix: extract default highest severity to a named const Address review feedback: replace hardcoded "255" with a defaultHighestSeverity const and add ModSecurity v2/v3 source references. Signed-off-by: majiayu000 <1835304752@qq.com> --- internal/corazawaf/waf.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/corazawaf/waf.go b/internal/corazawaf/waf.go index f5a6867dd..5c00f0676 100644 --- a/internal/corazawaf/waf.go +++ b/internal/corazawaf/waf.go @@ -32,6 +32,14 @@ var wafIDCounter atomic.Uint64 const ( // DefaultRequestBodyJsonDepthLimit is the default limit for the depth of JSON objects in the request body DefaultRequestBodyJsonDepthLimit = 1024 + + // defaultHighestSeverity is the default value for HIGHEST_SEVERITY when no rules + // with severity have been matched. Value 255 aligns with ModSecurity behavior: + // - ModSec v2: apache2/msc_util.c highest_severity initialized to 255 + // https://github.com/owasp-modsecurity/ModSecurity/blob/v2/master/apache2/msc_util.c + // - ModSec v3: src/transaction.cc m_highestSeverity initialized to 255 + // https://github.com/owasp-modsecurity/ModSecurity/blob/v3/master/src/transaction.cc + defaultHighestSeverity = "255" ) // WAF instance is used to store configurations and rules @@ -259,7 +267,7 @@ func (w *WAF) newTransaction(opts Options) *Transaction { tx.variables.reqbodyProcessorError.Set("0") tx.variables.requestBodyLength.Set("0") tx.variables.duration.Set("0") - tx.variables.highestSeverity.Set("255") + tx.variables.highestSeverity.Set(defaultHighestSeverity) tx.variables.uniqueID.Set(tx.id) tx.setTimeVariables() From 6b81ee2038c95eaefe4519adeb9beac7768ba39f Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Sat, 28 Mar 2026 22:14:47 +0800 Subject: [PATCH 4/8] fix: skip severity update for rules without explicit severity Rules without a severity action have Severity_=0 (Go zero value), which is indistinguishable from RuleSeverityEmergency. This poisoned HIGHEST_SEVERITY by writing 0 on the first no-severity match, making later explicit severities unable to win. Add HasSeverity_ flag to RuleMetadata, set it in the severity action, and only update HIGHEST_SEVERITY when the flag is true. This mirrors ModSecurity v3 which checks m_severity > -1 before updating. Adds regression test: no-severity rule followed by severity:5 rule must result in HIGHEST_SEVERITY=5, not 0. Signed-off-by: majiayu000 <1835304752@qq.com> --- internal/actions/severity.go | 1 + internal/corazarules/rule.go | 5 +++-- internal/corazawaf/transaction.go | 17 ++++++++--------- testing/engine/rulemetadata.go | 31 +++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/internal/actions/severity.go b/internal/actions/severity.go index 6dd404cd7..f6855063b 100644 --- a/internal/actions/severity.go +++ b/internal/actions/severity.go @@ -46,6 +46,7 @@ func (a *severityFn) Init(r plugintypes.RuleMetadata, data string) error { return err } r.(*corazawaf.Rule).Severity_ = sev + r.(*corazawaf.Rule).HasSeverity_ = true return nil } diff --git a/internal/corazarules/rule.go b/internal/corazarules/rule.go index 09822481e..284020989 100644 --- a/internal/corazarules/rule.go +++ b/internal/corazarules/rule.go @@ -19,8 +19,9 @@ type RuleMetadata struct { File_ string Line_ int Rev_ string - Severity_ types.RuleSeverity - Version_ string + Severity_ types.RuleSeverity + HasSeverity_ bool + Version_ string Tags_ []string Maturity_ int Accuracy_ int diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index 2082c57eb..aa7cbbd86 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -555,15 +555,14 @@ func (tx *Transaction) MatchRule(r *Rule, mds []types.MatchData) { } // set highest_severity - // TODO: Rules without an explicit `severity` action have Severity_=0 - // (Go zero value = RuleSeverityEmergency), which is indistinguishable from - // an explicitly set Emergency severity. This may incorrectly update - // HIGHEST_SEVERITY (e.g., 5→0). A sentinel value or HasSeverity flag on - // RuleMetadata may be needed to distinguish "not set" from "Emergency". - hs := tx.variables.highestSeverity - currentVal, _ := strconv.Atoi(hs.Get()) - if r.Severity_.Int() < currentVal { - hs.Set(strconv.Itoa(r.Severity_.Int())) + // Only update when severity was explicitly set via the severity action. + // This mirrors ModSecurity v3 behavior (m_severity > -1 check in transaction.cc). + if r.HasSeverity_ { + hs := tx.variables.highestSeverity + currentVal, _ := strconv.Atoi(hs.Get()) + if r.Severity_.Int() < currentVal { + hs.Set(strconv.Itoa(r.Severity_.Int())) + } } mr := &corazarules.MatchedRule{ diff --git a/testing/engine/rulemetadata.go b/testing/engine/rulemetadata.go index dab431d5c..34bf211c9 100644 --- a/testing/engine/rulemetadata.go +++ b/testing/engine/rulemetadata.go @@ -60,6 +60,37 @@ SecRule HIGHEST_SEVERITY "@eq 255" "id:1, log" `, }) +// Regression: a rule without severity must not poison HIGHEST_SEVERITY. +// Without the HasSeverity_ guard, the no-severity rule's zero value (0) would +// win over a later explicit severity:5, leaving HIGHEST_SEVERITY stuck at 0. +var _ = profile.RegisterProfile(profile.Profile{ + Meta: profile.Meta{ + Author: "majiayu000", + Description: "Test that rules without severity do not poison HIGHEST_SEVERITY", + Enabled: true, + Name: "rulemetadata_no_severity_poison.yaml", + }, + Tests: []profile.Test{ + { + Title: "no_severity_then_explicit", + Stages: []profile.Stage{ + { + Stage: profile.SubStage{ + Output: profile.ExpectedOutput{ + TriggeredRules: []int{1, 2, 3}, + }, + }, + }, + }, + }, + }, + Rules: ` +SecAction "id:1, log" +SecAction "id:2, log, severity:5" +SecRule HIGHEST_SEVERITY "@eq 5" "id:3, log" +`, +}) + var _ = profile.RegisterProfile(profile.Profile{ Meta: profile.Meta{ Author: "majiayu000", From 06fb118da3ad481d336a837a7d13c9a2a455d1e5 Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Sun, 29 Mar 2026 11:40:34 +0800 Subject: [PATCH 5/8] fix: handle strconv.Atoi error in HIGHEST_SEVERITY tracking Fall back to sentinel value 255 if Atoi fails, so any real severity can still win. In practice this path is unreachable since the variable is always set via strconv.Itoa, but handle it explicitly for correctness. Signed-off-by: majiayu000 <1835304752@qq.com> --- internal/corazawaf/transaction.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index aa7cbbd86..277791c10 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -559,7 +559,12 @@ func (tx *Transaction) MatchRule(r *Rule, mds []types.MatchData) { // This mirrors ModSecurity v3 behavior (m_severity > -1 check in transaction.cc). if r.HasSeverity_ { hs := tx.variables.highestSeverity - currentVal, _ := strconv.Atoi(hs.Get()) + currentVal, err := strconv.Atoi(hs.Get()) + if err != nil { + // HIGHEST_SEVERITY is initialized to "255" and only updated via strconv.Itoa, + // so this should not happen. Fall back to the sentinel to allow any real severity to win. + currentVal = 255 + } if r.Severity_.Int() < currentVal { hs.Set(strconv.Itoa(r.Severity_.Int())) } From 79f79238f5a30406a504a037e6b1a242de45c850 Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Wed, 1 Apr 2026 20:54:20 +0800 Subject: [PATCH 6/8] style: apply gofmt to rule.go Signed-off-by: majiayu000 <1835304752@qq.com> --- internal/corazarules/rule.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/corazarules/rule.go b/internal/corazarules/rule.go index 284020989..9e2fb8c8a 100644 --- a/internal/corazarules/rule.go +++ b/internal/corazarules/rule.go @@ -15,20 +15,20 @@ type RuleMetadata struct { // If the rule is part of a chain, the parent ID is used as log ID. // This approach prevents repeated computations in performance-critical sections, enhancing efficiency. // It is stored for performance reasons, avoiding to perfrom the computation multiple times in the hot path - LogID_ string - File_ string - Line_ int - Rev_ string + LogID_ string + File_ string + Line_ int + Rev_ string Severity_ types.RuleSeverity HasSeverity_ bool Version_ string - Tags_ []string - Maturity_ int - Accuracy_ int - Operator_ string - Phase_ types.RulePhase - Raw_ string - SecMark_ string + Tags_ []string + Maturity_ int + Accuracy_ int + Operator_ string + Phase_ types.RulePhase + Raw_ string + SecMark_ string // Contains the Id of the parent rule if you are inside // a chain. Otherwise, it will be 0 ParentID_ int From 6d71bbc6cd0b3d09ad7b76a5b4864ec85d4f790f Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Wed, 1 Apr 2026 21:55:54 +0800 Subject: [PATCH 7/8] docs: add inline comment for HasSeverity_ field semantics Signed-off-by: majiayu000 <1835304752@qq.com> --- internal/corazarules/rule.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/corazarules/rule.go b/internal/corazarules/rule.go index 9e2fb8c8a..b0f11278d 100644 --- a/internal/corazarules/rule.go +++ b/internal/corazarules/rule.go @@ -15,11 +15,13 @@ type RuleMetadata struct { // If the rule is part of a chain, the parent ID is used as log ID. // This approach prevents repeated computations in performance-critical sections, enhancing efficiency. // It is stored for performance reasons, avoiding to perfrom the computation multiple times in the hot path - LogID_ string - File_ string - Line_ int - Rev_ string - Severity_ types.RuleSeverity + LogID_ string + File_ string + Line_ int + Rev_ string + Severity_ types.RuleSeverity + // HasSeverity_ reports whether the rule explicitly set a severity action. + // This distinguishes an unset value from RuleSeverityEmergency (0). HasSeverity_ bool Version_ string Tags_ []string From ce4e4b7577f5dc2ec752ae961df64be721d9e290 Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Wed, 1 Apr 2026 22:25:07 +0800 Subject: [PATCH 8/8] refactor: change defaultHighestSeverity to int Signed-off-by: majiayu000 <1835304752@qq.com> --- internal/corazawaf/transaction.go | 10 +++++----- internal/corazawaf/waf.go | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index eac33052d..af0daf452 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -559,11 +559,11 @@ func (tx *Transaction) MatchRule(r *Rule, mds []types.MatchData) { // This mirrors ModSecurity v3 behavior (m_severity > -1 check in transaction.cc). if r.HasSeverity_ { hs := tx.variables.highestSeverity - currentVal, err := strconv.Atoi(hs.Get()) - if err != nil { - // HIGHEST_SEVERITY is initialized to "255" and only updated via strconv.Itoa, - // so this should not happen. Fall back to the sentinel to allow any real severity to win. - currentVal = 255 + currentVal := defaultHighestSeverity + if v := hs.Get(); v != "" { + if parsed, err := strconv.Atoi(v); err == nil { + currentVal = parsed + } } if r.Severity_.Int() < currentVal { hs.Set(strconv.Itoa(r.Severity_.Int())) diff --git a/internal/corazawaf/waf.go b/internal/corazawaf/waf.go index 5c00f0676..f72f07889 100644 --- a/internal/corazawaf/waf.go +++ b/internal/corazawaf/waf.go @@ -39,7 +39,7 @@ const ( // https://github.com/owasp-modsecurity/ModSecurity/blob/v2/master/apache2/msc_util.c // - ModSec v3: src/transaction.cc m_highestSeverity initialized to 255 // https://github.com/owasp-modsecurity/ModSecurity/blob/v3/master/src/transaction.cc - defaultHighestSeverity = "255" + defaultHighestSeverity = 255 ) // WAF instance is used to store configurations and rules @@ -267,7 +267,7 @@ func (w *WAF) newTransaction(opts Options) *Transaction { tx.variables.reqbodyProcessorError.Set("0") tx.variables.requestBodyLength.Set("0") tx.variables.duration.Set("0") - tx.variables.highestSeverity.Set(defaultHighestSeverity) + tx.variables.highestSeverity.Set(strconv.Itoa(defaultHighestSeverity)) tx.variables.uniqueID.Set(tx.id) tx.setTimeVariables()