feat: add SecRequestBodyJsonDepthLimit directive

coraza.conf-recommended

@@ -34,6 +34,9 @@ SecRule REQUEST_HEADERS:Content-Type "^application/json" \
 SecRule REQUEST_HEADERS:Content-Type "^application/[a-z0-9.-]+[+]json" \
      "id:'200006',phase:1,t:none,t:lowercase,pass,nolog,ctl:requestBodyProcessor=JSON"
 
+# Configures the maximum JSON recursion depth limit Coraza will accept.
+SecRequestBodyJsonDepthLimit 1024
+
 # Maximum request body size we will accept for buffering. If you support
 # file uploads then the value given on the first line has to be as large
 # as the largest file you are willing to accept. The second value refers

experimental/plugins/plugintypes/bodyprocessor.go

@@ -21,6 +21,8 @@ type BodyProcessorOptions struct {
 	FileMode fs.FileMode
 	// DirMode is the mode of the directory that will be created
 	DirMode fs.FileMode
+	// RequestBodyRecursionLimit is the maximum recursion level accepted in a body processor
+	RequestBodyRecursionLimit int
 }
 
 // BodyProcessor interface is used to create

internal/bodyprocessors/json.go

@@ -4,6 +4,7 @@
 package bodyprocessors
 
 import (
+	"errors"
 	"io"
 	"strconv"
 	"strings"
@@ -13,13 +14,15 @@ import (
 	"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
 )
 
+const ResponseBodyRecursionLimit = -1
+
 type jsonBodyProcessor struct{}
 
 var _ plugintypes.BodyProcessor = &jsonBodyProcessor{}
 
-func (js *jsonBodyProcessor) ProcessRequest(reader io.Reader, v plugintypes.TransactionVariables, _ plugintypes.BodyProcessorOptions) error {
+func (js *jsonBodyProcessor) ProcessRequest(reader io.Reader, v plugintypes.TransactionVariables, bpo plugintypes.BodyProcessorOptions) error {
 	col := v.ArgsPost()
-	data, err := readJSON(reader)
+	data, err := readJSON(reader, bpo.RequestBodyRecursionLimit)
 	if err != nil {
 		return err
 	}
@@ -31,7 +34,7 @@ func (js *jsonBodyProcessor) ProcessRequest(reader io.Reader, v plugintypes.Tran
 
 func (js *jsonBodyProcessor) ProcessResponse(reader io.Reader, v plugintypes.TransactionVariables, _ plugintypes.BodyProcessorOptions) error {
 	col := v.ResponseArgs()
-	data, err := readJSON(reader)
+	data, err := readJSON(reader, ResponseBodyRecursionLimit)
 	if err != nil {
 		return err
 	}
@@ -41,28 +44,38 @@ func (js *jsonBodyProcessor) ProcessResponse(reader io.Reader, v plugintypes.Tra
 	return nil
 }
 
-func readJSON(reader io.Reader) (map[string]string, error) {
+func readJSON(reader io.Reader, maxRecursion int) (map[string]string, error) {
 	s := strings.Builder{}
 	_, err := io.Copy(&s, reader)
 	if err != nil {
 		return nil, err
 	}
 
-	json := gjson.Parse(s.String())
 	res := make(map[string]string)
 	key := []byte("json")
-	readItems(json, key, res)
-	return res, nil
+
+	if !gjson.Valid(s.String()) {
+		return res, errors.New("invalid JSON")
+	}
+	json := gjson.Parse(s.String())
+	err = readItems(json, key, maxRecursion, res)
+	return res, err
 }
 
 // Transform JSON to a map[string]string
+// This function is recursive and will call itself for nested objects.
+// The limit in recursion is defined by maxItems.
 // Example input: {"data": {"name": "John", "age": 30}, "items": [1,2,3]}
 // Example output: map[string]string{"json.data.name": "John", "json.data.age": "30", "json.items.0": "1", "json.items.1": "2", "json.items.2": "3"}
 // Example input: [{"data": {"name": "John", "age": 30}, "items": [1,2,3]}]
 // Example output: map[string]string{"json.0.data.name": "John", "json.0.data.age": "30", "json.0.items.0": "1", "json.0.items.1": "2", "json.0.items.2": "3"}
-// TODO add some anti DOS protection
-func readItems(json gjson.Result, objKey []byte, res map[string]string) {
+func readItems(json gjson.Result, objKey []byte, maxRecursion int, res map[string]string) error {
 	arrayLen := 0
+	var iterationError error
+	if maxRecursion == 0 {
+		// we reached the limit of nesting we want to handle
+		return errors.New("max recursion reached while reading json object")
+	}
 	json.ForEach(func(key, value gjson.Result) bool {
 		// Avoid string concatenation to maintain a single buffer for key aggregation.
 		prevParentLength := len(objKey)
@@ -77,7 +90,11 @@ func readItems(json gjson.Result, objKey []byte, res map[string]string) {
 		var val string
 		switch value.Type {
 		case gjson.JSON:
-			readItems(value, objKey, res)
+			// call recursively with one less item to avoid doing infinite recursion
+			iterationError = readItems(value, objKey, maxRecursion-1, res)
+			if iterationError != nil {
+				return false
+			}
 			objKey = objKey[:prevParentLength]
 			return true
 		case gjson.String:
@@ -97,6 +114,7 @@ func readItems(json gjson.Result, objKey []byte, res map[string]string) {
 	if arrayLen > 0 {
 		res[string(objKey)] = strconv.Itoa(arrayLen)
 	}
+	return iterationError
 }
 
 func init() {

internal/bodyprocessors/json_test.go

@@ -4,14 +4,21 @@
 package bodyprocessors
 
 import (
+	"errors"
 	"strings"
 	"testing"
 )
 
+const (
+	deeplyNestedJSONObject = 15000
+	maxRecursion           = 10000
+)
+
 var jsonTests = []struct {
 	name string
 	json string
 	want map[string]string
+	err  error
 }{
 	{
 		name: "map",
@@ -56,6 +63,7 @@
 			"json.f.0.0":     "1",
 			"json.f.0.0.0.z": "abc",
 		},
+		err: nil,
 	},
 	{
 		name: "array",
@@ -116,18 +124,49 @@
 			"json.1.f.0.0":     "1",
 			"json.1.f.0.0.0.z": "abc",
 		},
+		err: nil,
+	},
+	{
+		name: "broken1", # this json test has more opening brackets than closing, so it is broken
+		json: `{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a": 1 }}}}}}}}}}}}}}}}}}}}}}`,
+		want: map[string]string{},
+		err:  errors.New("invalid JSON"),
+	},
+	{
+		name: "broken2",
+		json: `{"test": 123, "test2": 456, "test3": [22, 44, 55], "test4": 3}`,
+		want: map[string]string{
+			"json.test3.0": "22",
+			"json.test3.1": "44",
+			"json.test3.2": "55",
+			"json.test4":   "3",
+			"json.test":    "123",
+			"json.test2":   "456",
+			"json.test3":   "3",
+		},
+		err: nil,
+	},
+	{
+		name: "bomb",
+		json: strings.Repeat(`{"a":`, deeplyNestedJSONObject) + "1" + strings.Repeat(`}`, deeplyNestedJSONObject),
+		want: map[string]string{
+			"json." + strings.Repeat(`a.`, deeplyNestedJSONObject-1) + "a": "1",
+		},
+		err: errors.New("max recursion reached while reading json object"),
 	},
 }
 
 func TestReadJSON(t *testing.T) {
 	for _, tc := range jsonTests {
 		tt := tc
 		t.Run(tt.name, func(t *testing.T) {
-			jsonMap, err := readJSON(strings.NewReader(tt.json))
-			if err != nil {
-				t.Error(err)
-			}
+			jsonMap, err := readJSON(strings.NewReader(tt.json), maxRecursion)
 			for k, want := range tt.want {
+				if err != nil && err.Error() == tt.err.Error() {
+					continue
+				} else if err != nil {
+					t.Error(err)
+				}
 				if have, ok := jsonMap[k]; ok {
 					if want != have {
 						t.Errorf("key=%s, want %s, have %s", k, want, have)
@@ -150,7 +189,7 @@
 		tt := tc
 		b.Run(tt.name, func(b *testing.B) {
 			for i := 0; i < b.N; i++ {
-				_, err := readJSON(strings.NewReader(tt.json))
+				_, err := readJSON(strings.NewReader(tt.json), maxRecursion)
 				if err != nil {
 					b.Error(err)
 				}

internal/corazawaf/transaction.go

@@ -70,6 +70,7 @@ type Transaction struct {
 	ForceRequestBodyVariable  bool
 	RequestBodyAccess         bool
 	RequestBodyLimit          int64
+	RequestBodyJsonDepthLimit int
 	ForceResponseBodyVariable bool
 	ResponseBodyAccess        bool
 	ResponseBodyLimit         int64
@@ -78,7 +79,7 @@ type Transaction struct {
 	HashEnforcement           bool
 
 	// Stores the last phase that was evaluated
-	// Used by allow to skip phases
+	// Used by allow to skip phasesx
 	lastPhase types.RulePhase
 
 	// Handles request body buffers
@@ -996,9 +997,9 @@ func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) {
 		tx.WAF.Rules.Eval(types.PhaseRequestBody, tx)
 		return tx.interruption, nil
 	}
-	mime := ""
+	mimeType := ""
 	if m := tx.variables.requestHeaders.Get("content-type"); len(m) > 0 {
-		mime = m[0]
+		mimeType = m[0]
 	}
 
 	reader, err := tx.requestBodyBuffer.Reader()
@@ -1011,7 +1012,7 @@ func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) {
 	// Default variables.ReqbodyProcessor values
 	// XML and JSON must be forced with ctl:requestBodyProcessor=JSON
 	if tx.ForceRequestBodyVariable {
-		// We force URLENCODED if mime is x-www... or we have an empty RBP and ForceRequestBodyVariable
+		// We force URLENCODED if mimeType is x-www... or we have an empty RBP and ForceRequestBodyVariable
 		if rbp == "" {
 			rbp = "URLENCODED"
 		}
@@ -1035,8 +1036,9 @@ func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) {
 		Msg("Attempting to process request body")
 
 	if err := bodyprocessor.ProcessRequest(reader, tx.Variables(), plugintypes.BodyProcessorOptions{
-		Mime:        mime,
-		StoragePath: tx.WAF.UploadDir,
+		Mime:                      mimeType,
+		StoragePath:               tx.WAF.UploadDir,
+		RequestBodyRecursionLimit: tx.WAF.RequestBodyJsonDepthLimit,
 	}); err != nil {
 		tx.debugLogger.Error().Err(err).Msg("Failed to process request body")
 		tx.generateRequestBodyError(err)
@@ -1262,7 +1264,6 @@ func (tx *Transaction) ProcessResponseBody() (*types.Interruption, error) {
 		}
 
 		tx.debugLogger.Debug().Str("body_processor", bp).Msg("Attempting to process response body")
-
 		if err := b.ProcessResponse(reader, tx.Variables(), plugintypes.BodyProcessorOptions{}); err != nil {
 			tx.debugLogger.Error().Err(err).Msg("Failed to process response body")
 			tx.generateResponseBodyError(err)

internal/corazawaf/transaction_test.go

@@ -1665,7 +1665,7 @@ func TestResponseBodyForceProcessing(t *testing.T) {
 	if _, err := tx.ProcessRequestBody(); err != nil {
 		t.Fatal(err)
 	}
-	tx.ProcessResponseHeaders(200, "HTTP/1")
+	tx.ProcessResponseHeaders(200, "HTTP/1.1")
 	if _, _, err := tx.WriteResponseBody([]byte(`{"key":"value"}`)); err != nil {
 		t.Fatal(err)
 	}

internal/corazawaf/waf.go

@@ -23,6 +23,12 @@ import (
 	"github.com/corazawaf/coraza/v3/types"
 )
 
+// Default settings
+const (
+	// DefaultRequestBodyJsonDepthLimit is the default limit for the depth of JSON objects in the request body
+	DefaultRequestBodyJsonDepthLimit = 1024
+)
+
 // WAF instance is used to store configurations and rules
 // Every web application should have a different WAF instance,
 // but you can share an instance if you are ok with sharing
@@ -44,6 +50,9 @@ type WAF struct {
 	// Request body page file limit
 	RequestBodyLimit int64
 
+	// Request body JSON recursive depth limit
+	RequestBodyJsonDepthLimit int
+
 	// Request body in memory limit
 	requestBodyInMemoryLimit *int64
 
@@ -176,9 +185,10 @@ func (w *WAF) newTransaction(opts Options) *Transaction {
 	tx.AuditLogParts = w.AuditLogParts
 	tx.ForceRequestBodyVariable = false
 	tx.RequestBodyAccess = w.RequestBodyAccess
-	tx.RequestBodyLimit = int64(w.RequestBodyLimit)
+	tx.RequestBodyLimit = w.RequestBodyLimit
+	tx.RequestBodyJsonDepthLimit = w.RequestBodyJsonDepthLimit
 	tx.ResponseBodyAccess = w.ResponseBodyAccess
-	tx.ResponseBodyLimit = int64(w.ResponseBodyLimit)
+	tx.ResponseBodyLimit = w.ResponseBodyLimit
 	tx.RuleEngine = w.RuleEngine
 	tx.HashEngine = false
 	tx.HashEnforcement = false
@@ -194,13 +204,13 @@ func (w *WAF) newTransaction(opts Options) *Transaction {
 	tx.Timestamp = time.Now().UnixNano()
 	tx.audit = false
 
-	// Always non-nil if buffers / collections were already initialized so we don't do any of them
+	// Always non-nil if buffers / collections were already initialized, so we don't do any of them
 	// based on the presence of RequestBodyBuffer.
 	if tx.requestBodyBuffer == nil {
 		// if no requestBodyInMemoryLimit has been set we default to the requestBodyLimit
-		var requestBodyInMemoryLimit int64 = w.RequestBodyLimit
+		var requestBodyInMemoryLimit = w.RequestBodyLimit
 		if w.requestBodyInMemoryLimit != nil {
-			requestBodyInMemoryLimit = int64(*w.requestBodyInMemoryLimit)
+			requestBodyInMemoryLimit = *w.requestBodyInMemoryLimit
 		}
 
 		tx.requestBodyBuffer = NewBodyBuffer(types.BodyBufferOptions{
@@ -294,6 +304,7 @@ func NewWAF() *WAF {
 		RequestBodyAccess:         false,
 		RequestBodyLimit:          134217728, // Hard limit equal to _1gb
 		RequestBodyLimitAction:    types.BodyLimitActionReject,
+		RequestBodyJsonDepthLimit: DefaultRequestBodyJsonDepthLimit,
 		ResponseBodyAccess:        false,
 		ResponseBodyLimit:         524288, // Hard limit equal to _1gb
 		auditLogWriter:            logWriter,
@@ -412,5 +423,9 @@ func (w *WAF) Validate() error {
 		return errors.New("argument limit should be bigger than 0")
 	}
 
+	if w.RequestBodyJsonDepthLimit <= 0 {
+		return errors.New("request body json depth limit should be bigger than 0")
+	}
+
 	return nil
 }

internal/seclang/directives.go

@@ -271,6 +271,24 @@ func directiveSecRequestBodyAccess(options *DirectiveOptions) error {
 	return nil
 }
 
+// Description: Configures the maximum JSON recursion depth limit Coraza will accept.
+// Default: 1024
+// Syntax: SecRequestBodyJsonDepthLimit [LIMIT]
+// ---
+// Anything over the limit will generate a REQBODY_ERROR in the JSON body processor.
+func directiveSecRequestBodyJsonDepthLimit(options *DirectiveOptions) error {
+	if len(options.Opts) == 0 {
+		return errEmptyOptions
+	}
+
+	limit, err := strconv.Atoi(options.Opts)
+	if err != nil {
+		return err
+	}
+	options.WAF.RequestBodyJsonDepthLimit = limit
+	return nil
+}
+
 // Description: Configures the rules engine.
 // Syntax: SecRuleEngine On|Off|DetectionOnly
 // Default: Off