Skip to content

Commit

Permalink
Limit accepted chunk-size to 2^63-1 and be strict about chunk-ext
Browse files Browse the repository at this point in the history 
authorized characters.

Submitted by: Yann Ylavic



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1684513 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@wrowe
wrowe committed Jun 9, 2015
1 parent 15e096c commit e427c41
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 48 deletions.
3 changes: 3 additions & 0 deletions CHANGES
View file
@@ -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]
Expand Down
2 changes: 1 addition & 1 deletion docs/log-message-tags/next-number
View file
@@ -1 +1 @@
2901
2902
130 changes: 83 additions & 47 deletions modules/http/http_filters.c
View file
Expand Up @@ -57,24 +57,24 @@

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 */
BODY_LENGTH, /* data constrained by content length */
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;
Expand All @@ -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;

Expand All @@ -99,10 +99,20 @@ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
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;
}
Expand All @@ -111,44 +121,62 @@ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
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;
}
else {
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') {
i++;
continue;
}

ctx->chunkbits -= 4;
if (ctx->chunkbits < 0) {
/* overflow */
return APR_ENOSPC;
}

if (c >= '0' && c <= '9') {
xvalue = c - '0';
}
Expand All @@ -160,16 +188,18 @@ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
}
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++;
Expand Down Expand Up @@ -232,7 +262,8 @@ static apr_status_t read_chunked_trailers(http_ctx_t *ctx, ap_filter_t *f,
* 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;
Expand Down Expand Up @@ -282,8 +313,8 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
* 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 {
Expand All @@ -308,22 +339,20 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
|| 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;
}
}
Expand Down Expand Up @@ -378,6 +407,7 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
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;
}
Expand All @@ -401,7 +431,9 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
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);

Expand Down Expand Up @@ -433,8 +465,9 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
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;
}
}
Expand All @@ -446,9 +479,8 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,

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;
Expand Down Expand Up @@ -522,9 +554,10 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
* 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;
}
}
Expand All @@ -549,7 +582,10 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
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;
}
}

Expand Down

0 comments on commit e427c41

Please sign in to comment.