From 56f4552f4fdcecca8063ccd48232f123d39505de Mon Sep 17 00:00:00 2001 From: Ruediger Pluem Date: Sun, 6 Jan 2008 20:32:20 +0000 Subject: * Fix cases with non blocking reads from the ap_http_filter input filter where chunk size lines or empty lines after a chunk are read incomplete. This can lead to a corruption inside the dechunking algorithm. This patch has an issue with larger chunk-extensions which need to get thrown away since we ignore them anyway. PR: 19954, 41056 Tested by: niq git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@609394 13f79535-47bb-0310-9956-ffa450edef68 --- modules/http/http_filters.c | 81 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 71 insertions(+), 10 deletions(-) (limited to 'modules/http') diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c index 4f43340659..2eccb52e22 100644 --- a/modules/http/http_filters.c +++ b/modules/http/http_filters.c @@ -70,8 +70,43 @@ typedef struct http_filter_ctx { BODY_CHUNK_PART } state; int eos_sent; + char chunk_ln[30]; + char *pos; } http_ctx_t; +static apr_status_t get_chunk_line(http_ctx_t *ctx, int len) +{ + /* + * Check if we do not overflow our chunksize / empty line buffer + * (ctx->chunk_ln). If we do the chunksize / empty line is bogus anyway so + * bail out in this case. + * XXX: Currently we are very limited in accepting chunk-extensions. If + * this is needed the chunk_ln buffer must be much larger or we must + * find a different way to discard them as we do not process them anyway. + */ + if ((ctx->pos - ctx->chunk_ln) + len < sizeof(ctx->chunk_ln)) { + ctx->pos += len; + *(ctx->pos) = '\0'; + /* + * Check if we really got a full line. If yes the + * last char in the just read buffer must be LF. + * If not advance the buffer and return APR_EAGAIN. + * We do not start processing until we have the + * full line. + */ + if (ctx->pos[-1] != APR_ASCII_LF) { + return APR_EAGAIN; + } + /* + * Line is complete. So reset ctx->pos for next round. + */ + ctx->pos = ctx->chunk_ln; + return APR_SUCCESS; + } + return APR_ENOSPC; +} + + /* This is the HTTP_INPUT filter for HTTP requests and responses from * proxied servers (mod_proxy). It handles chunked and content-length * bodies. This can only be inserted/used after the headers @@ -98,6 +133,7 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ctx->remaining = 0; ctx->limit_used = 0; ctx->eos_sent = 0; + ctx->pos = ctx->chunk_ln; /* LimitRequestBody does not apply to proxied responses. * Consider implementing this check in its own filter. @@ -229,9 +265,8 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, /* We can't read the chunk until after sending 100 if required. */ if (ctx->state == BODY_CHUNK) { - char line[30]; apr_bucket_brigade *bb; - apr_size_t len = 30; + apr_size_t len = sizeof(ctx->chunk_ln) - (ctx->pos - ctx->chunk_ln); apr_off_t brigade_length; bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc); @@ -258,9 +293,17 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, rv = APR_ENOSPC; } if (rv == APR_SUCCESS) { - rv = apr_brigade_flatten(bb, line, &len); + rv = apr_brigade_flatten(bb, ctx->pos, &len); if (rv == APR_SUCCESS) { - ctx->remaining = get_chunk_size(line); + rv = get_chunk_line(ctx, len); + if (APR_STATUS_IS_EAGAIN(rv)) { + apr_brigade_cleanup(bb); + ctx->state = BODY_CHUNK_PART; + return rv; + } + if (rv == APR_SUCCESS) { + ctx->remaining = get_chunk_size(ctx->chunk_ln); + } } } } @@ -310,9 +353,9 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, case BODY_CHUNK: case BODY_CHUNK_PART: { - char line[30]; apr_bucket_brigade *bb; - apr_size_t len = 30; + apr_size_t len = sizeof(ctx->chunk_ln) + - (ctx->pos - ctx->chunk_ln); apr_status_t http_error = HTTP_REQUEST_ENTITY_TOO_LARGE; bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc); @@ -321,11 +364,21 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, if (ctx->state == BODY_CHUNK) { rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE, block, 0); - apr_brigade_cleanup(bb); if (block == APR_NONBLOCK_READ && - (APR_STATUS_IS_EAGAIN(rv))) { + ( (rv == APR_SUCCESS && APR_BRIGADE_EMPTY(bb)) || + (APR_STATUS_IS_EAGAIN(rv)) )) { return APR_EAGAIN; } + rv = apr_brigade_flatten(bb, ctx->pos, &len); + apr_brigade_cleanup(bb); + if (rv == APR_SUCCESS) { + rv = get_chunk_line(ctx, len); + if (APR_STATUS_IS_EAGAIN(rv)) { + return rv; + } + } + /* Reset len */ + len = sizeof(ctx->chunk_ln) - (ctx->pos - ctx->chunk_ln); } else { rv = APR_SUCCESS; } @@ -343,7 +396,7 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, } ctx->state = BODY_CHUNK; if (rv == APR_SUCCESS) { - rv = apr_brigade_flatten(bb, line, &len); + rv = apr_brigade_flatten(bb, ctx->pos, &len); if (rv == APR_SUCCESS) { /* Wait a sec, that's a blank line! Oh no. */ if (!len) { @@ -351,7 +404,15 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, http_error = HTTP_SERVICE_UNAVAILABLE; } else { - ctx->remaining = get_chunk_size(line); + rv = get_chunk_line(ctx, len); + if (APR_STATUS_IS_EAGAIN(rv)) { + ctx->state = BODY_CHUNK_PART; + apr_brigade_cleanup(bb); + return rv; + } + if (rv == APR_SUCCESS) { + ctx->remaining = get_chunk_size(ctx->chunk_ln); + } } } } -- cgit v1.2.3