]> git.ipfire.org Git - thirdparty/apache/httpd.git/commitdiff
*) strict content-length parsing
authorGraham Leggett <minfrin@apache.org>
Wed, 8 Jul 2020 11:09:13 +0000 (11:09 +0000)
committerGraham Leggett <minfrin@apache.org>
Wed, 8 Jul 2020 11:09:13 +0000 (11:09 +0000)
     trunk patch http://svn.apache.org/r1877954
                 http://svn.apache.org/r1877955
                 http://svn.apache.org/r1879369
                 http://svn.apache.org/r1879373
     2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-ap_parse_strict_length.patch
    +1: ylavic, minfrin, jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1879639 13f79535-47bb-0310-9956-ffa450edef68

19 files changed:
CHANGES
STATUS
include/ap_mmn.h
include/httpd.h
modules/cache/mod_cache.c
modules/cache/mod_cache_disk.c
modules/cache/mod_cache_socache.c
modules/dav/main/mod_dav.c
modules/filters/mod_data.c
modules/filters/mod_reflector.c
modules/filters/mod_request.c
modules/http/byterange_filter.c
modules/http/http_filters.c
modules/mappers/mod_negotiation.c
modules/proxy/mod_proxy.c
modules/proxy/mod_proxy_ajp.c
modules/proxy/mod_proxy_http.c
server/protocol.c
server/util.c

diff --git a/CHANGES b/CHANGES
index f8ffce7b6edf7e78625e0c02311796e8c0e2e6e6..71ba9b1055afd2ef2a817d2219e767adf8f78ad6 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.44
 
+  *) Add support for strict content-length parsing through addition of
+     ap_parse_strict_length() [Yann Ylavic]
+
   *) mod_proxy_fcgi: ProxyFCGISetEnvIf unsets variables when expression
      evaluates to false.  PR64365. [Michael König <mail ikoenig.net>]
 
diff --git a/STATUS b/STATUS
index fc92ed982cffb185a9631002583c079d973fcb8e..a834eb789db827890a1678cff534c51ae16d5b4a 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -135,15 +135,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) strict content-length parsing
-     trunk patch http://svn.apache.org/r1877954
-                 http://svn.apache.org/r1877955
-                 http://svn.apache.org/r1879369
-                 http://svn.apache.org/r1879373
-     2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-ap_parse_strict_length.patch
-    +1: ylavic, minfrin, jim
-    ylavic: the patch drops changes to apreq, which seems to not be in 2.4.
-
   *) core: Drop an invalid Last-Modified header value coming
      from a (F)CGI script instead of replacing it with Unix epoch.
      Warn the users about Last-Modified header value replacements
index 0af9c9874f58f9908349104c0d8eca9564bc10c8..805dc0c780d55941b3e57b461988faca99992322 100644 (file)
  * 20120211.90 (2.4.42-dev) AP_REG_DEFAULT macro in ap_regex.h
  * 20120211.91 (2.4.42-dev) Add ap_is_chunked() in httpd.h
  * 20120211.92 (2.4.42-dev) AP_REG_NO_DEFAULT macro in ap_regex.h
+ * 20120211.93 (2.4.44-dev) Add ap_parse_strict_length()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20120211
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 92                  /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 93                  /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
index fa3be3853fdd105902fe07927b69a6e3f6dffd32..22fa8c405884830df7bd552ff44823288f9cf1ff 100644 (file)
@@ -2052,6 +2052,15 @@ AP_DECLARE(char *) ap_escape_quotes(apr_pool_t *p, const char *instring);
 AP_DECLARE(char *) ap_append_pid(apr_pool_t *p, const char *string,
                                  const char *delim);
 
+/**
+ * Parse a length string with decimal characters only, no leading sign nor
+ * trailing character, like Content-Length or (Content-)Range headers.
+ * @param len The parsed length (apr_off_t)
+ * @param str The string to parse
+ * @return 1 (success), 0 (failure)
+ */
+AP_DECLARE(int) ap_parse_strict_length(apr_off_t *len, const char *str);
+
 /**
  * Parse a given timeout parameter string into an apr_interval_time_t value.
  * The unit of the time interval is given as postfix string to the numeric
index 3b9aa4fadcfb1145cf8dd8d5ea4fe7325227b229..3b4469ef79c18529504d1feed621ffd12be61903 100644 (file)
@@ -1229,6 +1229,16 @@ static apr_status_t cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
         return APR_SUCCESS;
     }
 
+    /* Set the content length if known.
+     */
+    cl = apr_table_get(r->err_headers_out, "Content-Length");
+    if (cl == NULL) {
+        cl = apr_table_get(r->headers_out, "Content-Length");
+    }
+    if (cl && !ap_parse_strict_length(&size, cl)) {
+        reason = "invalid content length";
+    }
+
     if (reason) {
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00768)
                 "cache: %s not cached for request %s. Reason: %s",
@@ -1251,19 +1261,6 @@ static apr_status_t cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     /* Make it so that we don't execute this path again. */
     cache->in_checked = 1;
 
-    /* Set the content length if known.
-     */
-    cl = apr_table_get(r->err_headers_out, "Content-Length");
-    if (cl == NULL) {
-        cl = apr_table_get(r->headers_out, "Content-Length");
-    }
-    if (cl) {
-        char *errp;
-        if (apr_strtoff(&size, cl, &errp, 10) || *errp || size < 0) {
-            cl = NULL; /* parse error, see next 'if' block */
-        }
-    }
-
     if (!cl) {
         /* if we don't get the content-length, see if we have all the
          * buckets and use their length to calculate the size
index 7392b9981a816c9f4c1399d2b8938c0b72e71059..4b1e0227bf3c419ff064549c4c4ccd7b1fca67f5 100644 (file)
@@ -1276,9 +1276,9 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
      * sanity checks.
      */
     if (seen_eos) {
-        const char *cl_header = apr_table_get(r->headers_out, "Content-Length");
-
         if (!dobj->disk_info.header_only) {
+            const char *cl_header;
+            apr_off_t cl;
 
             if (dobj->data.tempfd) {
                 rv = apr_file_close(dobj->data.tempfd);
@@ -1297,6 +1297,7 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
                 apr_pool_destroy(dobj->data.pool);
                 return APR_EGENERAL;
             }
+
             if (dobj->file_size < dconf->minfs) {
                 ap_log_rerror(
                         APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00734) "URL %s failed the size check "
@@ -1305,17 +1306,16 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
                 apr_pool_destroy(dobj->data.pool);
                 return APR_EGENERAL;
             }
-            if (cl_header) {
-                apr_int64_t cl = apr_atoi64(cl_header);
-                if ((errno == 0) && (dobj->file_size != cl)) {
-                    ap_log_rerror(
-                            APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00735) "URL %s didn't receive complete response, not caching", h->cache_obj->key);
-                    /* Remove the intermediate cache file and return non-APR_SUCCESS */
-                    apr_pool_destroy(dobj->data.pool);
-                    return APR_EGENERAL;
-                }
-            }
 
+            cl_header = apr_table_get(r->headers_out, "Content-Length");
+            if (cl_header && (!ap_parse_strict_length(&cl, cl_header)
+                              || cl != dobj->file_size)) {
+                ap_log_rerror(
+                        APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00735) "URL %s didn't receive complete response, not caching", h->cache_obj->key);
+                /* Remove the intermediate cache file and return non-APR_SUCCESS */
+                apr_pool_destroy(dobj->data.pool);
+                return APR_EGENERAL;
+            }
         }
 
         /* All checks were fine, we're good to go when the commit comes */
index 2cdc9504acb11552a7a5098f31f25de12720af9a..978b8b78e77e9173d266a450efc9eb1807ae6bda 100644 (file)
@@ -1044,7 +1044,8 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
     /* Was this the final bucket? If yes, perform sanity checks.
      */
     if (seen_eos) {
-        const char *cl_header = apr_table_get(r->headers_out, "Content-Length");
+        const char *cl_header;
+        apr_off_t cl;
 
         if (r->connection->aborted || r->no_cache) {
             ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02380)
@@ -1055,18 +1056,16 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
             sobj->pool = NULL;
             return APR_EGENERAL;
         }
-        if (cl_header) {
-            apr_off_t cl;
-            char *cl_endp;
-            if (apr_strtoff(&cl, cl_header, &cl_endp, 10) != APR_SUCCESS
-                    || *cl_endp != '\0' || cl != sobj->body_length) {
-                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02381)
-                        "URL %s didn't receive complete response, not caching",
-                        h->cache_obj->key);
-                apr_pool_destroy(sobj->pool);
-                sobj->pool = NULL;
-                return APR_EGENERAL;
-            }
+
+        cl_header = apr_table_get(r->headers_out, "Content-Length");
+        if (cl_header && (!ap_parse_strict_length(&cl, cl_header)
+                          || cl != sobj->body_length)) {
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02381)
+                    "URL %s didn't receive complete response, not caching",
+                    h->cache_obj->key);
+            apr_pool_destroy(sobj->pool);
+            sobj->pool = NULL;
+            return APR_EGENERAL;
         }
 
         /* All checks were fine, we're good to go when the commit comes */
index 73d3ce645ea34f73677ca6730864b047ea4a71b4..bf049df332c2b34587cfab16b6a6fea1889acd9e 100644 (file)
@@ -800,7 +800,6 @@ static int dav_parse_range(request_rec *r,
     char *range;
     char *dash;
     char *slash;
-    char *errp;
 
     range_c = apr_table_get(r->headers_in, "content-range");
     if (range_c == NULL)
@@ -817,20 +816,19 @@ static int dav_parse_range(request_rec *r,
     *dash++ = *slash++ = '\0';
 
     /* detect invalid ranges */
-    if (apr_strtoff(range_start, range + 6, &errp, 10)
-        || *errp || *range_start < 0) {
+    if (!ap_parse_strict_length(range_start, range + 6)) {
         return -1;
     }
-    if (apr_strtoff(range_end, dash, &errp, 10)
-        || *errp || *range_end < 0 || *range_end < *range_start) {
+    if (!ap_parse_strict_length(range_end, dash)
+            || *range_end < *range_start) {
         return -1;
     }
 
     if (*slash != '*') {
         apr_off_t dummy;
 
-        if (apr_strtoff(&dummy, slash, &errp, 10)
-            || *errp || dummy <= *range_end) {
+        if (!ap_parse_strict_length(&dummy, slash)
+                || dummy <= *range_end) {
             return -1;
         }
     }
@@ -2485,20 +2483,13 @@ static int process_mkcol_body(request_rec *r)
         r->read_chunked = 1;
     }
     else if (lenp) {
-        const char *pos = lenp;
-
-        while (apr_isdigit(*pos) || apr_isspace(*pos)) {
-            ++pos;
-        }
-
-        if (*pos != '\0') {
+        if (!ap_parse_strict_length(&r->remaining, lenp)) {
+            r->remaining = 0;
             /* This supplies additional information for the default message. */
             ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00590)
                           "Invalid Content-Length %s", lenp);
             return HTTP_BAD_REQUEST;
         }
-
-        r->remaining = apr_atoi64(lenp);
     }
 
     if (r->read_chunked || r->remaining > 0) {
index d083d322047c429596233de9f7065da649b773d8..ddadd1b360540a3966c30e8cdcf36cb011ef0669 100644 (file)
@@ -107,8 +107,8 @@ static apr_status_t data_out_filter(ap_filter_t *f, apr_bucket_brigade *bb)
         if (content_length) {
             apr_off_t len, clen;
             apr_brigade_length(ctx->bb, 1, &len);
-            clen = apr_atoi64(content_length);
-            if (clen >= 0 && clen < APR_INT32_MAX) {
+            if (ap_parse_strict_length(&clen, content_length)
+                    && clen < APR_INT32_MAX) {
                 ap_set_content_length(r, len +
                                       apr_base64_encode_len((int)clen) - 1);
             }
index 961092d0ea7cb8214f6488a8c54f8641bf8bc15e..5979cb8f727db55f033b56767a65347a27efb26a 100644 (file)
@@ -91,11 +91,16 @@ static int reflector_handler(request_rec * r)
 
         /* reflect the content length, if present */
         if ((content_length = apr_table_get(r->headers_in, "Content-Length"))) {
-            apr_off_t offset;
+            apr_off_t clen;
 
-            apr_strtoff(&offset, content_length, NULL, 10);
-            ap_set_content_length(r, offset);
+            if (!ap_parse_strict_length(&clen, content_length)) {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10243)
+                              "reflector_handler: invalid content-length '%s'",
+                              content_length);
+                return HTTP_BAD_REQUEST;
+            }
 
+            ap_set_content_length(r, clen);
         }
 
         /* reflect the content type, if present */
index 21db7de36cf86c4c79920c56b87ac2dde02994c5..daa8e5977b9d456d23b656c6e056d7ebb27e8ed5 100644 (file)
@@ -76,7 +76,6 @@ static apr_status_t keep_body_filter(ap_filter_t *f, apr_bucket_brigade *b,
 
     if (!ctx) {
         const char *lenp;
-        char *endstr = NULL;
         request_dir_conf *dconf = ap_get_module_config(f->r->per_dir_config,
                                                        &request_module);
 
@@ -93,13 +92,12 @@ static apr_status_t keep_body_filter(ap_filter_t *f, apr_bucket_brigade *b,
         if (lenp) {
 
             /* Protects against over/underflow, non-digit chars in the
-             * string (excluding leading space) (the endstr checks)
-             * and a negative number. */
-            if (apr_strtoff(&ctx->remaining, lenp, &endstr, 10)
-                || endstr == lenp || *endstr || ctx->remaining < 0) {
-
+             * string, leading plus/minus signs, trailing characters and
+             * a negative number.
+             */
+            if (!ap_parse_strict_length(&ctx->remaining, lenp)) {
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r, APLOGNO(01411)
-                              "Invalid Content-Length");
+                              "Invalid Content-Length '%s'", lenp);
 
                 ap_remove_input_filter(f);
                 return bail_out_on_error(b, f, HTTP_REQUEST_ENTITY_TOO_LARGE);
index de585c5750c770160ba3868f31f5fae5b47bb4d7..5ebe853320c39217bcfd7ab654efe3ca8ff6307e 100644 (file)
@@ -152,7 +152,6 @@ static int ap_set_byterange(request_rec *r, apr_off_t clength,
     *indexes = apr_array_make(r->pool, ranges, sizeof(indexes_t));
     while ((cur = ap_getword(r->pool, &range, ','))) {
         char *dash;
-        char *errp;
         apr_off_t number, start, end;
 
         if (!*cur)
@@ -169,7 +168,7 @@ static int ap_set_byterange(request_rec *r, apr_off_t clength,
 
         if (dash == cur) {
             /* In the form "-5" */
-            if (apr_strtoff(&number, dash+1, &errp, 10) || *errp) {
+            if (!ap_parse_strict_length(&number, dash+1)) {
                 return 0;
             }
             if (number < 1) {
@@ -180,12 +179,12 @@ static int ap_set_byterange(request_rec *r, apr_off_t clength,
         }
         else {
             *dash++ = '\0';
-            if (apr_strtoff(&number, cur, &errp, 10) || *errp) {
+            if (!ap_parse_strict_length(&number, cur)) {
                 return 0;
             }
             start = number;
             if (*dash) {
-                if (apr_strtoff(&number, dash, &errp, 10) || *errp) {
+                if (!ap_parse_strict_length(&number, dash)) {
                     return 0;
                 }
                 end = number;
index d507d1695ed91066f92b89a514d8095f9b2c98aa..3bb85c71c031f3b9d969127ed1c05a27d2e70c02 100644 (file)
@@ -405,16 +405,13 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
             lenp = NULL;
         }
         if (lenp) {
-            char *endstr;
-
             ctx->state = BODY_LENGTH;
 
             /* Protects against over/underflow, non-digit chars in the
-             * string (excluding leading space) (the endstr checks)
-             * and a negative number. */
-            if (apr_strtoff(&ctx->remaining, lenp, &endstr, 10)
-                || endstr == lenp || *endstr || ctx->remaining < 0) {
-
+             * string, leading plus/minus signs, trailing characters and
+             * a negative number.
+             */
+            if (!ap_parse_strict_length(&ctx->remaining, lenp)) {
                 ctx->remaining = 0;
                 ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01587)
                               "Invalid Content-Length");
@@ -1734,13 +1731,10 @@ AP_DECLARE(int) ap_setup_client_block(request_rec *r, int read_policy)
         r->read_chunked = 1;
     }
     else if (lenp) {
-        char *endstr;
-
-        if (apr_strtoff(&r->remaining, lenp, &endstr, 10)
-            || *endstr || r->remaining < 0) {
+        if (!ap_parse_strict_length(&r->remaining, lenp)) {
             r->remaining = 0;
             ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(01594)
-                          "Invalid Content-Length");
+                          "Invalid Content-Length '%s'", lenp);
             return HTTP_BAD_REQUEST;
         }
     }
index b6dfedc6c0eb341359dde912406efedd2cd1b23f..e62d67bb1792dd7a66857ec0df472eb381f83424 100644 (file)
@@ -988,19 +988,17 @@ static int read_type_map(apr_file_t **map, negotiation_state *neg,
                 has_content = 1;
             }
             else if (!strncmp(buffer, "content-length:", 15)) {
-                char *errp;
-                apr_off_t number;
+                apr_off_t clen;
 
                 body1 = ap_get_token(neg->pool, &body, 0);
-                if (apr_strtoff(&number, body1, &errp, 10) != APR_SUCCESS
-                    || *errp || number < 0) {
+                if (!ap_parse_strict_length(&clen, body1)) {
                     ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00684)
                                   "Parse error in type map, Content-Length: "
                                   "'%s' in %s is invalid.",
                                   body1, r->filename);
                     break;
                 }
-                mime_info.bytes = number;
+                mime_info.bytes = clen;
                 has_content = 1;
             }
             else if (!strncmp(buffer, "content-language:", 17)) {
index aa62329b1b32d1a1524a9783e65bee1aed9ad21e..e599515d5953e7fe507e11551a8a7dbdad7e9cb3 100644 (file)
@@ -1208,7 +1208,6 @@ static int proxy_handler(request_rec *r)
                     /* Did the scheme handler process the request? */
                     if (access_status != DECLINED) {
                         const char *cl_a;
-                        char *end;
                         apr_off_t cl;
 
                         /*
@@ -1218,18 +1217,17 @@ static int proxy_handler(request_rec *r)
                         if (access_status != HTTP_BAD_GATEWAY) {
                             goto cleanup;
                         }
+
                         cl_a = apr_table_get(r->headers_in, "Content-Length");
-                        if (cl_a) {
-                            apr_strtoff(&cl, cl_a, &end, 10);
+                        if (cl_a && (!ap_parse_strict_length(&cl, cl_a)
+                                     || cl > 0)) {
                             /*
                              * The request body is of length > 0. We cannot
                              * retry with a direct connection since we already
                              * sent (parts of) the request body to the proxy
                              * and do not have any longer.
                              */
-                            if (cl > 0) {
-                                goto cleanup;
-                            }
+                            goto cleanup;
                         }
                         /*
                          * Transfer-Encoding was set as input header, so we had
index 8579fb5d860b5cd4083033befc82237b7bf90f96..28908c161aee7d0fd6f67a28e0b416f7500186dc 100644 (file)
@@ -126,11 +126,8 @@ static apr_off_t get_content_length(request_rec * r)
     if (r->main == NULL) {
         const char *clp = apr_table_get(r->headers_in, "Content-Length");
 
-        if (clp) {
-            char *errp;
-            if (apr_strtoff(&len, clp, &errp, 10) || *errp || len < 0) {
-                len = 0; /* parse error */
-            }
+        if (clp && !ap_parse_strict_length(&len, clp)) {
+            len = -1; /* parse error */
         }
     }
 
@@ -255,10 +252,14 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
     } else {
         /* Get client provided Content-Length header */
         content_length = get_content_length(r);
-        status = ap_get_brigade(r->input_filters, input_brigade,
-                                AP_MODE_READBYTES, APR_BLOCK_READ,
-                                maxsize - AJP_HEADER_SZ);
-
+        if (content_length < 0) {
+            status = APR_EINVAL;
+        }
+        else {
+            status = ap_get_brigade(r->input_filters, input_brigade,
+                                    AP_MODE_READBYTES, APR_BLOCK_READ,
+                                    maxsize - AJP_HEADER_SZ);
+        }
         if (status != APR_SUCCESS) {
             /* We had a failure: Close connection to backend */
             conn->close = 1;
index 7a0f063f705fe6c238c875055b7fdd511d2feb21..044bfcd7ab431399ca18f3109e75734fb45fba34 100644 (file)
@@ -769,9 +769,7 @@ static int ap_proxy_http_prefetch(proxy_http_req_t *req,
     }
     else if (req->old_cl_val) {
         if (r->input_filters == r->proto_input_filters) {
-            char *endstr;
-            status = apr_strtoff(&req->cl_val, req->old_cl_val, &endstr, 10);
-            if (status != APR_SUCCESS || *endstr || req->cl_val < 0) {
+            if (!ap_parse_strict_length(&req->cl_val, req->old_cl_val)) {
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(01085)
                               "could not parse request Content-Length (%s)",
                               req->old_cl_val);
index 74ccda2233011e1b98a52e42722cd253f403287a..d6c67934f52dc1f3835d790dea364e86d673304a 100644 (file)
@@ -1367,7 +1367,7 @@ request_rec *ap_read_request(conn_rec *conn)
     }
 
     if (!r->assbackwards) {
-        const char *tenc;
+        const char *tenc, *clen;
 
         ap_get_mime_headers_core(r, tmp_bb);
         if (r->status != HTTP_OK) {
@@ -1380,9 +1380,27 @@ request_rec *ap_read_request(conn_rec *conn)
             goto traceout;
         }
 
+        clen = apr_table_get(r->headers_in, "Content-Length");
+        if (clen) {
+            apr_off_t cl;
+
+            if (!ap_parse_strict_length(&cl, clen)) {
+                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(10242)
+                              "client sent invalid Content-Length "
+                              "(%s): %s", clen, r->uri);
+                r->status = HTTP_BAD_REQUEST;
+                conn->keepalive = AP_CONN_CLOSE;
+                ap_send_error_response(r, 0);
+                ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
+                ap_run_log_transaction(r);
+                apr_brigade_destroy(tmp_bb);
+                goto traceout;
+            }
+        }
+
         tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
         if (tenc) {
-            /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
+            /* https://tools.ietf.org/html/rfc7230
              * Section 3.3.3.3: "If a Transfer-Encoding header field is
              * present in a request and the chunked transfer coding is not
              * the final encoding ...; the server MUST respond with the 400
@@ -1401,13 +1419,20 @@ request_rec *ap_read_request(conn_rec *conn)
                 goto traceout;
             }
 
-            /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
+            /* https://tools.ietf.org/html/rfc7230
              * Section 3.3.3.3: "If a message is received with both a
              * Transfer-Encoding and a Content-Length header field, the
              * Transfer-Encoding overrides the Content-Length. ... A sender
              * MUST remove the received Content-Length field".
              */
-            apr_table_unset(r->headers_in, "Content-Length");
+            if (clen) {
+                apr_table_unset(r->headers_in, "Content-Length");
+
+                /* Don't reuse this connection anyway to avoid confusion with
+                 * intermediaries and request/reponse spltting.
+                 */
+                conn->keepalive = AP_CONN_CLOSE;
+            }
         }
     }
 
index bfc9cf996d70ffe4af53fe3162d23752a2cc6159..44c61c8709db16a2a667f83689be261d939cf448 100644 (file)
@@ -2589,6 +2589,15 @@ AP_DECLARE(apr_status_t) ap_timeout_parameter_parse(
     return APR_SUCCESS;
 }
 
+AP_DECLARE(int) ap_parse_strict_length(apr_off_t *len, const char *str)
+{
+    char *end;
+
+    return (apr_isdigit(*str)
+            && apr_strtoff(len, str, &end, 10) == APR_SUCCESS
+            && *end == '\0');
+}
+
 /**
  * Determine if a request has a request body or not.
  *
@@ -2598,20 +2607,13 @@ AP_DECLARE(apr_status_t) ap_timeout_parameter_parse(
 AP_DECLARE(int) ap_request_has_body(request_rec *r)
 {
     apr_off_t cl;
-    char *estr;
     const char *cls;
-    int has_body;
-
-    has_body = (!r->header_only
-                && (r->kept_body
-                    || apr_table_get(r->headers_in, "Transfer-Encoding")
-                    || ( (cls = apr_table_get(r->headers_in, "Content-Length"))
-                        && (apr_strtoff(&cl, cls, &estr, 10) == APR_SUCCESS)
-                        && (!*estr)
-                        && (cl > 0) )
-                    )
-                );
-    return has_body;
+
+    return (!r->header_only
+            && (r->kept_body
+                || apr_table_get(r->headers_in, "Transfer-Encoding")
+                || ((cls = apr_table_get(r->headers_in, "Content-Length"))
+                    && ap_parse_strict_length(&cl, cls) && cl > 0)));
 }
 
 AP_DECLARE_NONSTD(apr_status_t) ap_pool_cleanup_set_null(void *data_)