httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j..@apache.org
Subject svn commit: r1165607 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/http/byterange_filter.c
Date Tue, 06 Sep 2011 11:26:29 GMT
Author: jim
Date: Tue Sep  6 11:26:28 2011
New Revision: 1165607

URL: http://svn.apache.org/viewvc?rev=1165607&view=rev
Log:
core: All the latest Range fixes through 1165268

Modified:
    httpd/httpd/branches/2.2.x/CHANGES
    httpd/httpd/branches/2.2.x/STATUS
    httpd/httpd/branches/2.2.x/modules/http/byterange_filter.c

Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=1165607&r1=1165606&r2=1165607&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Tue Sep  6 11:26:28 2011
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.2.21
 
+  *) Fix a regression in the CVE-2011-3192 byterange fix.
+     PR 51748. [low_priority <lowprio20 gmail.com>]
+
   *) mod_dav_fs: Fix segfault if apr DBM driver cannot be loaded. PR 51751.
      [Stefan Fritsch]
 

Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1165607&r1=1165606&r2=1165607&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Tue Sep  6 11:26:28 2011
@@ -93,16 +93,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  * core: All the latest Range fixes through 1165268
-    Trunk patch: Revisions 1163819-1165268 of byterange_filter.c
-      (see http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/byterange_filter.c?view=log)
-    CHANGES entry for the critical issue in the patch:
-        *) Fix a regression in the CVE-2011-3192 byterange fix.
-           PR 51748. [low_priority <lowprio20 gmail.com>]
-    2.2.x patch: http://people.apache.org/~trawick/byterange_filter-1163819-through-1165268.txt
-    Diff between trunk r1165268 and 2.2.x with this patch:
-                 http://people.apache.org/~trawick/byterange_filter-1165268-2.2.x-vs-trunk.txt
-    +1: trawick, rpluem, jim
 
  PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

Modified: httpd/httpd/branches/2.2.x/modules/http/byterange_filter.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/http/byterange_filter.c?rev=1165607&r1=1165606&r2=1165607&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/http/byterange_filter.c (original)
+++ httpd/httpd/branches/2.2.x/modules/http/byterange_filter.c Tue Sep  6 11:26:28 2011
@@ -87,8 +87,6 @@ static apr_status_t copy_brigade_range(a
     apr_bucket *first = NULL, *last = NULL, *out_first = NULL, *e;
     apr_uint64_t pos = 0, off_first = 0, off_last = 0;
     apr_status_t rv;
-    const char *s;
-    apr_size_t len;
     apr_uint64_t start64, end64;
     apr_off_t pofft = 0;
 
@@ -140,43 +138,9 @@ static apr_status_t copy_brigade_range(a
         if (e == first) {
             if (off_first != start64) {
                 rv = apr_bucket_split(copy, (apr_size_t)(start64 - off_first));
-                if (rv == APR_ENOTIMPL) {
-                    rv = apr_bucket_read(copy, &s, &len, APR_BLOCK_READ);
-                    if (rv != APR_SUCCESS) {
-                        apr_brigade_cleanup(bbout);
-                        return rv;
-                    }
-                    /*
-                     * The read above might have morphed copy in a bucket
-                     * of shorter length. So read and delete until we reached
-                     * the correct bucket for splitting.
-                     */
-                    while (start64 - off_first > (apr_uint64_t)copy->length) {
-                        apr_bucket *tmp = APR_BUCKET_NEXT(copy);
-                        off_first += (apr_uint64_t)copy->length;
-                        APR_BUCKET_REMOVE(copy);
-                        apr_bucket_destroy(copy);
-                        copy = tmp;
-                        rv = apr_bucket_read(copy, &s, &len, APR_BLOCK_READ);
-                        if (rv != APR_SUCCESS) {
-                            apr_brigade_cleanup(bbout);
-                            return rv;
-                        }
-                    }
-                    if (start64 > off_first) {
-                        rv = apr_bucket_split(copy, (apr_size_t)(start64 - off_first));
-                        if (rv != APR_SUCCESS) {
-                            apr_brigade_cleanup(bbout);
-                            return rv;
-                        }
-                    }
-                    else {
-                        copy = APR_BUCKET_PREV(copy);
-                    }
-                }
-                else if (rv != APR_SUCCESS) {
-                        apr_brigade_cleanup(bbout);
-                        return rv;
+                if (rv != APR_SUCCESS) {
+                    apr_brigade_cleanup(bbout);
+                    return rv;
                 }
                 out_first = APR_BUCKET_NEXT(copy);
                 APR_BUCKET_REMOVE(copy);
@@ -193,37 +157,9 @@ static apr_status_t copy_brigade_range(a
             }
             if (end64 - off_last != (apr_uint64_t)e->length) {
                 rv = apr_bucket_split(copy, (apr_size_t)(end64 + 1 - off_last));
-                if (rv == APR_ENOTIMPL) {
-                    rv = apr_bucket_read(copy, &s, &len, APR_BLOCK_READ);
-                    if (rv != APR_SUCCESS) {
-                        apr_brigade_cleanup(bbout);
-                        return rv;
-                    }
-                    /*
-                     * The read above might have morphed copy in a bucket
-                     * of shorter length. So read until we reached
-                     * the correct bucket for splitting.
-                     */
-                    while (end64 + 1 - off_last > (apr_uint64_t)copy->length) {
-                        off_last += (apr_uint64_t)copy->length;
-                        copy = APR_BUCKET_NEXT(copy);
-                        rv = apr_bucket_read(copy, &s, &len, APR_BLOCK_READ);
-                        if (rv != APR_SUCCESS) {
-                            apr_brigade_cleanup(bbout);
-                            return rv;
-                        }
-                    }
-                    if (end64 < off_last + (apr_uint64_t)copy->length - 1) {
-                        rv = apr_bucket_split(copy, end64 + 1 - off_last);
-                        if (rv != APR_SUCCESS) {
-                            apr_brigade_cleanup(bbout);
-                            return rv;
-                        }
-                    }
-                }
-                else if (rv != APR_SUCCESS) {
-                        apr_brigade_cleanup(bbout);
-                        return rv;
+                if (rv != APR_SUCCESS) {
+                    apr_brigade_cleanup(bbout);
+                    return rv;
                 }
                 copy = APR_BUCKET_NEXT(copy);
                 if (copy != APR_BRIGADE_SENTINEL(bbout)) {
@@ -253,6 +189,20 @@ static int get_max_ranges(request_rec *r
     return core_conf->max_ranges == -1 ? DEFAULT_MAX_RANGES : core_conf->max_ranges;
 }
 
+static apr_status_t send_416(ap_filter_t *f, apr_bucket_brigade *tmpbb)
+{
+    apr_bucket *e;
+    conn_rec *c = f->r->connection;
+    ap_remove_output_filter(f);
+    f->r->status = HTTP_OK;
+    e = ap_bucket_error_create(HTTP_RANGE_NOT_SATISFIABLE, NULL,
+                               f->r->pool, c->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(tmpbb, e);
+    e = apr_bucket_eos_create(c->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(tmpbb, e);
+    return ap_pass_brigade(f->next, tmpbb);
+}
+
 AP_CORE_DECLARE_NONSTD(apr_status_t) ap_byterange_filter(ap_filter_t *f,
                                                          apr_bucket_brigade *bb)
 {
@@ -271,8 +221,8 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
     char *bound_head = NULL;
     apr_array_header_t *indexes;
     indexes_t *idx;
-    int original_status;
     int i;
+    int original_status;
     int max_ranges = get_max_ranges(r);
 
     /*
@@ -307,11 +257,17 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
         return ap_pass_brigade(f->next, bb);
     }
 
+    /* this brigade holds what we will be sending */
+    bsend = apr_brigade_create(r->pool, c->bucket_alloc);
+
+    if (num_ranges < 0)
+        return send_416(f, bsend);
+
     if (num_ranges > 1) {
         /* Is ap_make_content_type required here? */
         const char *orig_ct = ap_make_content_type(r, r->content_type);
         boundary = apr_psprintf(r->pool, "%" APR_UINT64_T_HEX_FMT "%lx",
-                                (apr_uint64_t)r->request_time, (long) getpid());
+                                (apr_uint64_t)r->request_time, c->id);
 
         ap_set_content_type(r, apr_pstrcat(r->pool, "multipart",
                                            use_range_x(r) ? "/x-" : "/",
@@ -336,8 +292,6 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
         ap_xlate_proto_to_ascii(bound_head, strlen(bound_head));
     }
 
-    /* this brigade holds what we will be sending */
-    bsend = apr_brigade_create(r->pool, c->bucket_alloc);
     tmpbb = apr_brigade_create(r->pool, c->bucket_alloc);
 
     idx = (indexes_t *)indexes->elts;
@@ -395,15 +349,8 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
     }
 
     if (found == 0) {
-        ap_remove_output_filter(f);
-        r->status = HTTP_OK;
         /* bsend is assumed to be empty if we get here. */
-        e = ap_bucket_error_create(HTTP_RANGE_NOT_SATISFIABLE, NULL,
-                                   r->pool, c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(bsend, e);
-        e = apr_bucket_eos_create(c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(bsend, e);
-        return ap_pass_brigade(f->next, bsend);
+        return send_416(f, bsend);
     }
 
     if (num_ranges > 1) {
@@ -435,7 +382,7 @@ static int ap_set_byterange(request_rec 
     const char *match;
     const char *ct;
     char *cur;
-    int num_ranges = 0;
+    int num_ranges = 0, unsatisfiable = 0;
     apr_off_t sum_lengths = 0;
     indexes_t *idx;
     int ranges = 1;
@@ -508,14 +455,25 @@ static int ap_set_byterange(request_rec 
         char *errp;
         apr_off_t number, start, end;
 
-        if (!(dash = strchr(cur, '-'))) {
+        if (!*cur)
             break;
+
+        /*
+         * Per RFC 2616 14.35.1: If there is at least one syntactically invalid
+         * byte-range-spec, we must ignore the whole header.
+         */
+
+        if (!(dash = strchr(cur, '-'))) {
+            return 0;
         }
 
-        if (dash == range) {
+        if (dash == cur) {
             /* In the form "-5" */
             if (apr_strtoff(&number, dash+1, &errp, 10) || *errp) {
-                break;
+                return 0;
+            }
+            if (number < 1) {
+                return 0;
             }
             start = clength - number;
             end = clength - 1;
@@ -523,14 +481,17 @@ static int ap_set_byterange(request_rec 
         else {
             *dash++ = '\0';
             if (apr_strtoff(&number, cur, &errp, 10) || *errp) {
-                break;
+                return 0;
             }
             start = number;
             if (*dash) {
                 if (apr_strtoff(&number, dash, &errp, 10) || *errp) {
-                    break;
+                    return 0;
                 }
                 end = number;
+                if (start > end) {
+                    return 0;
+                }
             }
             else {                  /* "5-" */
                 end = clength - 1;
@@ -540,15 +501,14 @@ static int ap_set_byterange(request_rec 
         if (start < 0) {
             start = 0;
         }
+        if (start >= clength) {
+            unsatisfiable = 1;
+            continue;
+        }
         if (end >= clength) {
             end = clength - 1;
         }
 
-        if (start > end) {
-            /* ignore? count? */
-            break;
-        }
-
         idx = (indexes_t *)apr_array_push(*indexes);
         idx->start = start;
         idx->end = end;
@@ -557,6 +517,10 @@ static int ap_set_byterange(request_rec 
         num_ranges++;
     }
 
+    if (num_ranges == 0 && unsatisfiable) {
+        /* If all ranges are unsatisfiable, we should return 416 */
+        return -1;
+    }
     if (sum_lengths >= clength) {
         ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
                       "Sum of ranges not smaller than file, ignoring.");



Mime
View raw message