search

corazawaf / coraza

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

Incorrect log message from coraza v3 for CRS rule 920450 #570

Closed rmb122 closed 1 year ago

rmb122 commented 1 year ago

Description

The issue is same as https://github.com/SpiderLabs/ModSecurity/issues/2423. For CRS rule 920450 https://github.com/coreruleset/coreruleset/blob/v4.0/dev/rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf#L1137, the MATCHED_VAR in log is wrong.

Steps to reproduce

run golang program below

package main

import (
    "fmt"
    "log"
    "net/http"
    "os"
    "strings"

    "github.com/corazawaf/coraza/v3"
    txhttp "github.com/corazawaf/coraza/v3/http"
    "github.com/corazawaf/coraza/v3/types"
)

func exampleHandler(w http.ResponseWriter, req *http.Request) {
    w.Header().Set("Content-Type", "text/plain")
    resBody := "Hello world, transaction not disrupted."

    if body := os.Getenv("RESPONSE_BODY"); body != "" {
        resBody = body
    }

    if h := os.Getenv("RESPONSE_HEADERS"); h != "" {
        kv := strings.Split(h, ":")
        w.Header().Set(kv[0], kv[1])
    }

    // The server generates the response
    w.Write([]byte(resBody))
}

func main() {
    waf := createWAF()

    http.Handle("/", txhttp.WrapHandler(waf, txhttp.StdLogger, http.HandlerFunc(exampleHandler)))

    fmt.Println("Server is running. Listening port: 8090")

    log.Fatal(http.ListenAndServe(":8090", nil))
}

func createWAF() coraza.WAF {
    waf, err := coraza.NewWAF(
        coraza.NewWAFConfig().
            WithErrorCallback(logError).
            WithDirectives(`SecDebugLogLevel 5
SecDebugLog /dev/null
SecRuleEngine On

SecRule &TX:restricted_headers "@eq 0" \
    "id:901165,\
    phase:1,\
    pass,\
    nolog,\
    ver:'OWASP_CRS/4.0.0-rc1',\
    setvar:'tx.restricted_headers=/accept-charset/ /content-encoding/ /proxy/ /lock-token/ /content-range/ /if/'"

SecRule REQUEST_HEADERS_NAMES "@rx ^.*$" \
    "id:920450,\
    phase:1,\
    deny,\
    log,auditlog,\
    capture,\
    t:none,t:lowercase,\
    msg:'HTTP header is restricted by policy (%{MATCHED_VAR})',\
    logdata:'Restricted header detected: %{MATCHED_VAR}',\
    severity:'CRITICAL',\
    setvar:'tx.header_name_%{tx.0}=/%{tx.0}/',\
    chain"
    SecRule TX:/^header_name_/ "@within %{tx.restricted_headers}" \
        "status:437"
`),
    )
    if err != nil {
        log.Fatal(err)
    }
    return waf
}

func logError(error types.MatchedRule) {
    msg := error.ErrorLog(0)
    fmt.Printf("[logError][%s] %s", error.Rule().Severity(), msg)
}

curl the server with restricted header

curl 127.0.0.1:8090 -H "proxy: 1"

Expected result

[client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (proxy) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (proxy)"] [data "Restricted header detected: proxy"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"]

Actual result

[logError][critical] [client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (user-agent)"] [data "Restricted header detected: user-agent"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"]
[client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (accept)"] [data "Restricted header detected: accept"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"]
[client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (proxy)"] [data "Restricted header detected: proxy"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"]
[client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (host)"] [data "Restricted header detected: host"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"]
[client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg ""] [data ""] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"]
jcchavezs commented 1 year ago

Ping @M4tteoP

On Thu, 12 Jan 2023, 08:53 rmb122, @.***> wrote:

Description

The issue is same as SpiderLabs/ModSecurity#2423 https://github.com/SpiderLabs/ModSecurity/issues/2423. For CRS rule 920450 https://github.com/coreruleset/coreruleset/blob/v4.0/dev/rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf#L1137, the MATCHED_VAR in log is wrong. Steps to reproduce

run golang program below

package main import ( "fmt" "log" "net/http" "os" "strings"

"github.com/corazawaf/coraza/v3"
txhttp "github.com/corazawaf/coraza/v3/http"
"github.com/corazawaf/coraza/v3/types"

) func exampleHandler(w http.ResponseWriter, req *http.Request) { w.Header().Set("Content-Type", "text/plain") resBody := "Hello world, transaction not disrupted."

if body := os.Getenv("RESPONSE_BODY"); body != "" {
    resBody = body
}

if h := os.Getenv("RESPONSE_HEADERS"); h != "" {
    kv := strings.Split(h, ":")
    w.Header().Set(kv[0], kv[1])
}

// The server generates the response
w.Write([]byte(resBody))

} func main() { waf := createWAF()

http.Handle("/", txhttp.WrapHandler(waf, txhttp.StdLogger, http.HandlerFunc(exampleHandler)))

fmt.Println("Server is running. Listening port: 8090")

log.Fatal(http.ListenAndServe(":8090", nil))

} func createWAF() coraza.WAF { waf, err := coraza.NewWAF( coraza.NewWAFConfig(). WithErrorCallback(logError). WithDirectives(SecDebugLogLevel 5SecDebugLog /dev/nullSecRuleEngine OnSecRule &TX:restricted_headers ***@***.*** 0" \ "id:901165,\ phase:1,\ pass,\ nolog,\ ver:'OWASP_CRS/4.0.0-rc1',\ setvar:'tx.restricted_headers=/accept-charset/ /content-encoding/ /proxy/ /lock-token/ /content-range/ /if/'"SecRule REQUEST_HEADERS_NAMES ***@***.*** ^.*$" \ "id:920450,\ phase:1,\ deny,\ log,auditlog,\ capture,\ t:none,t:lowercase,\ msg:'HTTP header is restricted by policy (%{MATCHED_VAR})',\ logdata:'Restricted header detected: %{MATCHED_VAR}',\ severity:'CRITICAL',\ setvar:'tx.header_name_%{tx.0}=/%{tx.0}/',\ chain" SecRule TX:/^header_name_/ ***@***.*** %{tx.restricted_headers}" \ "status:437"), ) if err != nil { log.Fatal(err) } return waf } func logError(error types.MatchedRule) { msg := error.ErrorLog(0) fmt.Printf("[logError][%s] %s", error.Rule().Severity(), msg) }

curl the server with restricted header

curl 127.0.0.1:8090 -H "proxy: 1"

Expected result

[client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (proxy) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (proxy)"] [data "Restricted header detected: proxy"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"]

Actual result

[logError][critical] [client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (user-agent)"] [data "Restricted header detected: user-agent"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"] [client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (accept)"] [data "Restricted header detected: accept"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"] [client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (proxy)"] [data "Restricted header detected: proxy"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"] [client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg "HTTP header is restricted by policy (host)"] [data "Restricted header detected: host"] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"] [client "127.0.0.1"] Coraza: Warning. HTTP header is restricted by policy (user-agent) [file ""] [line "0"] [id "920450"] [rev ""] [msg ""] [data ""] [severity "critical"] [ver ""] [maturity "0"] [accuracy "0"] [hostname ""] [uri "/"] [unique_id "fmtcIalunsExuoQXkJC"]

— Reply to this email directly, view it on GitHub https://github.com/corazawaf/coraza/issues/570, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAWFQUEC3GTOI5LCO4TWR6Z6XANCNFSM6AAAAAATY52VZ4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

piyushroshan commented 1 year ago

IMHO fix should be in the rule where the msg, logdata should be moved in the chain since that is the blocking criteria. Should give that a try. This works

SecRule REQUEST_HEADERS_NAMES "@rx ^.*$" \
    "id:920450,\
    phase:1,\
    log,auditlog,\
    capture,\
    t:none,t:lowercase,\
    severity:'CRITICAL',\
    setvar:'tx.header_name_%{tx.0}=/%{tx.0}/',\
    chain"
    SecRule TX:/^header_name_/ "@within %{tx.restricted_headers}" \
        "msg:'HTTP header is restricted by policy (%{MATCHED_VAR})',\
        logdata:'Restricted header detected: %{MATCHED_VAR}',\
        deny,\
        status:437"
jcchavezs commented 1 year ago

Ping @fzipi

fzipi commented 1 year ago

Well, according to https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual-(v2.x)#user-content-chain, metadata actions can be only in the first rule in the chain. 🤷

Also note that disruptive actions, execution phases, metadata actions (id, rev, msg, tag, severity, logdata), skip, and skipAfter actions can be specified only by the chain starter rule.

jptosso commented 1 year ago

Well, according to https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual-(v2.x)#user-content-chain, metadata actions can be only in the first rule in the chain. 🤷

Also note that disruptive actions, execution phases, metadata actions (id, rev, msg, tag, severity, logdata), skip, and skipAfter actions can be specified only by the chain starter rule.

I think we can change this in coraza without affecting regression. It does make sense to change metadata in different chain stages. It could be allowed for certain actions, we could even add a new group like "conditional actions" or chained actions

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 30 days with no activity.

github-actions[bot] commented 1 year ago

This issue was closed because it has been inactive for 14 days since being marked as stale.