You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2011/09/06 13:26:29 UTC
svn commit: r1165607 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS
modules/http/byterange_filter.c
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.");