From f01aba748667fdb369cbb6905a7d1884a469513c Mon Sep 17 00:00:00 2001 From: Jim Jagielski Date: Fri, 27 Mar 2015 12:20:26 +0000 Subject: [PATCH] Merge r1619383, r1619444, r1662245, r1662246 from trunk: A misplaced check for inflation limits prevented limiting relatively small inputs. PR56872 Submitted By: Edward Lu Committed By: covener mod_deflate: follow up to r1619383. deflate_in_filter(): - use inflated bytes per inflate() call to compute the total output bytes, - check zlib errors before limits, - add missing check_ratio() when asked to flush. deflate_out_filter(): - check ratio after each inflate() call. mod_deflate: follow up to r1619383 and r1619444: CHANGES entry. CHANGES: follow up to r1662245: Add PR number. Submitted by: covener, ylavic, ylavic, ylavic Reviewed/backported by: jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1669555 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 + STATUS | 10 ---- modules/filters/mod_deflate.c | 102 +++++++++++++++++++--------------- 3 files changed, 59 insertions(+), 56 deletions(-) diff --git a/CHANGES b/CHANGES index 12e49354aff..360aaad4106 100644 --- a/CHANGES +++ b/CHANGES @@ -44,6 +44,9 @@ Changes with Apache 2.4.13 *) mod_ssl: Fix renegotiation failures redirected to an ErrorDocument. PR 57334. [Yann Ylavic]. + *) mod_deflate: A misplaced check prevents limiting small bodies with the + new inflate limits. PR56872. [Edward Lu, Eric Covener, Yann Ylavic] + *) mod_proxy_ajp: Forward SSL protocol name (SSLv3, TLSv1.1 etc.) as a request attribute to the backend. Recent Tomcat versions will extract it and provide it as a servlet request attribute named diff --git a/STATUS b/STATUS index c9873619c39..778c8b54496 100644 --- a/STATUS +++ b/STATUS @@ -106,16 +106,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - *) mod_deflate: A misplaced check prevents limiting small bodies with the - new inflate limits. PR56872. - trunk patch: http://svn.apache.org/r1619383 - http://svn.apache.org/r1619444 - http://svn.apache.org/r1662245 (CHANGES entry) - http://svn.apache.org/r1662246 (updated CHANGES entry) - 2.4.x patch: trunk works (modulo CHANGES) - http://people.apache.org/~ylavic/httpd-2.4.x-mod_deflate-PR56872.patch - +1: ylavic, covener, jim - *) mod_proxy: use the original (non absolute) form of the request-line's URI for requests embedded in CONNECT payloads used to connect SSL backends via a ProxyRemote forward-proxy. PR 55892. diff --git a/modules/filters/mod_deflate.c b/modules/filters/mod_deflate.c index b892fa7c762..5e390d78750 100644 --- a/modules/filters/mod_deflate.c +++ b/modules/filters/mod_deflate.c @@ -1278,6 +1278,15 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, return APR_ENOSPC; } + if (!check_ratio(r, ctx, dc)) { + inflateEnd(&ctx->stream); + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO() + "Inflated content ratio is larger than the " + "configured limit %i by %i time(s)", + dc->ratio_limit, dc->ratio_burst); + return APR_EINVAL; + } + ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len); tmp_b = apr_bucket_heap_create((char *)ctx->buffer, len, NULL, f->c->bucket_alloc); @@ -1335,26 +1344,6 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, ctx->stream.next_out = ctx->buffer; len = c->bufferSize - ctx->stream.avail_out; - ctx->inflate_total += len; - if (inflate_limit && ctx->inflate_total > inflate_limit) { - inflateEnd(&ctx->stream); - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02648) - "Inflated content length of %" APR_OFF_T_FMT - " is larger than the configured limit" - " of %" APR_OFF_T_FMT, - ctx->inflate_total, inflate_limit); - return APR_ENOSPC; - } - - if (!check_ratio(r, ctx, dc)) { - inflateEnd(&ctx->stream); - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02649) - "Inflated content ratio is larger than the " - "configured limit %i by %i time(s)", - dc->ratio_limit, dc->ratio_burst); - return APR_EINVAL; - } - ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len); tmp_heap = apr_bucket_heap_create((char *)ctx->buffer, len, NULL, f->c->bucket_alloc); @@ -1362,22 +1351,43 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, ctx->stream.avail_out = c->bufferSize; } + len = ctx->stream.avail_out; zRC = inflate(&ctx->stream, Z_NO_FLUSH); - if (zRC == Z_STREAM_END) { - ctx->validation_buffer = apr_pcalloc(r->pool, - VALIDATION_SIZE); - ctx->validation_buffer_length = 0; - break; - } - - if (zRC != Z_OK) { + if (zRC != Z_OK && zRC != Z_STREAM_END) { inflateEnd(&ctx->stream); ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01392) "Zlib error %d inflating data (%s)", zRC, ctx->stream.msg); return APR_EGENERAL; } + + ctx->inflate_total += len - ctx->stream.avail_out; + if (inflate_limit && ctx->inflate_total > inflate_limit) { + inflateEnd(&ctx->stream); + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02648) + "Inflated content length of %" APR_OFF_T_FMT + " is larger than the configured limit" + " of %" APR_OFF_T_FMT, + ctx->inflate_total, inflate_limit); + return APR_ENOSPC; + } + + if (!check_ratio(r, ctx, dc)) { + inflateEnd(&ctx->stream); + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02649) + "Inflated content ratio is larger than the " + "configured limit %i by %i time(s)", + dc->ratio_limit, dc->ratio_burst); + return APR_EINVAL; + } + + if (zRC == Z_STREAM_END) { + ctx->validation_buffer = apr_pcalloc(r->pool, + VALIDATION_SIZE); + ctx->validation_buffer_length = 0; + break; + } } } @@ -1786,15 +1796,6 @@ static apr_status_t inflate_out_filter(ap_filter_t *f, while (ctx->stream.avail_in != 0) { if (ctx->stream.avail_out == 0) { - - if (!check_ratio(r, ctx, dc)) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02650) - "Inflated content ratio is larger than the " - "configured limit %i by %i time(s)", - dc->ratio_limit, dc->ratio_burst); - return APR_EINVAL; - } - ctx->stream.next_out = ctx->buffer; len = c->bufferSize - ctx->stream.avail_out; @@ -1812,6 +1813,21 @@ static apr_status_t inflate_out_filter(ap_filter_t *f, zRC = inflate(&ctx->stream, Z_NO_FLUSH); + if (zRC != Z_OK && zRC != Z_STREAM_END) { + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01409) + "Zlib error %d inflating data (%s)", zRC, + ctx->stream.msg); + return APR_EGENERAL; + } + + if (!check_ratio(r, ctx, dc)) { + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(02650) + "Inflated content ratio is larger than the " + "configured limit %i by %i time(s)", + dc->ratio_limit, dc->ratio_burst); + return APR_EINVAL; + } + if (zRC == Z_STREAM_END) { /* * We have inflated all data. Now try to capture the @@ -1826,21 +1842,15 @@ static apr_status_t inflate_out_filter(ap_filter_t *f, "Zlib: %d bytes of garbage at the end of " "compressed stream.", ctx->stream.avail_in - VALIDATION_SIZE); - } else if (ctx->stream.avail_in > 0) { - ctx->validation_buffer_length = ctx->stream.avail_in; + } + else if (ctx->stream.avail_in > 0) { + ctx->validation_buffer_length = ctx->stream.avail_in; } if (ctx->validation_buffer_length) memcpy(ctx->validation_buffer, ctx->stream.next_in, ctx->validation_buffer_length); break; } - - if (zRC != Z_OK) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01409) - "Zlib error %d inflating data (%s)", zRC, - ctx->stream.msg); - return APR_EGENERAL; - } } apr_bucket_delete(e); -- 2.47.2