You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2015/06/09 22:12:32 UTC
svn commit: r1684513 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/http/http_filters.c
Author: wrowe
Date: Tue Jun 9 20:12:31 2015
New Revision: 1684513
URL: http://svn.apache.org/r1684513
Log:
Limit accepted chunk-size to 2^63-1 and be strict about chunk-ext
authorized characters.
Submitted by: Yann Ylavic
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/docs/log-message-tags/next-number
httpd/httpd/trunk/modules/http/http_filters.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1684513&r1=1684512&r2=1684513&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue Jun 9 20:12:31 2015
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) core: Limit accepted chunk-size to 2^63-1 and be strict about chunk-ext
+ authorized characters. [Yann Ylavic]
+
*) mod_ssl: When SSLVerify is disabled (NONE), don't force a renegotiation if
the SSLVerifyDepth applied with the default/handshaken vhost differs from
the one applicable with the finally selected vhost. [Yann Ylavic]
Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1684513&r1=1684512&r2=1684513&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
+++ httpd/httpd/trunk/docs/log-message-tags/next-number Tue Jun 9 20:12:31 2015
@@ -1 +1 @@
-2901
+2902
Modified: httpd/httpd/trunk/modules/http/http_filters.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1684513&r1=1684512&r2=1684513&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_filters.c (original)
+++ httpd/httpd/trunk/modules/http/http_filters.c Tue Jun 9 20:12:31 2015
@@ -57,15 +57,13 @@
APLOG_USE_MODULE(http);
-#define INVALID_CHAR -2
-
typedef struct http_filter_ctx
{
apr_off_t remaining;
apr_off_t limit;
apr_off_t limit_used;
apr_int32_t chunk_used;
- apr_int16_t chunkbits;
+ apr_int32_t chunkbits;
enum
{
BODY_NONE, /* streamed data */
@@ -73,8 +71,10 @@ typedef struct http_filter_ctx
BODY_CHUNK, /* chunk expected */
BODY_CHUNK_PART, /* chunk digits */
BODY_CHUNK_EXT, /* chunk extension */
+ BODY_CHUNK_LF, /* got CR, expect LF after digits/extension */
BODY_CHUNK_DATA, /* data constrained by chunked encoding */
- BODY_CHUNK_END, /* chunk terminating CRLF */
+ BODY_CHUNK_END, /* chunked data terminating CRLF */
+ BODY_CHUNK_END_LF, /* got CR, expect LF after data */
BODY_CHUNK_TRAILER /* trailers */
} state;
unsigned int eos_sent :1;
@@ -89,7 +89,7 @@ typedef struct http_filter_ctx
* In general, any negative number can be considered an overflow error.
*/
static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
- apr_size_t len, int linelimit)
+ apr_size_t len, int linelimit)
{
apr_size_t i = 0;
@@ -99,10 +99,20 @@ static apr_status_t parse_chunk_size(htt
ap_xlate_proto_from_ascii(&c, 1);
/* handle CRLF after the chunk */
- if (ctx->state == BODY_CHUNK_END) {
+ if (ctx->state == BODY_CHUNK_END
+ || ctx->state == BODY_CHUNK_END_LF) {
if (c == LF) {
ctx->state = BODY_CHUNK;
}
+ else if (c == CR && ctx->state == BODY_CHUNK_END) {
+ ctx->state = BODY_CHUNK_END_LF;
+ }
+ else {
+ /*
+ * LF expected.
+ */
+ return APR_EINVAL;
+ }
i++;
continue;
}
@@ -111,28 +121,20 @@ static apr_status_t parse_chunk_size(htt
if (ctx->state == BODY_CHUNK) {
if (!apr_isxdigit(c)) {
/*
- * Detect invalid character at beginning. This also works for empty
- * chunk size lines.
+ * Detect invalid character at beginning. This also works for
+ * empty chunk size lines.
*/
- return APR_EGENERAL;
+ return APR_EINVAL;
}
else {
ctx->state = BODY_CHUNK_PART;
}
ctx->remaining = 0;
- ctx->chunkbits = sizeof(long) * 8;
+ ctx->chunkbits = sizeof(apr_off_t) * 8;
ctx->chunk_used = 0;
}
- /* handle a chunk part, or a chunk extension */
- /*
- * In theory, we are supposed to expect CRLF only, but our
- * test suite sends LF only. Tolerate a missing CR.
- */
- if (c == ';' || c == CR) {
- ctx->state = BODY_CHUNK_EXT;
- }
- else if (c == LF) {
+ if (c == LF) {
if (ctx->remaining) {
ctx->state = BODY_CHUNK_DATA;
}
@@ -140,8 +142,28 @@ static apr_status_t parse_chunk_size(htt
ctx->state = BODY_CHUNK_TRAILER;
}
}
- else if (ctx->state != BODY_CHUNK_EXT) {
- int xvalue = 0;
+ else if (ctx->state == BODY_CHUNK_LF) {
+ /*
+ * LF expected.
+ */
+ return APR_EINVAL;
+ }
+ else if (c == CR) {
+ ctx->state = BODY_CHUNK_LF;
+ }
+ else if (c == ';') {
+ ctx->state = BODY_CHUNK_EXT;
+ }
+ else if (ctx->state == BODY_CHUNK_EXT) {
+ /*
+ * Control chars (but tabs) are invalid.
+ */
+ if (c != '\t' && apr_iscntrl(c)) {
+ return APR_EINVAL;
+ }
+ }
+ else if (ctx->state == BODY_CHUNK_PART) {
+ int xvalue;
/* ignore leading zeros */
if (!ctx->remaining && c == '0') {
@@ -149,6 +171,12 @@ static apr_status_t parse_chunk_size(htt
continue;
}
+ ctx->chunkbits -= 4;
+ if (ctx->chunkbits < 0) {
+ /* overflow */
+ return APR_ENOSPC;
+ }
+
if (c >= '0' && c <= '9') {
xvalue = c - '0';
}
@@ -160,16 +188,18 @@ static apr_status_t parse_chunk_size(htt
}
else {
/* bogus character */
- return APR_EGENERAL;
+ return APR_EINVAL;
}
ctx->remaining = (ctx->remaining << 4) | xvalue;
- ctx->chunkbits -= 4;
- if (ctx->chunkbits <= 0 || ctx->remaining < 0) {
+ if (ctx->remaining < 0) {
/* overflow */
return APR_ENOSPC;
}
-
+ }
+ else {
+ /* Should not happen */
+ return APR_EGENERAL;
}
i++;
@@ -232,7 +262,8 @@ static apr_status_t read_chunked_trailer
* are successfully parsed.
*/
apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
- ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes)
+ ap_input_mode_t mode, apr_read_type_e block,
+ apr_off_t readbytes)
{
core_server_config *conf;
apr_bucket *e;
@@ -282,8 +313,8 @@ apr_status_t ap_http_filter(ap_filter_t
* reading the connection until it is closed by the server."
*/
ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(02555)
- "Unknown Transfer-Encoding: %s;"
- " using read-until-close", tenc);
+ "Unknown Transfer-Encoding: %s; "
+ "using read-until-close", tenc);
tenc = NULL;
}
else {
@@ -308,22 +339,20 @@ apr_status_t ap_http_filter(ap_filter_t
|| endstr == lenp || *endstr || ctx->remaining < 0) {
ctx->remaining = 0;
- ap_log_rerror(
- APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01587)
- "Invalid Content-Length");
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01587)
+ "Invalid Content-Length");
- return APR_ENOSPC;
+ return APR_EINVAL;
}
/* If we have a limit in effect and we know the C-L ahead of
* time, stop it here if it is invalid.
*/
if (ctx->limit && ctx->limit < ctx->remaining) {
- ap_log_rerror(
- APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01588)
- "Requested content-length of %" APR_OFF_T_FMT
- " is larger than the configured limit"
- " of %" APR_OFF_T_FMT, ctx->remaining, ctx->limit);
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01588)
+ "Requested content-length of %" APR_OFF_T_FMT
+ " is larger than the configured limit"
+ " of %" APR_OFF_T_FMT, ctx->remaining, ctx->limit);
return APR_ENOSPC;
}
}
@@ -378,6 +407,7 @@ apr_status_t ap_http_filter(ap_filter_t
APR_BRIGADE_INSERT_TAIL(bb, e);
rv = ap_pass_brigade(f->c->output_filters, bb);
+ apr_brigade_cleanup(bb);
if (rv != APR_SUCCESS) {
return AP_FILTER_ERROR;
}
@@ -401,7 +431,9 @@ apr_status_t ap_http_filter(ap_filter_t
case BODY_CHUNK:
case BODY_CHUNK_PART:
case BODY_CHUNK_EXT:
- case BODY_CHUNK_END: {
+ case BODY_CHUNK_LF:
+ case BODY_CHUNK_END:
+ case BODY_CHUNK_END_LF: {
rv = ap_get_brigade(f->next, b, AP_MODE_GETLINE, block, 0);
@@ -433,8 +465,9 @@ apr_status_t ap_http_filter(ap_filter_t
f->r->server->limit_req_fieldsize);
}
if (rv != APR_SUCCESS) {
- ap_log_rerror(
- APLOG_MARK, APLOG_INFO, rv, f->r, APLOGNO(01590) "Error reading chunk %s ", (APR_ENOSPC == rv) ? "(overflow)" : "");
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, rv, f->r, APLOGNO(01590)
+ "Error reading chunk %s ",
+ (APR_ENOSPC == rv) ? "(overflow)" : "");
return rv;
}
}
@@ -446,9 +479,8 @@ apr_status_t ap_http_filter(ap_filter_t
if (ctx->state == BODY_CHUNK_TRAILER) {
/* Treat UNSET as DISABLE - trailers aren't merged by default */
- int merge_trailers =
- conf->merge_trailers == AP_MERGE_TRAILERS_ENABLE;
- return read_chunked_trailers(ctx, f, b, merge_trailers);
+ return read_chunked_trailers(ctx, f, b,
+ conf->merge_trailers == AP_MERGE_TRAILERS_ENABLE);
}
break;
@@ -522,9 +554,10 @@ apr_status_t ap_http_filter(ap_filter_t
* really count. This seems to be up for interpretation. */
ctx->limit_used += totalread;
if (ctx->limit < ctx->limit_used) {
- ap_log_rerror(
- APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01591) "Read content-length of %" APR_OFF_T_FMT " is larger than the configured limit"
- " of %" APR_OFF_T_FMT, ctx->limit_used, ctx->limit);
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01591)
+ "Read content-length of %" APR_OFF_T_FMT
+ " is larger than the configured limit"
+ " of %" APR_OFF_T_FMT, ctx->limit_used, ctx->limit);
return APR_ENOSPC;
}
}
@@ -549,7 +582,10 @@ apr_status_t ap_http_filter(ap_filter_t
break;
}
default: {
- break;
+ /* Should not happen */
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r, APLOGNO(02901)
+ "Unexpected body state (%i)", (int)ctx->state);
+ return APR_EGENERAL;
}
}