Core Rule Set reads request bodies up to SWWAF_WAF_BODY_LIMIT (closes #116)

SWWAF_WAF_BODY_LIMIT (default off) has the Core Rule Set read form data
and multipart up to the limit, the rest streaming on, and JSON and XML
no larger than it; other bodies pass uninspected. The part read is held
and sent to the app first. A size or time limit met while it is read
ends the request before it reaches the app. Content-Encoding is refused
again on those four kinds. Rule 900300 moves to phase 2, to count form
and JSON fields past Coraza's 1000 too.

Judgement call: Content-Encoding is refused on a JSON or XML body too
large to be read, which SPEC.md allows.

Model: opus-5-5

internal/config/config.go

@@ -244,14 +244,16 @@ type Config struct {
 	// level, from 1 to 4 (SWWAF_WAF_PARANOIA_LEVEL), and
 	// WAFAnomalyThreshold the anomaly score at which a request is a match
 	// (SWWAF_WAF_ANOMALY_THRESHOLD), 0 while it is off. WAFDisabledRules
-	// are the ids of its rules switched off (SWWAF_WAF_DISABLED_RULES), and
+	// are the ids of its rules switched off (SWWAF_WAF_DISABLED_RULES),
 	// WAFExemptPaths the path prefixes it does not inspect
-	// (SWWAF_WAF_EXEMPT_PATHS).
+	// (SWWAF_WAF_EXEMPT_PATHS), and WAFBodyLimit the most of a request body
+	// it reads (SWWAF_WAF_BODY_LIMIT), 0 while it is off and it reads none.
 	WAFMode             string
 	WAFParanoiaLevel    int
 	WAFAnomalyThreshold int
 	WAFDisabledRules    []int
 	WAFExemptPaths      []string
+	WAFBodyLimit        int64
 	// TrapPaths are the paths a request for which is a clear sign of
 	// attack (SWWAF_TRAP_PATHS), each starting with / and without a ?.
 	TrapPaths []string
@@ -559,6 +561,7 @@ func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) {
 		WAFDisabledRules: env.ruleIDs("SWWAF_WAF_DISABLED_RULES",
 			"920340,920420,920440,920640,930130,930140"),
 		WAFExemptPaths:       env.pathPrefixes("SWWAF_WAF_EXEMPT_PATHS", ""),
+		WAFBodyLimit:         env.size("SWWAF_WAF_BODY_LIMIT", off),
 		TrapPaths:            env.trapPaths("SWWAF_TRAP_PATHS"),
 		ErrorBurstThreshold:  env.count("SWWAF_ERROR_BURST_THRESHOLD", "30"),
 		LogRemoteURL:         env.logRemoteURL("SWWAF_LOG_REMOTE_URL"),

internal/config/config_test.go

@@ -96,6 +96,7 @@ const (
 	wafAnomalyThreshold     = "SWWAF_WAF_ANOMALY_THRESHOLD"
 	wafDisabledRules        = "SWWAF_WAF_DISABLED_RULES"
 	wafExemptPaths          = "SWWAF_WAF_EXEMPT_PATHS"
+	wafBodyLimit            = "SWWAF_WAF_BODY_LIMIT"
 	trapPaths               = "SWWAF_TRAP_PATHS"
 	errorBurstThreshold     = "SWWAF_ERROR_BURST_THRESHOLD"
 	logRemoteURL            = "SWWAF_LOG_REMOTE_URL"
@@ -580,15 +581,20 @@ func TestCoreRuleSetSettings(t *testing.T) {
 			environment{
 				wafMode: config.WAFModeDetect, wafParanoiaLevel: "4", wafAnomalyThreshold: "10",
 				wafDisabledRules: "942100, 920350", wafExemptPaths: "/api/, /static/",
+				wafBodyLimit: "128K",
 			},
 			config.Config{
 				WAFMode: config.WAFModeDetect, WAFParanoiaLevel: 4, WAFAnomalyThreshold: 10,
 				WAFDisabledRules: []int{942100, 920350},
 				WAFExemptPaths:   []string{"/api/", "/static/"},
+				WAFBodyLimit:     128 << 10,
 			},
 		},
 		{
-			environment{wafMode: off, wafAnomalyThreshold: off, wafDisabledRules: ""},
+			environment{
+				wafMode: off, wafAnomalyThreshold: off, wafDisabledRules: "",
+				wafBodyLimit: off,
+			},
 			config.Config{
 				WAFMode: config.WAFModeOff, WAFParanoiaLevel: 1, WAFAnomalyThreshold: 0,
 				WAFDisabledRules: []int{}, WAFExemptPaths: []string{},
@@ -601,6 +607,7 @@ func TestCoreRuleSetSettings(t *testing.T) {
 			WAFMode: cfg.WAFMode, WAFParanoiaLevel: cfg.WAFParanoiaLevel,
 			WAFAnomalyThreshold: cfg.WAFAnomalyThreshold,
 			WAFDisabledRules:    cfg.WAFDisabledRules, WAFExemptPaths: cfg.WAFExemptPaths,
+			WAFBodyLimit: cfg.WAFBodyLimit,
 		}
 		if !reflect.DeepEqual(got, tc.want) {
 			t.Errorf("%v gave\n%+v\nwant\n%+v", tc.env, got, tc.want)
@@ -646,6 +653,10 @@ func TestInvalidCoreRuleSetSettingStopsTheStart(t *testing.T) {
 			wafExemptPaths, "api/",
 			`"api/" is not a path prefix starting with /, such as /assets/`,
 		},
+		{
+			wafBodyLimit, "128KB",
+			`"128KB" is not a size such as 512K, 100M or 5G, or off`,
+		},
 	} {
 		t.Run(tc.name+"="+tc.value, func(t *testing.T) {
 			t.Parallel()
@@ -2403,6 +2414,7 @@ func TestLogsEachSettingWithItsValue(t *testing.T) {
 		wafAnomalyThreshold:     "5",
 		wafDisabledRules:        defaultWAFDisabledRules,
 		wafExemptPaths:          "",
+		wafBodyLimit:            off,
 		trapPaths:               "",
 		errorBurstThreshold:     "30",
 		logRemoteURL:            "",

internal/proxy/bodies.go

@@ -21,6 +21,9 @@ type requestBody struct {
 	// SWWAF_REQUEST_MAX_BYTES.
 	body io.ReadCloser
 	rq   *request
+	// readByCoreRuleSet is what the Core Rule Set read of the body before
+	// the request went to the app, and Read gives first.
+	readByCoreRuleSet []byte
 	// waiting is true while a Read waits for the client to send more.
 	waiting atomic.Bool
 	// received is true once the client has sent the whole body.
@@ -29,8 +32,16 @@ type requestBody struct {
 	bytes atomic.Int64
 }
 
-// Read reads from the client's body.
+// Read reads from the client's body, after what the Core Rule Set read of
+// it, which has been counted already.
 func (b *requestBody) Read(p []byte) (int, error) {
+	if len(b.readByCoreRuleSet) > 0 {
+		n := copy(p, b.readByCoreRuleSet)
+		b.readByCoreRuleSet = b.readByCoreRuleSet[n:]
+
+		return n, nil
+	}
+
 	b.waiting.Store(true)
 	n, err := b.body.Read(p)
 	b.waiting.Store(false)

internal/proxy/coreruleset.go

@@ -1,6 +1,9 @@
 package proxy
 
 import (
+	"errors"
+	"net/http"
+	"os"
 	"time"
 
 	"sneak.berlin/go/smallwebwaf/internal/alerts"
@@ -16,7 +19,8 @@ import (
 // SWWAF_WAF_ANOMALY_THRESHOLD is a match: it raises the waf_block alert,
 // and in block mode refuses the request, which is an offence its client's
 // history counts, and so returns ActionWAFBlocked. It returns "" for a
-// request it does not refuse.
+// request it does not refuse, and for one whose body meets a size or time
+// limit while the Core Rule Set reads it, which it notes nothing of.
 func (rq *request) checkCoreRuleSet() string {
 	cfg := rq.h.config
 	if cfg.WAFMode == config.WAFModeOff || pathExempt(rq.in.URL, cfg.WAFExemptPaths) {
@@ -24,7 +28,12 @@ func (rq *request) checkCoreRuleSet() string {
 	}
 
 	start := time.Now()
-	result := rq.h.coreRuleSet.Inspect(rq.in, rq.client)
+
+	result := rq.inspect()
+	if rq.refused.Load() != nil {
+		return "" // the refusal for that limit, which check returns
+	}
+
 	rq.line.DurationWAF = new(requestlog.Milliseconds(time.Since(start)))
 	rq.line.WAFRuleIDs = result.RuleIDs
 	rq.line.WAFScore = &result.Score
@@ -49,6 +58,39 @@ func (rq *request) checkCoreRuleSet() string {
 	return requestlog.ActionWAFBlocked
 }
 
+// inspect runs the Core Rule Set on the request, which reads the part of
+// its body it inspects within SWWAF_CLIENT_REQUEST_TIMEOUT, and keeps that
+// part for the app. A client that runs out of time is refused with 408
+// here, and a body over SWWAF_REQUEST_MAX_BYTES with 413 as it is read;
+// check returns the refusal. A body that breaks off for any other reason
+// is passed on as far as it came, and the request to the app fails there,
+// as it would have without the Core Rule Set.
+func (rq *request) inspect() waf.Result {
+	if rq.body == nil {
+		// Nothing is read of no body, so nothing can go wrong reading it.
+		result, _, _ := rq.h.coreRuleSet.Inspect(rq.in, rq.client, http.NoBody)
+
+		return result
+	}
+
+	_ = rq.rc.SetReadDeadline(rq.clientRequestDeadline())
+	result, read, err := rq.h.coreRuleSet.Inspect(rq.in, rq.client, rq.body)
+	// The timeouts that run while the request goes to the app take over.
+	_ = rq.rc.SetReadDeadline(time.Time{})
+
+	rq.body.readByCoreRuleSet = read
+
+	if errors.Is(err, os.ErrDeadlineExceeded) {
+		rq.refuse(refusal{
+			status: http.StatusRequestTimeout,
+			action: requestlog.ActionTimedOut,
+			limit:  "SWWAF_CLIENT_REQUEST_TIMEOUT",
+		})
+	}
+
+	return result
+}
+
 // alertWAFBlock raises the waf_block alert for the request, which the Core
 // Rule Set scored at result, at or over SWWAF_WAF_ANOMALY_THRESHOLD. Its
 // detail gives the rule ids, the score, the method and the path with the

internal/proxy/coreruleset_test.go

@@ -1,11 +1,14 @@
 package proxy_test
 
 import (
+	"io"
 	"net/http"
 	"net/netip"
 	"slices"
+	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"sneak.berlin/go/smallwebwaf/internal/alerts"
 	"sneak.berlin/go/smallwebwaf/internal/ratelimit"
@@ -18,10 +21,14 @@ const (
 	wafAnomalyThreshold = "SWWAF_WAF_ANOMALY_THRESHOLD"
 	wafDisabledRules    = "SWWAF_WAF_DISABLED_RULES"
 	wafExemptPaths      = "SWWAF_WAF_EXEMPT_PATHS"
+	wafBodyLimit        = "SWWAF_WAF_BODY_LIMIT"
 	block               = "block"
 	detect              = "detect"
 )
 
+// formData is the type of a form's body.
+const formData = "application/x-www-form-urlencoded"
+
 // sqlInjection asks for / with an SQL injection in its query, which only
 // the Core Rule Set's rule 942100 matches, with a score of 5, the default
 // SWWAF_WAF_ANOMALY_THRESHOLD.
@@ -224,6 +231,197 @@ func TestDisabledRulesSwitchOffWhatGiteaWouldBeRefused(t *testing.T) {
 	}
 }
 
+func TestAttackInAFormBodyIsRefusedOnlyWhileBodiesAreRead(t *testing.T) {
+	t.Parallel()
+
+	const body = "id=1'%20OR%20'1'='1"
+
+	header := "Content-Type: " + formData + "\r\nContent-Length: " +
+		strconv.Itoa(len(body))
+
+	s, _, _ := startWithClock(t, "", map[string]string{wafMode: block})
+	line, _ := s.requestWithBody(http.MethodPost, client, "/", header, body,
+		http.StatusOK, requestlog.ActionForward)
+	wantWAF(t, line, new(0))
+
+	s, _, _ = startWithClock(t, "", map[string]string{
+		wafMode: block, wafBodyLimit: sizeLimitSetting,
+	})
+	line, _ = s.requestWithBody(http.MethodPost, client, "/", header, body,
+		http.StatusForbidden, requestlog.ActionWAFBlocked)
+	wantWAF(t, line, new(5), 942100)
+}
+
+func TestBodiesReachTheAppAsSentWhileBodiesAreRead(t *testing.T) {
+	t.Parallel()
+
+	// The app answers with the body it was sent, once it has the whole of
+	// it: Go's server reads no more of a body once the answer has begun.
+	app := startApp(t, func(w http.ResponseWriter, r *http.Request) {
+		body, _ := io.ReadAll(r.Body)
+		_, _ = w.Write(body)
+	})
+	addr, out := startProxy(t, app.URL, map[string]string{
+		wafMode: block, wafBodyLimit: sizeLimitSetting,
+	})
+	longer := "a=" + strings.Repeat("b", 64*sizeLimit)
+
+	for i, tc := range []struct {
+		name, contentType, body string
+		// announced sends the body's length in Content-Length; otherwise
+		// the body is sent in chunks with no length given.
+		announced bool
+	}{
+		{"form data within the limit", formData, "a=b", true},
+		{"form data longer than the limit", formData, longer, true},
+		{"form data longer than the limit, not announced", formData, longer, false},
+		{
+			"JSON larger than the limit", "application/json",
+			`{"a":"` + strings.Repeat("b", 2*sizeLimit) + `"}`, true,
+		},
+		{
+			"a binary body", "application/octet-stream",
+			strings.Repeat("\x00\xff", sizeLimit), true,
+		},
+	} {
+		// A reader whose length the client cannot tell is sent in chunks.
+		var body io.Reader = strings.NewReader(tc.body)
+		if !tc.announced {
+			body = io.MultiReader(body)
+		}
+
+		req := newRequest(t, http.MethodPost, addr, "/", body)
+		req.Header.Set("Content-Type", tc.contentType)
+
+		got := do(t, req)
+		if got.status != http.StatusOK || string(got.body) != tc.body {
+			t.Errorf("%s: the app got %d bytes, answered %d, want the %d sent, 200",
+				tc.name, len(got.body), got.status, len(tc.body))
+		}
+
+		line := out.requestLines(t, i+1)[i]
+		wantLine(t, line, http.StatusOK, requestlog.ActionForward)
+
+		if line.RequestBytes != int64(len(tc.body)) {
+			t.Errorf("%s: log line has request_bytes %d, want %d", tc.name,
+				line.RequestBytes, len(tc.body))
+		}
+	}
+}
+
+func TestFormBodyLongerThanTheLimitStreamsOnToTheApp(t *testing.T) {
+	t.Parallel()
+
+	const (
+		first = "a=" // and twice the limit of b's, then the rest
+		rest  = 64 * sizeLimit
+	)
+
+	// past is closed once the app has received twice what the Core Rule
+	// Set reads, and got is the length of the whole body it received.
+	past := make(chan struct{})
+	got := make(chan int64, 1)
+	app := startApp(t, func(_ http.ResponseWriter, r *http.Request) {
+		n, _ := io.CopyN(io.Discard, r.Body, 2*sizeLimit)
+
+		close(past)
+
+		m, _ := io.Copy(io.Discard, r.Body)
+		got <- n + m
+	})
+	addr, out := startProxy(t, app.URL, map[string]string{
+		wafMode: block, wafBodyLimit: sizeLimitSetting,
+	})
+
+	// The client sends the rest only once the app has received the first
+	// part: were smallwebwaf to hold the body until the end, it would
+	// never come.
+	body, sender := io.Pipe()
+
+	go func() {
+		_, _ = io.WriteString(sender, first+strings.Repeat("b", 2*sizeLimit))
+
+		select {
+		case <-past:
+		case <-time.After(waitLimit):
+			t.Error("the app got no more than the Core Rule Set reads " +
+				"before the whole body was sent")
+
+			_ = sender.CloseWithError(io.ErrUnexpectedEOF)
+
+			return
+		}
+
+		_, _ = io.WriteString(sender, strings.Repeat("b", rest))
+		_ = sender.Close()
+	}()
+
+	req := newRequest(t, http.MethodPost, addr, "/", body)
+	req.Header.Set("Content-Type", formData)
+	wantStatus(t, do(t, req), http.StatusOK)
+
+	want := int64(len(first) + 2*sizeLimit + rest)
+	if n := <-got; n != want {
+		t.Errorf("the app got %d bytes, want %d", n, want)
+	}
+
+	wantLine(t, out.requestLine(t), http.StatusOK, requestlog.ActionForward)
+}
+
+func TestClientTooSlowToSendWhatTheCoreRuleSetReads(t *testing.T) {
+	t.Parallel()
+
+	app := startApp(t, func(http.ResponseWriter, *http.Request) {})
+	addr, out := startProxy(t, app.URL, map[string]string{
+		wafMode: block, wafBodyLimit: sizeLimitSetting,
+		clientRequestTimeout: shortTimeoutSetting, metricsToken: token,
+	})
+
+	conn := dial(t, addr)
+	send(t, conn, "POST /comment HTTP/1.1\r\nHost: app\r\nContent-Type: "+formData+
+		"\r\nContent-Length: 100\r\n\r\ncontent=the first bytes")
+
+	wantStatus(t, readResponse(t, conn), http.StatusRequestTimeout)
+
+	line := out.requestLine(t)
+	wantLine(t, line, http.StatusRequestTimeout, requestlog.ActionTimedOut)
+	wantNotSentToTheApp(t, line)
+	wantLimitHits(t, addr, clientRequestTimeout, 1)
+}
+
+func TestBodyOverTheSizeLimitWhileTheCoreRuleSetReadsIt(t *testing.T) {
+	t.Parallel()
+
+	app := startApp(t, func(http.ResponseWriter, *http.Request) {})
+	addr, out := startProxy(t, app.URL, map[string]string{
+		wafMode: block, wafBodyLimit: "4K",
+		requestMaxBytes: sizeLimitSetting, metricsToken: token,
+	})
+
+	// Sent in chunks, its length is not announced, and is found to be over
+	// the limit as the Core Rule Set reads it.
+	body := io.MultiReader(strings.NewReader("a=" + strings.Repeat("b", 2*sizeLimit)))
+	req := newRequest(t, http.MethodPost, addr, "/", body)
+	req.Header.Set("Content-Type", formData)
+	wantStatus(t, do(t, req), http.StatusRequestEntityTooLarge)
+
+	line := out.requestLine(t)
+	wantLine(t, line, http.StatusRequestEntityTooLarge, requestlog.ActionTooLarge)
+	wantNotSentToTheApp(t, line)
+	wantLimitHits(t, addr, requestMaxBytes, 1)
+}
+
+// wantNotSentToTheApp checks that the request of line was not sent to the
+// app at all.
+func wantNotSentToTheApp(t *testing.T, line logLine) {
+	t.Helper()
+
+	_, sent := line.fields["duration_upstream_total"]
+	if sent {
+		t.Error("log line has duration_upstream_total, for a request sent to the app")
+	}
+}
+
 func TestResponsesAreNotInspected(t *testing.T) {
 	t.Parallel()
 

internal/proxy/proxy.go

@@ -232,8 +232,8 @@ func newReputation(
 }
 
 // newCoreRuleSet returns the Core Rule Set at SWWAF_WAF_PARANOIA_LEVEL,
-// without the rules SWWAF_WAF_DISABLED_RULES switches off, or nil while
-// SWWAF_WAF_MODE is off.
+// without the rules SWWAF_WAF_DISABLED_RULES switches off, reading bodies
+// up to SWWAF_WAF_BODY_LIMIT, or nil while SWWAF_WAF_MODE is off.
 func newCoreRuleSet(cfg *config.Config) *waf.CoreRuleSet {
 	if cfg.WAFMode == config.WAFModeOff {
 		return nil
@@ -241,6 +241,7 @@ func newCoreRuleSet(cfg *config.Config) *waf.CoreRuleSet {
 
 	coreRuleSet, err := waf.New(waf.Params{
 		ParanoiaLevel: cfg.WAFParanoiaLevel, DisabledRules: cfg.WAFDisabledRules,
+		BodyLimit: cfg.WAFBodyLimit,
 	})
 	if err != nil {
 		// The Core Rule Set is built in, and the settings cannot break it:

internal/proxy/request.go

@@ -188,16 +188,23 @@ func requestHeaders(r *http.Request, names []string) map[string]string {
 }
 
 // check is the one place where a request can be refused once its client
-// is known, before its body is read or anything reaches the app. It
-// returns nil to let the request through. The checks of checkClient come
-// first, answered with SWWAF_BAN_RESPONSE, or 403 for a block rule or the
-// Core Rule Set, and then the size limit, so that a request the rate
-// limits count is counted even when it is refused for its size. In
-// observe mode a request checkClient refuses goes on to the size limit
-// like any other. ctx is the request's own context.
+// is known, before anything reaches the app, and before its body is read,
+// but for the part the Core Rule Set reads. It returns nil to let the
+// request through. The checks of checkClient come first, answered with
+// SWWAF_BAN_RESPONSE, or 403 for a block rule or the Core Rule Set, and
+// then the size limit, so that a request the rate limits count is counted
+// even when it is refused for its size. In observe mode a request
+// checkClient refuses goes on to the size limit like any other. A size or
+// time limit the Core Rule Set's reading of the body meets ends the
+// request in either mode. ctx is the request's own context.
 func (rq *request) check(ctx context.Context) *refusal {
 	action := rq.checkClient(ctx)
 
+	refused := rq.refused.Load()
+	if refused != nil {
+		return refused
+	}
+
 	switch {
 	case action == "":
 	case rq.h.config.Observe:

internal/waf/waf.go

@@ -1,12 +1,13 @@
 // Package waf runs the OWASP Core Rule Set 4.25.0, through Coraza, on the
-// method, the URL with its query and the headers of a request, with the
-// six changes smallwebwaf makes to it, as "Attack detection" under
-// "Configuration surface" in SPEC.md describes them. It reads no request
-// body and no response.
+// method, the URL with its query and the headers of a request, and on its
+// body while SWWAF_WAF_BODY_LIMIT is set, with the six changes smallwebwaf
+// makes to it, as "Attack detection" under "Configuration surface" in
+// SPEC.md describes them. It reads no response.
 package waf
 
 import (
 	"fmt"
+	"io"
 	"net/http"
 	"net/netip"
 	"slices"
@@ -20,15 +21,16 @@ import (
 )
 
 // directives are the Core Rule Set as smallwebwaf runs it, with the
-// paranoia level for %d. Each rule smallwebwaf adds has an id from 900000
-// to 900999, the ids the Core Rule Set keeps for the rules that set it
-// up, which SWWAF_WAF_DISABLED_RULES refuses, so that no setting switches
-// one off. Coraza joins a line ending in \ to the next, without the spaces
-// at the start of the next.
+// paranoia level for %d, and bodyDirectives for %s while
+// SWWAF_WAF_BODY_LIMIT is set. Each rule smallwebwaf adds has an id from
+// 900000 to 900999, the ids the Core Rule Set keeps for the rules that set
+// it up, which SWWAF_WAF_DISABLED_RULES refuses, so that no setting
+// switches one off. Coraza joins a line ending in \ to the next, without
+// the spaces at the start of the next.
 const directives = `
 # The engine only detects. smallwebwaf compares the request's anomaly
 # score with SWWAF_WAF_ANOMALY_THRESHOLD itself, in block and detect mode
-# alike. It reads no body.
+# alike. It reads no body, unless bodyDirectives switch that on.
 SecRuleEngine DetectionOnly
 SecRequestBodyAccess Off
 SecResponseBodyAccess Off
@@ -44,12 +46,23 @@ SecAction "id:900200,phase:1,pass,nolog,\
     setvar:'tx.allowed_methods=GET HEAD POST OPTIONS PUT PATCH DELETE'"
 
 # The second: Expect and Content-Encoding are taken off the Core Rule Set's
-# list of the headers it refuses. Content-Encoding stays refused on a body
-# the Core Rule Set reads, and it reads none.
+# list of the headers it refuses. Content-Encoding goes back on it for a
+# body the Core Rule Set reads (900260 in bodyDirectives).
 SecAction "id:900250,phase:1,pass,nolog,\
     setvar:'tx.restricted_headers_basic=/proxy/ /lock-token/ /content-range/ \
     /if/ /x-http-method-override/ /x-http-method/ /x-method-override/ \
     /x-middleware-subrequest/'"
+%s
+# Coraza keeps the first 1000 query parameters of a request, and the first
+# 1000 fields of a form data or JSON body, and drops the rest, which no
+# rule then reads, so a request with more adds 5 to the score, as a rule
+# the Core Rule Set rates critical does. Coraza's recommended
+# configuration refuses such a request in its rules 200004 and 200005.
+# This rule runs once the body is read, and before the Core Rule Set adds
+# up the score in the same phase.
+SecArgumentsLimit 1000
+SecRule ARGUMENTS_LIMIT_REACHED "@eq 1" "id:900300,phase:2,pass,\
+    severity:'CRITICAL',setvar:'tx.inbound_anomaly_score_pl1=+5'"
 
 # The sixth: only the rules for requests are loaded, and no response is
 # inspected.
@@ -57,7 +70,9 @@ Include @owasp_crs/REQUEST-*.conf
 
 # The third: redirect_uri is not checked for a URL naming an IP address or
 # localhost. Coraza matches a parameter name here, and in the fourth,
-# without regard to case.
+# without regard to case. ARGS holds the fields of a form data or multipart
+# body Coraza reads as well as the query parameters, so a field of one of
+# these names is left out too.
 SecRuleUpdateTargetById 931100 "!ARGS:redirect_uri"
 SecRuleUpdateTargetById 934110 "!ARGS:redirect_uri"
 
@@ -80,16 +95,30 @@ SecRuleUpdateTargetById 932260 "!ARGS:path|!ARGS:files|!ARGS:skip-to|\
 # Inspect.
 SecRuleUpdateTargetById 932340 "!REQUEST_HEADERS:Referer"
 SecRuleUpdateTargetById 944110 "!REQUEST_HEADERS:Referer"
+`
 
-# Coraza keeps the first 1000 query parameters of a request and drops the
-# rest, which no rule then reads, so a request with more adds 5 to the
-# score, as a rule the Core Rule Set rates critical does. Coraza's
-# recommended configuration refuses such a request in its rule 200004.
-# This rule comes after the Core Rule Set's, which set the score to 0 in
-# the same phase.
-SecArgumentsLimit 1000
-SecRule ARGUMENTS_LIMIT_REACHED "@eq 1" "id:900300,phase:1,pass,\
-    severity:'CRITICAL',setvar:'tx.inbound_anomaly_score_pl1=+5'"
+// bodyDirectives have the Core Rule Set read the part of a request body
+// Inspect gives it, which is at most one byte longer than the limit, up to
+// the limit, %d bytes, and read JSON and XML as Coraza's recommended
+// configuration has it in its rules 200000, 200001 and 200006; form data
+// and multipart Coraza knows by itself. %% stands for a % Coraza reads.
+const bodyDirectives = `
+SecRequestBodyAccess On
+SecRequestBodyLimit %d
+SecRequestBodyLimitAction ProcessPartial
+
+SecRule REQUEST_HEADERS:Content-Type "@rx ^(?:application(?:/soap[+]|/)|text/)xml" \
+    "id:900410,phase:1,pass,nolog,t:none,t:lowercase,ctl:requestBodyProcessor=XML"
+SecRule REQUEST_HEADERS:Content-Type "@rx ^application/(?:[a-z0-9.-]+[+])?json" \
+    "id:900420,phase:1,pass,nolog,t:none,t:lowercase,ctl:requestBodyProcessor=JSON"
+
+# The rest of the second change: Content-Encoding is refused again on a
+# body of a kind the Core Rule Set reads, since a compressed body cannot be
+# inspected.
+SecRule REQBODY_PROCESSOR "@rx ^(?:URLENCODED|MULTIPART|JSON|XML)$" \
+    "id:900260,phase:1,pass,nolog,\
+    setvar:'tx.restricted_headers_basic=%%{tx.restricted_headers_basic} \
+    /content-encoding/'"
 `
 
 // cookiesNotRead are the cookies the Core Rule Set reads a request
@@ -105,18 +134,28 @@ type Params struct {
 	// DisabledRules are the ids of the rules switched off
 	// (SWWAF_WAF_DISABLED_RULES).
 	DisabledRules []int
+	// BodyLimit is the most of a request body the Core Rule Set reads
+	// (SWWAF_WAF_BODY_LIMIT), 0 while it is off and it reads none.
+	BodyLimit int64
 }
 
 // CoreRuleSet is the Core Rule Set, ready to inspect requests. It is safe
 // for concurrent use.
 type CoreRuleSet struct {
-	waf coraza.WAF
+	waf       coraza.WAF
+	bodyLimit int64
 }
 
 // New returns the Core Rule Set with the six changes, at params'
-// paranoia level and without the rules it switches off.
+// paranoia level, without the rules it switches off, and reading request
+// bodies up to params' limit.
 func New(params Params) (*CoreRuleSet, error) {
-	text := fmt.Sprintf(directives, params.ParanoiaLevel)
+	body := ""
+	if params.BodyLimit > 0 {
+		body = fmt.Sprintf(bodyDirectives, params.BodyLimit)
+	}
+
+	text := fmt.Sprintf(directives, params.ParanoiaLevel, body)
 
 	if len(params.DisabledRules) > 0 {
 		ids := make([]string, len(params.DisabledRules))
@@ -134,7 +173,7 @@ func New(params Params) (*CoreRuleSet, error) {
 		return nil, fmt.Errorf("load the Core Rule Set: %w", err)
 	}
 
-	return &CoreRuleSet{waf: waf}, nil
+	return &CoreRuleSet{waf: waf, bodyLimit: params.BodyLimit}, nil
 }
 
 // Result is what the Core Rule Set found in a request.
@@ -148,10 +187,17 @@ type Result struct {
 
 // Inspect runs the Core Rule Set on r, a request from client: on its
 // method, its URL with the query, and its headers, the Cookie header
-// without the cookies in cookiesNotRead.
-func (c *CoreRuleSet) Inspect(r *http.Request, client netip.Addr) Result {
+// without the cookies in cookiesNotRead, and on body, r's body as the
+// caller has it, as readBody reads it. It returns what it found, what it
+// read of body, which the app is still to be sent, and the error that
+// ended the reading early, if one did.
+func (c *CoreRuleSet) Inspect(
+	r *http.Request, client netip.Addr, body io.Reader,
+) (Result, []byte, error) {
 	tx := c.waf.NewTransaction()
-	// With no body read, there is nothing whose closing can fail.
+	// Closing removes the files Coraza writes, in the system's temporary
+	// directory, for the file parts of a multipart body it reads. One it
+	// cannot remove is left there, and changes nothing in what was found.
 	defer func() { _ = tx.Close() }()
 
 	tx.ProcessConnection(client.String(), 0, "", 0)
@@ -178,7 +224,11 @@ func (c *CoreRuleSet) Inspect(r *http.Request, client netip.Addr) Result {
 	}
 
 	tx.ProcessRequestHeaders()
-	// With no body read, this runs the rest of the rules, and cannot fail.
+
+	read, err := c.readBody(tx, body)
+
+	// This reads the body in memory and runs the rest of the rules, and
+	// cannot fail.
 	_, _ = tx.ProcessRequestBody()
 
 	var ids []int
@@ -192,7 +242,44 @@ func (c *CoreRuleSet) Inspect(r *http.Request, client netip.Addr) Result {
 		}
 	}
 
-	return Result{RuleIDs: ids, Score: score(tx)}
+	return Result{RuleIDs: ids, Score: score(tx)}, read, err
+}
+
+// readBody reads body, the body of the request in tx, which has run on
+// the request's headers, while SWWAF_WAF_BODY_LIMIT is set and the body is
+// of a kind the Core Rule Set reads: form data and multipart, of which it
+// reads the first c.bodyLimit bytes, and JSON and XML, which it reads only
+// when they are no longer than that, since they cannot be read in part.
+// readBody reads one byte past the limit, to tell which they are, gives
+// the Core Rule Set what it reads, and returns what it read and the error
+// that ended the reading early, if one did.
+func (c *CoreRuleSet) readBody(tx types.Transaction, body io.Reader) ([]byte, error) {
+	if c.bodyLimit == 0 {
+		return nil, nil
+	}
+
+	inPart := false
+	// How the body is read is a variable of the transaction, which only
+	// Coraza's interface for plugins reads.
+	state := tx.(plugintypes.TransactionState) //nolint:forcetypeassert // every one is
+
+	switch state.Variables().RequestBodyProcessor().Get() {
+	case "URLENCODED", "MULTIPART":
+		inPart = true
+	case "JSON", "XML":
+	default:
+		return nil, nil
+	}
+
+	read, err := io.ReadAll(io.LimitReader(body, c.bodyLimit+1))
+
+	if inPart || (err == nil && int64(len(read)) <= c.bodyLimit) {
+		// Coraza holds what it reads of the body in memory, up to the
+		// limit, so this cannot fail.
+		_, _, _ = tx.WriteRequestBody(read)
+	}
+
+	return read, err
 }
 
 // score returns the anomaly score the Core Rule Set added up in tx, a

internal/waf/waf_test.go

@@ -4,7 +4,9 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
+	"net/url"
 	"reflect"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -48,12 +50,28 @@ func get(target string, headers ...string) request {
 func inspect(t *testing.T, crs *waf.CoreRuleSet, r request) waf.Result {
 	t.Helper()
 
+	result, _ := inspectBody(t, crs, r, "")
+
+	return result
+}
+
+// inspectBody is inspect for r with body, which is announced with its
+// Content-Length unless it is "", and returns what crs read of body too.
+func inspectBody(
+	t *testing.T, crs *waf.CoreRuleSet, r request, body string,
+) (waf.Result, string) {
+	t.Helper()
+
 	req := httptest.NewRequestWithContext(t.Context(), r.method,
-		"http://git.example"+r.target, http.NoBody)
+		"http://git.example"+r.target, strings.NewReader(body))
 	req.Header.Set("User-Agent", "Mozilla/5.0 (X11; Linux x86_64; rv:131.0) "+
 		"Gecko/20100101 Firefox/131.0")
 	req.Header.Set("Accept", "text/html")
 
+	if body != "" {
+		req.Header.Set("Content-Length", strconv.Itoa(len(body)))
+	}
+
 	for _, header := range r.headers {
 		// Go's server keeps Host and Transfer-Encoding out of the headers.
 		name, value, _ := strings.Cut(header, ": ")
@@ -67,7 +85,12 @@ func inspect(t *testing.T, crs *waf.CoreRuleSet, r request) waf.Result {
 		}
 	}
 
-	return crs.Inspect(req, netip.MustParseAddr("203.0.113.9"))
+	result, read, err := crs.Inspect(req, netip.MustParseAddr("203.0.113.9"), req.Body)
+	if err != nil {
+		t.Fatalf("read the body: %v", err)
+	}
+
+	return result, string(read)
 }
 
 // wantResult checks what crs finds in r.
@@ -174,7 +197,7 @@ func TestTransferEncodingIsRead(t *testing.T) {
 		waf.Result{})
 }
 
-func TestMoreQueryParametersThanCorazaKeepsIsAMatch(t *testing.T) {
+func TestMoreParametersThanCorazaKeepsIsAMatch(t *testing.T) {
 	t.Parallel()
 
 	const attack = "id=1'%20OR%20'1'='1"
@@ -185,6 +208,23 @@ func TestMoreQueryParametersThanCorazaKeepsIsAMatch(t *testing.T) {
 	// after it is not, but the request is a match all the same.
 	wantResult(t, crs, get("/?"+strings.Repeat("a=1&", 999)+attack), matched(942100))
 	wantResult(t, crs, get("/?"+strings.Repeat("a=1&", 1000)+attack), matched(900300))
+
+	// So it is with the fields of a form data or JSON body.
+	crs = readingBodies(t)
+
+	for _, tc := range []struct{ header, body string }{
+		{formData, strings.Repeat("a=1&", 999) + attack},
+		{jsonBody, `{"a":[` + strings.Repeat("1,", 998) + `1],"id":"` + injection + `"}`},
+	} {
+		wantBody(t, crs, post(tc.header), tc.body, matched(942100), tc.body)
+	}
+
+	for _, tc := range []struct{ header, body string }{
+		{formData, strings.Repeat("a=1&", 1000) + attack},
+		{jsonBody, `{"a":[` + strings.Repeat("1,", 999) + `1],"id":"` + injection + `"}`},
+	} {
+		wantBody(t, crs, post(tc.header), tc.body, matched(900300), tc.body)
+	}
 }
 
 func TestRedirectURIMayNameALocalAddress(t *testing.T) {
@@ -320,3 +360,221 @@ func TestEachDisabledRuleIsSwitchedOff(t *testing.T) {
 		waf.Result{RuleIDs: []int{911100, 920350}, Score: 8})
 	wantResult(t, newCoreRuleSet(t, 1, 920350, 911100), r, waf.Result{})
 }
+
+// bodyLimit is SWWAF_WAF_BODY_LIMIT in the tests that read bodies.
+const bodyLimit = 8 << 10
+
+// The Content-Type headers of the kinds of body the Core Rule Set reads.
+const (
+	formData  = "Content-Type: application/x-www-form-urlencoded"
+	multipart = "Content-Type: multipart/form-data; boundary=b"
+	jsonBody  = "Content-Type: application/json"
+	xmlBody   = "Content-Type: application/xml"
+)
+
+// injection is an SQL injection, which rule 942100 matches.
+const injection = "1' OR '1'='1"
+
+// readingBodies returns the Core Rule Set as smallwebwaf runs it by
+// default, but reading bodies up to bodyLimit.
+func readingBodies(t *testing.T) *waf.CoreRuleSet {
+	t.Helper()
+
+	crs, err := waf.New(waf.Params{
+		ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyLimit: bodyLimit,
+	})
+	if err != nil {
+		t.Fatalf("load the Core Rule Set: %v", err)
+	}
+
+	return crs
+}
+
+// post is a POST request for / with a body of the type contentType, a
+// Content-Type header, gives, and headers besides.
+func post(contentType string, headers ...string) request {
+	return request{http.MethodPost, "/", append([]string{contentType}, headers...)}
+}
+
+// field is a part of a multipart body: the field name, holding value.
+func field(name, value string) string {
+	return "--b\r\nContent-Disposition: form-data; name=\"" + name + "\"\r\n\r\n" +
+		value + "\r\n"
+}
+
+// end ends a multipart body.
+const end = "--b--\r\n"
+
+// padded returns head and tail with as many a's between them as make n
+// bytes in all.
+func padded(head, tail string, n int) string {
+	return head + strings.Repeat("a", n-len(head)-len(tail)) + tail
+}
+
+// wantBody checks what crs finds in r with body, and that what it read of
+// body is read.
+func wantBody(
+	t *testing.T, crs *waf.CoreRuleSet, r request, body string, want waf.Result,
+	read string,
+) {
+	t.Helper()
+
+	got, gotRead := inspectBody(t, crs, r, body)
+	if !reflect.DeepEqual(got, want) || gotRead != read {
+		t.Errorf("%q with a body of %d bytes, %.40q: %+v, reading %d bytes, "+
+			"want %+v, reading %d", r.headers, len(body), body, got, len(gotRead),
+			want, len(read))
+	}
+}
+
+func TestBodiesAreReadOnlyWhileBodyLimitIsSet(t *testing.T) {
+	t.Parallel()
+
+	off, on := atDefaults(t), readingBodies(t)
+
+	for _, tc := range []struct{ header, body string }{
+		{formData, "q=" + url.QueryEscape(injection)},
+		{multipart, field("q", injection) + end},
+		{jsonBody, `{"q":"` + injection + `"}`},
+		{xmlBody, "<q>" + injection + "</q>"},
+	} {
+		wantBody(t, off, post(tc.header), tc.body, waf.Result{}, "")
+		wantBody(t, on, post(tc.header), tc.body, matched(942100), tc.body)
+	}
+}
+
+func TestFormDataAndMultipartAreReadUpToTheLimit(t *testing.T) {
+	t.Parallel()
+
+	crs := readingBodies(t)
+	pad := strings.Repeat("a", bodyLimit)
+
+	for _, tc := range []struct{ header, attackFirst, attackLast string }{
+		{
+			formData, "q=" + url.QueryEscape(injection) + "&pad=" + pad,
+			"pad=" + pad + "&q=" + url.QueryEscape(injection),
+		},
+		{
+			multipart, field("q", injection) + field("pad", pad) + end,
+			field("pad", pad) + field("q", injection) + end,
+		},
+	} {
+		wantBody(t, crs, post(tc.header), tc.attackFirst, matched(942100),
+			tc.attackFirst[:bodyLimit+1])
+		wantBody(t, crs, post(tc.header), tc.attackLast, waf.Result{},
+			tc.attackLast[:bodyLimit+1])
+	}
+
+	// To the byte: a system file's path is found when it ends at the limit,
+	// and not when its last letter is past it, which is still read.
+	atLimit := padded("pad=", "&q=/etc/passwd", bodyLimit)
+	wantBody(t, crs, post(formData), atLimit, matched(930120, 932160), atLimit)
+
+	pastLimit := padded("pad=", "&q=/etc/passwd", bodyLimit+1)
+	wantBody(t, crs, post(formData), pastLimit, waf.Result{}, pastLimit)
+}
+
+func TestJSONAndXMLAreReadOnlyWhenNoLargerThanTheLimit(t *testing.T) {
+	t.Parallel()
+
+	crs := readingBodies(t)
+
+	for _, tc := range []struct{ header, head, tail string }{
+		{jsonBody, `{"q":"` + injection + `","pad":"`, `"}`},
+		{xmlBody, "<r><q>" + injection + "</q><pad>", "</pad></r>"},
+	} {
+		fits := padded(tc.head, tc.tail, bodyLimit)
+		wantBody(t, crs, post(tc.header), fits, matched(942100), fits)
+
+		larger := padded(tc.head, tc.tail, bodyLimit+1)
+		wantBody(t, crs, post(tc.header), larger, waf.Result{}, larger)
+	}
+}
+
+func TestOtherBodiesAreNotRead(t *testing.T) {
+	t.Parallel()
+
+	crs := readingBodies(t)
+
+	// Read as form data, which the Core Rule Set does with a body of a type
+	// it does not know, this would be an SQL injection.
+	body := "q=" + url.QueryEscape(injection)
+
+	for _, header := range []string{
+		"Content-Type: application/octet-stream",
+		"Content-Type: text/plain",
+		"Content-Type: application/x-git-receive-pack-request",
+	} {
+		wantBody(t, crs, post(header), body, waf.Result{}, "")
+	}
+}
+
+func TestContentEncodingIsRefusedOnTheKindsOfBodyTheCoreRuleSetReads(t *testing.T) {
+	t.Parallel()
+
+	const gzip = "Content-Encoding: gzip"
+
+	crs := readingBodies(t)
+
+	for _, header := range []string{formData, multipart, jsonBody, xmlBody} {
+		wantBody(t, crs, post(header, gzip), "a", matched(920450), "a")
+	}
+
+	// Whatever its size: a JSON body larger than the limit is not read, but
+	// Content-Encoding on it is refused all the same.
+	larger := strings.Repeat("a", bodyLimit+1)
+	wantBody(t, crs, post(jsonBody, gzip), larger, matched(920450), larger)
+
+	// It is allowed on a body of any other kind, and on every body while no
+	// body is read.
+	fetch := "Content-Type: application/x-git-upload-pack-request"
+	wantBody(t, crs, post(fetch, gzip), "a", waf.Result{}, "")
+	wantBody(t, atDefaults(t), post(formData, gzip), "a", waf.Result{}, "")
+}
+
+func TestParametersGiteaSendsNamesInAreLeftOutAmongFormFieldsToo(t *testing.T) {
+	t.Parallel()
+
+	crs := readingBodies(t)
+	local := url.QueryEscape("http://127.0.0.1:52341/")
+
+	for _, tc := range []struct {
+		body string
+		want waf.Result
+	}{
+		{"path=.gitignore", waf.Result{}},
+		{"q=.gitignore", matched(930120)},
+		{"redirect_uri=" + local, waf.Result{}},
+		{"next=" + local, matched(931100, 934110)},
+	} {
+		wantBody(t, crs, post(formData), tc.body, tc.want, tc.body)
+	}
+
+	body := field("path", ".gitignore") + end
+	wantBody(t, crs, post(multipart), body, waf.Result{}, body)
+
+	body = field("q", ".gitignore") + end
+	wantBody(t, crs, post(multipart), body, matched(930120), body)
+
+	// A JSON body's field is named by its path, here json.path, and is
+	// checked.
+	body = `{"path":".gitignore"}`
+	wantBody(t, crs, post(jsonBody), body, matched(930120), body)
+}
+
+func TestGiteaBodiesTheCoreRuleSetRefuses(t *testing.T) {
+	t.Parallel()
+
+	crs := readingBodies(t)
+
+	// A comment that shows a shell command.
+	const text = "Try `curl -s https://example.org | sh` first."
+
+	comment := "content=" + url.QueryEscape(text)
+	wantBody(t, crs, post(formData), comment, matched(932235), comment)
+
+	// An attachment named like a log file.
+	attachment := "--b\r\nContent-Disposition: form-data; name=\"file\"; " +
+		"filename=\"debug.log\"\r\nContent-Type: text/plain\r\n\r\nstarted\r\n" + end
+	wantBody(t, crs, post(multipart), attachment, matched(932180), attachment)
+}