From dfbde557acc41d858dbe04d4b6eaec64478347ff Mon Sep 17 00:00:00 2001
From: Ervin Hegedus <airween@gmail.com>
Date: Wed, 30 Jul 2025 10:55:33 +0200
Subject: [PATCH] Fix invalid request handling

---
 apache2/apache2_io.c    | 48 +++++++++++++++++-----------------
 apache2/mod_security2.c | 57 ++++++-----------------------------------
 2 files changed, 32 insertions(+), 73 deletions(-)

Index: modsecurity-2.8.0/apache2/apache2_io.c
===================================================================
--- modsecurity-2.8.0.orig/apache2/apache2_io.c
+++ modsecurity-2.8.0/apache2/apache2_io.c
@@ -168,21 +168,21 @@ apr_status_t read_request_body(modsec_re
     apr_bucket_brigade *bb_in;
     apr_bucket *bucket;
 
-    if (error_msg == NULL) return -1;
+    if (error_msg == NULL) return HTTP_INTERNAL_SERVER_ERROR;
     *error_msg = NULL;
 
     if (msr->reqbody_should_exist != 1) {
         if (msr->txcfg->debuglog_level >= 4) {
             msr_log(msr, 4, "Input filter: This request does not have a body.");
         }
-        return 0;
+        return APR_SUCCESS;
     }
 
     if (msr->txcfg->reqbody_access != 1) {
         if (msr->txcfg->debuglog_level >= 4) {
             msr_log(msr, 4, "Input filter: Request body access not enabled.");
         }
-        return 0;
+        return APR_SUCCESS;
     }
 
     if (msr->txcfg->debuglog_level >= 4) {
@@ -190,12 +190,12 @@ apr_status_t read_request_body(modsec_re
     }
 
     if (modsecurity_request_body_start(msr, error_msg) < 0) {
-        return -1;
+        return HTTP_INTERNAL_SERVER_ERROR;
     }
 
     seen_eos = 0;
     bb_in = apr_brigade_create(msr->mp, r->connection->bucket_alloc);
-    if (bb_in == NULL) return -1;
+    if (bb_in == NULL) return HTTP_INTERNAL_SERVER_ERROR;
     do {
         apr_status_t rc;
 
@@ -205,22 +205,17 @@ apr_status_t read_request_body(modsec_re
              *      too large and APR_EGENERAL when the client disconnects.
              */
             switch(rc) {
-                case APR_EOF :
-                    *error_msg = apr_psprintf(msr->mp, "Error reading request body: %s", get_apr_error(msr->mp, rc));
-                    return -6;
-                case APR_TIMEUP :
-                    *error_msg = apr_psprintf(msr->mp, "Error reading request body: %s", get_apr_error(msr->mp, rc));
-                    return -4;
                 case AP_FILTER_ERROR :
                     *error_msg = apr_psprintf(msr->mp, "Error reading request body: HTTP Error 413 - Request entity too large. (Most likely.)");
-                    return -3;
+                    break;
                 case APR_EGENERAL :
                     *error_msg = apr_psprintf(msr->mp, "Error reading request body: Client went away.");
-                    return -2;
+                    break;
                 default :
                     *error_msg = apr_psprintf(msr->mp, "Error reading request body: %s", get_apr_error(msr->mp, rc));
-                    return -1;
+                    break;
             }
+            return ap_map_http_request_error(rc, HTTP_BAD_REQUEST);
         }
 
         /* Loop through the buckets in the brigade in order
@@ -236,7 +231,7 @@ apr_status_t read_request_body(modsec_re
             rc = apr_bucket_read(bucket, &buf, &buflen, APR_BLOCK_READ);
             if (rc != APR_SUCCESS) {
                 *error_msg = apr_psprintf(msr->mp, "Failed reading input / bucket (%d): %s", rc, get_apr_error(msr->mp, rc));
-                return -1;
+                return HTTP_INTERNAL_SERVER_ERROR;
             }
 
             if (msr->txcfg->debuglog_level >= 9) {
@@ -249,7 +244,7 @@ apr_status_t read_request_body(modsec_re
                 if((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT)) {
                     *error_msg = apr_psprintf(msr->mp, "Request body is larger than the "
                             "configured limit (%ld).", msr->txcfg->reqbody_limit);
-                    return -5;
+                    return HTTP_REQUEST_ENTITY_TOO_LARGE;
                 } else if((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_PARTIAL)) {
 
                     *error_msg = apr_psprintf(msr->mp, "Request body is larger than the "
@@ -270,13 +265,15 @@ apr_status_t read_request_body(modsec_re
                     *error_msg = apr_psprintf(msr->mp, "Request body is larger than the "
                             "configured limit (%ld).", msr->txcfg->reqbody_limit);
 
-                    return -5;
+                    return HTTP_REQUEST_ENTITY_TOO_LARGE;
                 }
             }
 
             if (msr->txcfg->stream_inbody_inspection == 1)   {
                 msr->stream_input_length+=buflen;
-                modsecurity_request_body_to_stream(msr, buf, buflen, error_msg);
+                if (modsecurity_request_body_to_stream(msr, buf, buflen, error_msg) < 0) {
+                  return HTTP_INTERNAL_SERVER_ERROR;
+                }
             }
 
             msr->reqbody_length += buflen;
@@ -288,7 +285,7 @@ apr_status_t read_request_body(modsec_re
                         if((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT)) {
                             *error_msg = apr_psprintf(msr->mp, "Request body no files data length is larger than the "
                                     "configured limit (%ld).", msr->txcfg->reqbody_no_files_limit);
-                            return -5;
+                            return HTTP_REQUEST_ENTITY_TOO_LARGE;
                         } else if ((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_PARTIAL)) {
                             *error_msg = apr_psprintf(msr->mp, "Request body no files data length is larger than the "
                                     "configured limit (%ld).", msr->txcfg->reqbody_no_files_limit);
@@ -298,12 +295,12 @@ apr_status_t read_request_body(modsec_re
                         } else {
                             *error_msg = apr_psprintf(msr->mp, "Request body no files data length is larger than the "
                                     "configured limit (%ld).", msr->txcfg->reqbody_no_files_limit);
-                            return -5;
+                            return HTTP_REQUEST_ENTITY_TOO_LARGE;
                         }
                     }
 
                     if((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT))
-                        return -1;
+                        return HTTP_INTERNAL_SERVER_ERROR;
                 }
 
             }
@@ -316,8 +313,7 @@ apr_status_t read_request_body(modsec_re
         apr_brigade_cleanup(bb_in);
     } while(!seen_eos);
 
-    // TODO: Why ignore the return code here?
-    modsecurity_request_body_end(msr, error_msg);
+    apr_status_t rcbe = modsecurity_request_body_end(msr, error_msg);
 
     if (msr->txcfg->debuglog_level >= 4) {
         msr_log(msr, 4, "Input filter: Completed receiving request body (length %" APR_SIZE_T_FMT ").",
@@ -326,7 +322,13 @@ apr_status_t read_request_body(modsec_re
 
     msr->if_status = IF_STATUS_WANTS_TO_RUN;
 
-    return 1;
+    if (rcbe == -5) {
+        return HTTP_REQUEST_ENTITY_TOO_LARGE;
+    }
+    if (rcbe < 0) {
+        return HTTP_INTERNAL_SERVER_ERROR;
+    }
+    return APR_SUCCESS;
 }
 
 
Index: modsecurity-2.8.0/apache2/mod_security2.c
===================================================================
--- modsecurity-2.8.0.orig/apache2/mod_security2.c
+++ modsecurity-2.8.0/apache2/mod_security2.c
@@ -938,50 +938,17 @@ static int hook_request_late(request_rec
     }
 
     rc = read_request_body(msr, &my_error_msg);
-    if (rc < 0) {
-        switch(rc) {
-            case -1 :
-                if (my_error_msg != NULL) {
-                    msr_log(msr, 1, "%s", my_error_msg);
-                }
-                return HTTP_INTERNAL_SERVER_ERROR;
-                break;
-            case -4 : /* Timeout. */
-                if (my_error_msg != NULL) {
-                    msr_log(msr, 4, "%s", my_error_msg);
-                }
-                r->connection->keepalive = AP_CONN_CLOSE;
-                return HTTP_REQUEST_TIME_OUT;
-                break;
-            case -5 : /* Request body limit reached. */
-                msr->inbound_error = 1;
-                if((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT))    {
-                    r->connection->keepalive = AP_CONN_CLOSE;
-                    if (my_error_msg != NULL) {
-                        msr_log(msr, 1, "%s. Deny with code (%d)", my_error_msg, HTTP_REQUEST_ENTITY_TOO_LARGE);
-                    }
-                    return HTTP_REQUEST_ENTITY_TOO_LARGE;
-                } else  {
-                    if (my_error_msg != NULL) {
-                        msr_log(msr, 1, "%s", my_error_msg);
-                    }
-                }
-                break;
-            case -6 : /* EOF when reading request body. */
-                if (my_error_msg != NULL) {
-                    msr_log(msr, 4, "%s", my_error_msg);
-                }
-                r->connection->keepalive = AP_CONN_CLOSE;
-                return HTTP_BAD_REQUEST;
-                break;
-            default :
-                /* allow through */
-                break;
-        }
+    if (rc != OK) {
+        if (my_error_msg != NULL) {
+            msr_log(msr, 1, "%s", my_error_msg);
+         }
 
-        msr->msc_reqbody_error = 1;
-        msr->msc_reqbody_error_msg = my_error_msg;
-    }
+        if (rc == HTTP_REQUEST_ENTITY_TOO_LARGE) {
+            msr->inbound_error = 1;
+        }
+        r->connection->keepalive = AP_CONN_CLOSE;
+        return rc;
+     }
 
     /* Update the request headers. They might have changed after
      * the body was read (trailers).
