search

corazawaf / coraza

OWASP Coraza WAF is a golang modsecurity compatible web application firewall library
https://www.coraza.io
Apache License 2.0
2.29k stars 226 forks source link

Race condition in RuleMetadata.StrID() #1083

Closed MarcWort closed 5 months ago

MarcWort commented 5 months ago

Description

The Go Race Detector pointed out a data race on the new RuleMetadata.StrID() introduced in https://github.com/corazawaf/coraza/pull/1039

Steps to reproduce

Running tests which contain t.Parallel() to simulate multiple requests with the race detection enabled: go test -race

  WARNING: DATA RACE
 Read at 0x00c0021d1af0 by goroutine 31:
   github.com/corazawaf/coraza/v3/internal/corazarules.(*RuleMetadata).StrID()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazarules/rule.go:89 +0x44
   github.com/corazawaf/coraza/v3/internal/corazawaf.(*Rule).doEvaluate()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazawaf/rule.go:197 +0x1eb
   github.com/corazawaf/coraza/v3/internal/corazawaf.(*Rule).Evaluate()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazawaf/rule.go:179 +0x42d
   github.com/corazawaf/coraza/v3/internal/corazawaf.(*RuleGroup).Eval()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazawaf/rulegroup.go:219 +0x9a4
   github.com/corazawaf/coraza/v3/internal/corazawaf.(*Transaction).ProcessRequestBody()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazawaf/transaction.go:1047 +0x990
   mypackage.example.com/work/scrubbing/coraza-spoa.(*handler).requestAllowed()
       /src/coraza-spoa/waf.go:224 +0x585

 Previous write at 0x00c0021d1af0 by goroutine 41:
   github.com/corazawaf/coraza/v3/internal/corazarules.(*RuleMetadata).StrID()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazarules/rule.go:94 +0xa8
   github.com/corazawaf/coraza/v3/internal/corazawaf.(*Rule).doEvaluate()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazawaf/rule.go:197 +0x1eb
   github.com/corazawaf/coraza/v3/internal/corazawaf.(*Rule).Evaluate()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazawaf/rule.go:179 +0x42d
   github.com/corazawaf/coraza/v3/internal/corazawaf.(*RuleGroup).Eval()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazawaf/rulegroup.go:219 +0x9a4
   github.com/corazawaf/coraza/v3/internal/corazawaf.(*Transaction).ProcessRequestBody()
       /go/pkg/mod/github.com/corazawaf/coraza/v3@v3.2.0/internal/corazawaf/transaction.go:1047 +0x990
   mypackage.example.com/work/scrubbing/coraza-spoa.(*handler).requestAllowed()
       /src/coraza-spoa/waf.go:224 +0x585
jcchavezs commented 5 months ago

Thanks for the report, this was introduced in https://github.com/corazawaf/coraza/pull/1039/files#diff-397d8f5c981e6e197286890a559690b982dd806cd5873b8c8b9223cbe439717cR88-R97 I think.

We need to cut a path release fixing this and probably attempt to run tests in parallel cc @M4tteoP