Repository: corazawaf/coraza (go)

Issue:
Wrong Status Code returned when SecResponseBodyLimit is reached (Action Reject)
When `SecResponseBodyLimit` is reached and `SecResponseBodyLimitAction` is set to "Reject" the status code returned by coraza is incorrect. 

Expected Status Code would be HTTP 500
Received Status Code is HTTP 413

It can be easily reproduced by setting the two directives and use httpbin /bytes backend to receive more bytes than configured... 


The related code part is here:
https://github.com/corazawaf/coraza/blob/30a06bac28c64a3cb0894e19cf40a0236faaeba5/internal/corazawaf/transaction.go#L842-L849

The proposal is to make the `setAndReturnBodyLimitInterruption`  function more flexible to be able to receive a status code which is set in the interruption. 

Like this we can return an HTTP 413 status here (request): 
https://github.com/corazawaf/coraza/blob/30a06bac28c64a3cb0894e19cf40a0236faaeba5/internal/corazawaf/transaction.go#L890

and an HTTP 500 status here (response):
https://github.com/corazawaf/coraza/blob/30a06bac28c64a3cb0894e19cf40a0236faaeba5/internal/corazawaf/transaction.go#L1156


Additionally to the code change, the proposal is to mention Sec[Request/Response]LimitAction in the documentation so it is clear that the resulting behavior of the limit enforcement depends on these directives.. 



Pull request:
fix: wrong status returned when SecResponseBodyLimit is reached and Action is Reject
Closes #1377 

- Improved documentation
- Fixed Status code in response case, now 500 is returned

Patch:
diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go
index a851a68..8884321 100644
--- a/internal/corazawaf/transaction.go
+++ b/internal/corazawaf/transaction.go
@@ -839,10 +839,10 @@ func (tx *Transaction) ProcessRequestHeaders() *types.Interruption {
 	return tx.interruption
 }
 
-func setAndReturnBodyLimitInterruption(tx *Transaction) (*types.Interruption, int, error) {
+func setAndReturnBodyLimitInterruption(tx *Transaction, status int) (*types.Interruption, int, error) {
 	tx.debugLogger.Warn().Msg("Disrupting transaction with body size above the configured limit (Action Reject)")
 	tx.interruption = &types.Interruption{
-		Status: 413,
+		Status: status,
 		Action: "deny",
 	}
 	return tx.interruption, 0, nil
@@ -887,7 +887,7 @@ func (tx *Transaction) WriteRequestBody(b []byte) (*types.Interruption, int, err
 		tx.variables.inboundDataError.Set("1")
 		if tx.WAF.RequestBodyLimitAction == types.BodyLimitActionReject {
 			// We interrupt this transaction in case RequestBodyLimitAction is Reject
-			return setAndReturnBodyLimitInterruption(tx)
+			return setAndReturnBodyLimitInterruption(tx, 413)
 		}
 
 		if tx.WAF.RequestBodyLimitAction == types.BodyLimitActionProcessPartial {
@@ -952,7 +952,7 @@ func (tx *Transaction) ReadRequestBodyFrom(r io.Reader) (*types.Interruption, in
 		if tx.requestBodyBuffer.length+writingBytes >= tx.RequestBodyLimit {
 			tx.variables.inboundDataError.Set("1")
 			if tx.WAF.RequestBodyLimitAction == types.BodyLimitActionReject {
-				return setAndReturnBodyLimitInterruption(tx)
+				return setAndReturnBodyLimitInterruption(tx, 413)
 			}
 
 			if tx.WAF.RequestBodyLimitAction == types.BodyLimitActionProcessPartial {
@@ -972,7 +972,7 @@ func (tx *Transaction) ReadRequestBodyFrom(r io.Reader) (*types.Interruption, in
 	if tx.requestBodyBuffer.length == tx.RequestBodyLimit {
 		tx.variables.inboundDataError.Set("1")
 		if tx.WAF.RequestBodyLimitAction == types.BodyLimitActionReject {
-			return setAndReturnBodyLimitInterruption(tx)
+			return setAndReturnBodyLimitInterruption(tx, 413)
 		}
 
 		if tx.WAF.RequestBodyLimitAction == types.BodyLimitActionProcessPartial {
@@ -1153,7 +1153,7 @@ func (tx *Transaction) WriteResponseBody(b []byte) (*types.Interruption, int, er
 		tx.variables.outboundDataError.Set("1")
 		if tx.WAF.ResponseBodyLimitAction == types.BodyLimitActionReject {
 			// We interrupt this transaction in case ResponseBodyLimitAction is Reject
-			return setAndReturnBodyLimitInterruption(tx)
+			return setAndReturnBodyLimitInterruption(tx, 500)
 		}
 
 		if tx.WAF.ResponseBodyLimitAction == types.BodyLimitActionProcessPartial {
@@ -1203,7 +1203,7 @@ func (tx *Transaction) ReadResponseBodyFrom(r io.Reader) (*types.Interruption, i
 		if tx.responseBodyBuffer.length+writingBytes >= tx.ResponseBodyLimit {
 			tx.variables.outboundDataError.Set("1")
 			if tx.WAF.ResponseBodyLimitAction == types.BodyLimitActionReject {
-				return setAndReturnBodyLimitInterruption(tx)
+				return setAndReturnBodyLimitInterruption(tx, 500)
 			}
 
 			if tx.WAF.ResponseBodyLimitAction == types.BodyLimitActionProcessPartial {
@@ -1223,7 +1223,7 @@ func (tx *Transaction) ReadResponseBodyFrom(r io.Reader) (*types.Interruption, i
 	if tx.responseBodyBuffer.length == tx.ResponseBodyLimit {
 		tx.variables.outboundDataError.Set("1")
 		if tx.WAF.ResponseBodyLimitAction == types.BodyLimitActionReject {
-			return setAndReturnBodyLimitInterruption(tx)
+			return setAndReturnBodyLimitInterruption(tx, 500)
 		}
 
 		if tx.WAF.ResponseBodyLimitAction == types.BodyLimitActionProcessPartial {
diff --git a/internal/seclang/directives.go b/internal/seclang/directives.go
index 3b38bf6..c58b06e 100644
--- a/internal/seclang/directives.go
+++ b/internal/seclang/directives.go
@@ -234,7 +234,9 @@ func directiveSecResponseBodyAccess(options *DirectiveOptions) error {
 // Default: 134217728 (128 Mib)
 // Syntax: SecRequestBodyLimit [LIMIT_IN_BYTES]
 // ---
-// Anything over the limit will be rejected with status code 413 (Request Entity Too Large).
+// Depends on `SecRequestBodyLimitAction`
+// - Reject: Anything over this limit will be rejected with status code 413 (Request Entity Too Large).
+// - ProcessPartial: The first N bytes of the request body will be processed.
 // There is a hard limit of 1 GB.
 func directiveSecRequestBodyLimit(options *DirectiveOptions) error {
 	if len(options.Opts) == 0 {
@@ -430,7 +432,9 @@ func directiveSecResponseBodyLimitAction(options *DirectiveOptions) error {
 // Syntax: SecResponseBodyLimit [LIMIT_IN_BYTES]
 // Default: 524288 (512 Kib)
 // ---
-// Anything over this limit will be rejected with status code 500 (Internal Server Error).
+// Depends on `SecResponseBodyLimitAction`
+// - Reject: Anything over this limit will be rejected with status code 500 (Internal Server Error).
+// - ProcessPartial: The first N bytes of the response body will be processed.
 // This setting will not affect the responses with MIME types that are not selected for
 // buffering. There is a hard limit of 1 GB.
 func directiveSecResponseBodyLimit(options *DirectiveOptions) error {


Test patch:
diff --git a/examples/http-server/main_test.go b/examples/http-server/main_test.go
index ecbee00..e3f0eb8 100644
--- a/examples/http-server/main_test.go
+++ b/examples/http-server/main_test.go
@@ -79,7 +79,7 @@ func TestHttpServer(t *testing.T) {
 		{
 			"positive for response body limit reject",
 			"/",
-			413,
+			500,
 			map[string]string{
 				"DIRECTIVES_FILE": "./testdata/response-body-limits-reject.conf",
 				"RESPONSE_BODY":   "response body beyond the limit",