From a006d29b8aa0d5b3ea03dafc7315bc7294cfb86c Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Thu, 20 Apr 2017 18:26:37 +0300 Subject: [PATCH] Cleaned up r->headers_out.headers allocation error handling. If initialization of a header failed for some reason after ngx_list_push(), leaving the header as is can result in uninitialized memory access by the header filter or the log module. The fix is to clear partially initialized headers in case of errors. For the Cache-Control header, the fix is to postpone pushing r->headers_out.cache_control until its value is completed. --- src/http/modules/ngx_http_auth_basic_module.c | 2 ++ src/http/modules/ngx_http_dav_module.c | 1 + .../modules/ngx_http_headers_filter_module.c | 21 ++++++++++--------- .../modules/ngx_http_range_filter_module.c | 4 ++++ src/http/modules/ngx_http_static_module.c | 1 + src/http/modules/perl/nginx.xs | 2 ++ src/http/ngx_http_core_module.c | 1 + src/http/ngx_http_upstream.c | 11 +++++----- 8 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/http/modules/ngx_http_auth_basic_module.c b/src/http/modules/ngx_http_auth_basic_module.c index 1e7a0c2df..4aa684f8a 100644 --- a/src/http/modules/ngx_http_auth_basic_module.c +++ b/src/http/modules/ngx_http_auth_basic_module.c @@ -361,6 +361,8 @@ ngx_http_auth_basic_set_realm(ngx_http_request_t *r, ngx_str_t *realm) basic = ngx_pnalloc(r->pool, len); if (basic == NULL) { + r->headers_out.www_authenticate->hash = 0; + r->headers_out.www_authenticate = NULL; return NGX_HTTP_INTERNAL_SERVER_ERROR; } diff --git a/src/http/modules/ngx_http_dav_module.c b/src/http/modules/ngx_http_dav_module.c index 895a52dca..566b08ba1 100644 --- a/src/http/modules/ngx_http_dav_module.c +++ b/src/http/modules/ngx_http_dav_module.c @@ -1080,6 +1080,7 @@ ngx_http_dav_location(ngx_http_request_t *r, u_char *path) } else { location = ngx_pnalloc(r->pool, r->uri.len); if (location == NULL) { + ngx_http_clear_location(r); return NGX_ERROR; } diff --git a/src/http/modules/ngx_http_headers_filter_module.c b/src/http/modules/ngx_http_headers_filter_module.c index b5aac33a7..94dc51e77 100644 --- a/src/http/modules/ngx_http_headers_filter_module.c +++ b/src/http/modules/ngx_http_headers_filter_module.c @@ -271,11 +271,6 @@ ngx_http_set_expires(ngx_http_request_t *r, ngx_http_headers_conf_t *conf) return NGX_ERROR; } - ccp = ngx_array_push(&r->headers_out.cache_control); - if (ccp == NULL) { - return NGX_ERROR; - } - cc = ngx_list_push(&r->headers_out.headers); if (cc == NULL) { return NGX_ERROR; @@ -283,6 +278,12 @@ ngx_http_set_expires(ngx_http_request_t *r, ngx_http_headers_conf_t *conf) cc->hash = 1; ngx_str_set(&cc->key, "Cache-Control"); + + ccp = ngx_array_push(&r->headers_out.cache_control); + if (ccp == NULL) { + return NGX_ERROR; + } + *ccp = cc; } else { @@ -470,11 +471,6 @@ ngx_http_add_cache_control(ngx_http_request_t *r, ngx_http_header_val_t *hv, } } - ccp = ngx_array_push(&r->headers_out.cache_control); - if (ccp == NULL) { - return NGX_ERROR; - } - cc = ngx_list_push(&r->headers_out.headers); if (cc == NULL) { return NGX_ERROR; @@ -484,6 +480,11 @@ ngx_http_add_cache_control(ngx_http_request_t *r, ngx_http_header_val_t *hv, ngx_str_set(&cc->key, "Cache-Control"); cc->value = *value; + ccp = ngx_array_push(&r->headers_out.cache_control); + if (ccp == NULL) { + return NGX_ERROR; + } + *ccp = cc; return NGX_OK; diff --git a/src/http/modules/ngx_http_range_filter_module.c b/src/http/modules/ngx_http_range_filter_module.c index 951a00de1..8ffca82e6 100644 --- a/src/http/modules/ngx_http_range_filter_module.c +++ b/src/http/modules/ngx_http_range_filter_module.c @@ -425,6 +425,8 @@ ngx_http_range_singlepart_header(ngx_http_request_t *r, content_range->value.data = ngx_pnalloc(r->pool, sizeof("bytes -/") - 1 + 3 * NGX_OFF_T_LEN); if (content_range->value.data == NULL) { + content_range->hash = 0; + r->headers_out.content_range = NULL; return NGX_ERROR; } @@ -594,6 +596,8 @@ ngx_http_range_not_satisfiable(ngx_http_request_t *r) content_range->value.data = ngx_pnalloc(r->pool, sizeof("bytes */") - 1 + NGX_OFF_T_LEN); if (content_range->value.data == NULL) { + content_range->hash = 0; + r->headers_out.content_range = NULL; return NGX_ERROR; } diff --git a/src/http/modules/ngx_http_static_module.c b/src/http/modules/ngx_http_static_module.c index b0513be1c..0e16c05a5 100644 --- a/src/http/modules/ngx_http_static_module.c +++ b/src/http/modules/ngx_http_static_module.c @@ -169,6 +169,7 @@ ngx_http_static_handler(ngx_http_request_t *r) location = ngx_pnalloc(r->pool, len); if (location == NULL) { + ngx_http_clear_location(r); return NGX_HTTP_INTERNAL_SERVER_ERROR; } diff --git a/src/http/modules/perl/nginx.xs b/src/http/modules/perl/nginx.xs index cca64da37..ad1263287 100644 --- a/src/http/modules/perl/nginx.xs +++ b/src/http/modules/perl/nginx.xs @@ -510,10 +510,12 @@ header_out(r, key, value) header->hash = 1; if (ngx_http_perl_sv2str(aTHX_ r, &header->key, key) != NGX_OK) { + header->hash = 0; XSRETURN_EMPTY; } if (ngx_http_perl_sv2str(aTHX_ r, &header->value, value) != NGX_OK) { + header->hash = 0; XSRETURN_EMPTY; } diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c index f357ee4b6..af67b7f85 100644 --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -1002,6 +1002,7 @@ ngx_http_core_find_config_phase(ngx_http_request_t *r, p = ngx_pnalloc(r->pool, len); if (p == NULL) { + ngx_http_clear_location(r); ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); return NGX_OK; } diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c index 36952860c..fcfa2ad94 100644 --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -4897,17 +4897,18 @@ ngx_http_upstream_copy_multi_header_lines(ngx_http_request_t *r, } } - ph = ngx_array_push(pa); - if (ph == NULL) { - return NGX_ERROR; - } - ho = ngx_list_push(&r->headers_out.headers); if (ho == NULL) { return NGX_ERROR; } *ho = *h; + + ph = ngx_array_push(pa); + if (ph == NULL) { + return NGX_ERROR; + } + *ph = ho; return NGX_OK;