Correctly return 413 when an invalid chunk size is given on input.

- If get_chunk_size() returns a negative number, that probably implies
  an overflow.  So, create a 413 error and pass it to the output filters.
- Modify ap_discard_request_body() to return OK quickly if we're a subreq
  or our status code implies that we will be dropping the connection.
- Modify ap_die() so that if the new status implies that we will drop
  the connection, that we correctly indicate that we can not keepalive
  this connection.  (Without this, the error is returned, but the connection
  is not closed.)

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@95331 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 5541b06676bfa30f2bc4643fc3ed2cf71a52798d..86efd0c86abace0440644161e6320e3656427907 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,10 @@
 Changes with Apache 2.0.37
 
+  *) Correctly return 413 when an invalid chunk size is given on
+     input.  Also modify ap_discard_request_body to not do anything
+     on sub-requests or when the connection will be dropped.
+     [Justin Erenkrantz]
+
   *) Fix the TIME_* SSL var lookups to be threadsafe.  PR 9469.
      [Cliff Woolley]
 

diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c
index a41dca5c76547a29c9120e40b86fc4670a221cf3..c5301f084128afa3baf0a41f5a7e73a461418b0e 100644 (file)
--- a/modules/http/http_protocol.c
+++ b/modules/http/http_protocol.c
@@ -852,6 +852,17 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
             apr_brigade_flatten(bb, line, &len);
 
             ctx->remaining = get_chunk_size(line);
+            /* Detect invalid chunk sizes. */
+            if (ctx->remaining < 0) {
+                apr_brigade_cleanup(bb);
+                e = ap_bucket_error_create(HTTP_REQUEST_ENTITY_TOO_LARGE, NULL,
+                                           f->r->connection->pool,
+                                           f->r->connection->bucket_alloc);
+                APR_BRIGADE_INSERT_TAIL(bb, e);
+                e = apr_bucket_eos_create(f->r->connection->bucket_alloc);
+                APR_BRIGADE_INSERT_TAIL(bb, e);
+                return ap_pass_brigade(f->r->output_filters, bb);
+            }
         } 
     }
 
@@ -890,6 +901,17 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
                 apr_brigade_flatten(bb, line, &len);
                 ctx->remaining = get_chunk_size(line);
 
+                /* Detect invalid chunk sizes. */
+                if (ctx->remaining < 0) {
+                    apr_brigade_cleanup(bb);
+                    e = ap_bucket_error_create(HTTP_REQUEST_ENTITY_TOO_LARGE,
+                                               NULL, c->pool, c->bucket_alloc);
+                    APR_BRIGADE_INSERT_TAIL(bb, e);
+                    e = apr_bucket_eos_create(c->bucket_alloc);
+                    APR_BRIGADE_INSERT_TAIL(bb, e);
+                    return ap_pass_brigade(f->r->output_filters, bb);
+                }
+
                 if (!ctx->remaining) {
                     /* Handle trailers by calling ap_get_mime_headers again! */
                     ctx->state = BODY_NONE;
@@ -1777,6 +1799,17 @@ AP_DECLARE(int) ap_discard_request_body(request_rec *r)
 {
     int rv;
 
+    /* Sometimes we'll get in a state where the input handling has
+     * detected an error where we want to drop the connection, so if
+     * that's the case, don't read the data as that is what we're trying
+     * to avoid.
+     *
+     * This function is also a no-op on a subrequest.
+     */
+    if (r->main || ap_status_drops_connection(r->status)) {
+        return OK;
+    }
+
     if (r->read_length == 0) {  /* if not read already */
         if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK))) {
             return rv;

diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index d848916f007560be78b40da263961006476e3db0..22c7d7861a75b6d20eb5418a6743a43bf3f2c363 100644 (file)
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -139,13 +139,16 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
         r->status = HTTP_PROXY_AUTHENTICATION_REQUIRED;
     }
 
-    /*
-     * If we want to keep the connection, be sure that the request body
-     * (if any) has been read.
+    /* If we don't want to keep the connection, make sure we mark that the
+     * connection is not eligible for keepalive.  If we want to keep the
+     * connection, be sure that the request body (if any) has been read.
      */
-    if ((r->status != HTTP_NOT_MODIFIED) && (r->status != HTTP_NO_CONTENT)
-        && !ap_status_drops_connection(r->status)
-        && r->connection && (r->connection->keepalive != -1)) {
+    if (ap_status_drops_connection(r->status)) {
+        r->connection->keepalive = 0;
+    }
+    else if ((r->status != HTTP_NOT_MODIFIED) &&
+             (r->status != HTTP_NO_CONTENT) &&
+             r->connection && (r->connection->keepalive != -1)) {
 
         (void) ap_discard_request_body(r);
     }