From 3d82ee2c790a72bc60853d4b09e6bd0a3abc3469 Mon Sep 17 00:00:00 2001 From: Sergei Turchanov Date: Wed, 18 Apr 2018 23:27:33 +1000 Subject: [PATCH] Fix incorrect handling of request/response body Fix incorrect handling of request/response body data chain of ngx_buf_t buffers. The documentation [http://nginx.org/en/docs/dev/development_guide.html#buffer] clearly states that .pos, .last must be used to reference actual data contained by the buffer. Whereas .start, .end denote the boundaries of the memory block allocated for the buffer (in case of dynamically allocated data) or just NULL (when .pos, .last reference a static memory location - one can see that kind of usage in ngx_http_gzip_filter_module.c:ngx_http_gzip_filter_gzheader()). To back up my words I invite to examine ngx_http_charset_filter_module.c:ngx_http_charset_recode() as an example of iteration over data contained in data buffer. Without this fix ngx_http_modsecurity_body_filter feeds random bytes from memory pointed by .start, .end range to msc_append_response_body. In my case is was 8KB of data instead of 10 bytes when referenced by (.pos, .last). That is this vulnerability may disclose sensitive data like passwords or whatever from nginx heap. The fix for ngx_http_modsecurity_pre_access_handler is to use .pos not .start to reference data as they may differ in general case. --- src/ngx_http_modsecurity_body_filter.c | 4 ++-- src/ngx_http_modsecurity_pre_access.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index f8b3c71..edf44f6 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -150,9 +150,9 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) for (chain = in; chain != NULL; chain = chain->next) { - u_char *data = chain->buf->start; + u_char *data = chain->buf->pos; - msc_append_response_body(ctx->modsec_transaction, data, chain->buf->end - data); + msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data); ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { return ngx_http_filter_finalize_request(r, diff --git a/src/ngx_http_modsecurity_pre_access.c b/src/ngx_http_modsecurity_pre_access.c index 70f9feb..6f4cbcb 100644 --- a/src/ngx_http_modsecurity_pre_access.c +++ b/src/ngx_http_modsecurity_pre_access.c @@ -163,10 +163,10 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) while (chain && !already_inspected) { - u_char *data = chain->buf->start; + u_char *data = chain->buf->pos; msc_append_request_body(ctx->modsec_transaction, data, - chain->buf->last - chain->buf->pos); + chain->buf->last - data); if (chain->buf->last_buf) { break;