If a malformed Content-Range header is received for a PUT request, we

must not use the supplied content per RFC 2616 14.16. Send 400 response
instead of ignoring the Content-Range.

PR: 49825

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

diff --git a/CHANGES b/CHANGES
index 4c6bb5455b5724d02b0212be88f0f045cbd3f279..2bb6fca747156c185119f406728f42a7078c82f7 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,9 @@ Changes with Apache 2.3.9
      Fix a denial of service attack against mod_reqtimeout.
      [Stefan Fritsch]
 
+  *) mod_dav: Send 400 error if malformed Content-Range header is received for
+     a put request (RFC 2616 14.16). PR 49825. [Stefan Fritsch]
+
   *) mod_proxy: Release the backend connection as soon as EOS is detected,
      so the backend isn't forced to wait for the client to eventually
      acknowledge the data. [Graham Leggett]

diff --git a/STATUS b/STATUS
index d5a04103a366a2875e19a8416e82f0bcea6ff2f1..61aa891b5b5a940f71e0508bbfb3464646986ac8 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -142,7 +142,7 @@ RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP:
   * RFC 2616 violations.
     Closed PRs: 15852, 15857, 15859, 15861, 15864, 15869, 15870, 16120,
                 16125, 16135, 16136, 16137, 16138, 16139, 16140, 16518,
-                16520
+                16520, 49825 
     Open PRs:   15865, 15866, 15868, 16126, 16133, 16142, 16521
     jerenkrantz says: need to decide how many we need to backport and/or
                       if these rise to showstopper status.

diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c
index 081d72440330cc94d66cfbad9f11b736b2cd0968..42d3abaa751d57881d483ba79c919010ba14d2c6 100644 (file)
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -751,6 +751,11 @@ static dav_error * dav_open_lockdb(request_rec *r, int ro, dav_lockdb **lockdb)
     return (*hooks->open_lockdb)(r, ro, 0, lockdb);
 }
 
+/**
+ * @return  1 if valid content-range,
+ *          0 if no content-range,
+ *         -1 if malformed content-range
+ */
 static int dav_parse_range(request_rec *r,
                            apr_off_t *range_start, apr_off_t *range_end)
 {
@@ -768,21 +773,20 @@ static int dav_parse_range(request_rec *r,
     if (strncasecmp(range, "bytes ", 6) != 0
         || (dash = ap_strchr(range, '-')) == NULL
         || (slash = ap_strchr(range, '/')) == NULL) {
-        /* malformed header. ignore it (per S14.16 of RFC2616) */
-        return 0;
+        /* malformed header */
+        return -1;
     }
 
     *dash++ = *slash++ = '\0';
 
-    /* ignore invalid ranges. (per S14.16 of RFC2616) */
+    /* detect invalid ranges */
     if (apr_strtoff(range_start, range + 6, &errp, 10)
         || *errp || *range_start < 0) {
-        return 0;
+        return -1;
     }
-
     if (apr_strtoff(range_end, dash, &errp, 10)
         || *errp || *range_end < 0 || *range_end < *range_start) {
-        return 0;
+        return -1;
     }
 
     if (*slash != '*') {
@@ -790,7 +794,7 @@ static int dav_parse_range(request_rec *r,
 
         if (apr_strtoff(&dummy, slash, &errp, 10)
             || *errp || dummy <= *range_end) {
-            return 0;
+            return -1;
         }
     }
 
@@ -929,6 +933,22 @@ static int dav_method_put(request_rec *r)
         return dav_handle_err(r, err, multi_response);
     }
 
+    has_range = dav_parse_range(r, &range_start, &range_end);
+    if (has_range < 0) {
+        /* RFC 2616 14.16: If we receive an invalid Content-Range we must
+         * not use the content.
+         */
+        body = apr_psprintf(r->pool,
+                            "Malformed Content-Range header for PUT %s.",
+                            ap_escape_html(r->pool, r->uri));
+        return dav_error_response(r, HTTP_BAD_REQUEST, body);
+    } else if (has_range) {
+        mode = DAV_MODE_WRITE_SEEKABLE;
+    }
+    else {
+        mode = DAV_MODE_WRITE_TRUNC;
+    }
+
     /* make sure the resource can be modified (if versioning repository) */
     if ((err = dav_auto_checkout(r, resource,
                                  0 /* not parent_only */,
@@ -937,14 +957,6 @@ static int dav_method_put(request_rec *r)
         return dav_handle_err(r, err, NULL);
     }
 
-    /* truncate and rewrite the file unless we see a Content-Range */
-    mode = DAV_MODE_WRITE_TRUNC;
-
-    has_range = dav_parse_range(r, &range_start, &range_end);
-    if (has_range) {
-        mode = DAV_MODE_WRITE_SEEKABLE;
-    }
-
     /* Create the new file in the repository */
     if ((err = (*resource->hooks->open_stream)(resource, mode,
                                                &stream)) != NULL) {