From 04dd4497718279c1a64d70bdfcc048cb222cc367 Mon Sep 17 00:00:00 2001 From: Christos Trochalakis Date: Wed, 27 Jan 2016 12:17:44 +0200 Subject: [PATCH 01/10] Fix multiple resolver CVEs (Closes #812806) Backported all relevant commits from the 1.8.1 series. --- debian/changelog | 9 + ...ossible-segmentation-fault-on-DNS-fo.patch | 21 ++ ...ver-fixed-crashes-in-timeout-handler.patch | 128 +++++++++ ...NAME-processing-for-several-requests.patch | 78 ++++++ ...-the-ngx_resolver_create_-_query-arg.patch | 179 +++++++++++++ ...se-after-free-memory-accesses-with-C.patch | 248 ++++++++++++++++++ ...008-Resolver-limited-CNAME-recursion.patch | 60 +++++ debian/patches/series | 6 + 8 files changed, 729 insertions(+) create mode 100644 debian/patches/0003-Resolver-fixed-possible-segmentation-fault-on-DNS-fo.patch create mode 100644 debian/patches/0004-Resolver-fixed-crashes-in-timeout-handler.patch create mode 100644 debian/patches/0005-Resolver-fixed-CNAME-processing-for-several-requests.patch create mode 100644 debian/patches/0006-Resolver-changed-the-ngx_resolver_create_-_query-arg.patch create mode 100644 debian/patches/0007-Resolver-fixed-use-after-free-memory-accesses-with-C.patch create mode 100644 debian/patches/0008-Resolver-limited-CNAME-recursion.patch diff --git a/debian/changelog b/debian/changelog index be7399f..62e28fb 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +nginx (1.6.2-5+deb8u1) UNRELEASED; urgency=medium + + [ Christos Trochalakis ] + * Fixes multiple resolver CVEs, + CVE-2016-0742, CVE-2016-0746, CVE-2016-0747 + Closes: #812806 + + -- Christos Trochalakis Wed, 27 Jan 2016 12:19:54 +0200 + nginx (1.6.2-5) unstable; urgency=medium [ Christos Trochalakis ] diff --git a/debian/patches/0003-Resolver-fixed-possible-segmentation-fault-on-DNS-fo.patch b/debian/patches/0003-Resolver-fixed-possible-segmentation-fault-on-DNS-fo.patch new file mode 100644 index 0000000..976e0ba --- /dev/null +++ b/debian/patches/0003-Resolver-fixed-possible-segmentation-fault-on-DNS-fo.patch @@ -0,0 +1,21 @@ +From: Roman Arutyunyan +Date: Tue, 26 Jan 2016 16:46:18 +0300 +Subject: Resolver: fixed possible segmentation fault on DNS format error. + +--- + src/core/ngx_resolver.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c +index 5a944fc..726de9b 100644 +--- a/src/core/ngx_resolver.c ++++ b/src/core/ngx_resolver.c +@@ -1287,7 +1287,7 @@ ngx_resolver_process_response(ngx_resolver_t *r, u_char *buf, size_t n) + times = 0; + + for (q = ngx_queue_head(&r->name_resend_queue); +- q != ngx_queue_sentinel(&r->name_resend_queue) || times++ < 100; ++ q != ngx_queue_sentinel(&r->name_resend_queue) && times++ < 100; + q = ngx_queue_next(q)) + { + rn = ngx_queue_data(q, ngx_resolver_node_t, queue); diff --git a/debian/patches/0004-Resolver-fixed-crashes-in-timeout-handler.patch b/debian/patches/0004-Resolver-fixed-crashes-in-timeout-handler.patch new file mode 100644 index 0000000..8ec666a --- /dev/null +++ b/debian/patches/0004-Resolver-fixed-crashes-in-timeout-handler.patch @@ -0,0 +1,128 @@ +From: Ruslan Ermilov +Date: Tue, 26 Jan 2016 16:46:31 +0300 +Subject: Resolver: fixed crashes in timeout handler. + +If one or more requests were waiting for a response, then after +getting a CNAME response, the timeout event on the first request +remained active, pointing to the wrong node with an empty +rn->waiting list, and that could cause either null pointer +dereference or use-after-free memory access if this timeout +expired. + +If several requests were waiting for a response, and the first +request terminated (e.g., due to client closing a connection), +other requests were left without a timeout and could potentially +wait indefinitely. + +This is fixed by introducing per-request independent timeouts. +This change also reverts 954867a2f0a6 and 5004210e8c78. +--- + src/core/ngx_resolver.c | 50 +++++++++++++++++++++++++++++++++---------------- + 1 file changed, 34 insertions(+), 16 deletions(-) + +diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c +index 726de9b..8ebef6f 100644 +--- a/src/core/ngx_resolver.c ++++ b/src/core/ngx_resolver.c +@@ -417,7 +417,7 @@ ngx_resolve_name_done(ngx_resolver_ctx_t *ctx) + + /* lock name mutex */ + +- if (ctx->state == NGX_AGAIN) { ++ if (ctx->state == NGX_AGAIN || ctx->state == NGX_RESOLVE_TIMEDOUT) { + + hash = ngx_crc32_short(ctx->name.data, ctx->name.len); + +@@ -571,6 +571,20 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + + if (rn->waiting) { + ++ if (ctx->event == NULL) { ++ ctx->event = ngx_resolver_calloc(r, sizeof(ngx_event_t)); ++ if (ctx->event == NULL) { ++ return NGX_ERROR; ++ } ++ ++ ctx->event->handler = ngx_resolver_timeout_handler; ++ ctx->event->data = ctx; ++ ctx->event->log = r->log; ++ ctx->ident = -1; ++ ++ ngx_add_timer(ctx->event, ctx->timeout); ++ } ++ + ctx->next = rn->waiting; + rn->waiting = ctx; + ctx->state = NGX_AGAIN; +@@ -664,7 +678,7 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + } + + ctx->event->handler = ngx_resolver_timeout_handler; +- ctx->event->data = rn; ++ ctx->event->data = ctx; + ctx->event->log = r->log; + ctx->ident = -1; + +@@ -794,6 +808,18 @@ ngx_resolve_addr(ngx_resolver_ctx_t *ctx) + + if (rn->waiting) { + ++ ctx->event = ngx_resolver_calloc(r, sizeof(ngx_event_t)); ++ if (ctx->event == NULL) { ++ return NGX_ERROR; ++ } ++ ++ ctx->event->handler = ngx_resolver_timeout_handler; ++ ctx->event->data = ctx; ++ ctx->event->log = r->log; ++ ctx->ident = -1; ++ ++ ngx_add_timer(ctx->event, ctx->timeout); ++ + ctx->next = rn->waiting; + rn->waiting = ctx; + ctx->state = NGX_AGAIN; +@@ -857,7 +883,7 @@ ngx_resolve_addr(ngx_resolver_ctx_t *ctx) + } + + ctx->event->handler = ngx_resolver_timeout_handler; +- ctx->event->data = rn; ++ ctx->event->data = ctx; + ctx->event->log = r->log; + ctx->ident = -1; + +@@ -949,7 +975,7 @@ ngx_resolve_addr_done(ngx_resolver_ctx_t *ctx) + + /* lock addr mutex */ + +- if (ctx->state == NGX_AGAIN) { ++ if (ctx->state == NGX_AGAIN || ctx->state == NGX_RESOLVE_TIMEDOUT) { + + switch (ctx->addr.sockaddr->sa_family) { + +@@ -2791,21 +2817,13 @@ done: + static void + ngx_resolver_timeout_handler(ngx_event_t *ev) + { +- ngx_resolver_ctx_t *ctx, *next; +- ngx_resolver_node_t *rn; ++ ngx_resolver_ctx_t *ctx; + +- rn = ev->data; +- ctx = rn->waiting; +- rn->waiting = NULL; ++ ctx = ev->data; + +- do { +- ctx->state = NGX_RESOLVE_TIMEDOUT; +- next = ctx->next; +- +- ctx->handler(ctx); ++ ctx->state = NGX_RESOLVE_TIMEDOUT; + +- ctx = next; +- } while (ctx); ++ ctx->handler(ctx); + } + + diff --git a/debian/patches/0005-Resolver-fixed-CNAME-processing-for-several-requests.patch b/debian/patches/0005-Resolver-fixed-CNAME-processing-for-several-requests.patch new file mode 100644 index 0000000..d8759fe --- /dev/null +++ b/debian/patches/0005-Resolver-fixed-CNAME-processing-for-several-requests.patch @@ -0,0 +1,78 @@ +From: Ruslan Ermilov +Date: Tue, 26 Jan 2016 16:46:38 +0300 +Subject: Resolver: fixed CNAME processing for several requests. + +When several requests were waiting for a response, then after getting +a CNAME response only the last request was properly processed, while +others were left waiting. +--- + src/core/ngx_resolver.c | 21 +++++++++++++++------ + 1 file changed, 15 insertions(+), 6 deletions(-) + +diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c +index 8ebef6f..3c01584 100644 +--- a/src/core/ngx_resolver.c ++++ b/src/core/ngx_resolver.c +@@ -468,7 +468,7 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + ngx_int_t rc; + ngx_uint_t naddrs; + ngx_addr_t *addrs; +- ngx_resolver_ctx_t *next; ++ ngx_resolver_ctx_t *next, *last; + ngx_resolver_node_t *rn; + + ngx_strlow(ctx->name.data, ctx->name.data, ctx->name.len); +@@ -479,6 +479,9 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + + if (rn) { + ++ /* ctx can be a list after NGX_RESOLVE_CNAME */ ++ for (last = ctx; last->next; last = last->next); ++ + if (rn->valid >= ngx_time()) { + + ngx_log_debug0(NGX_LOG_DEBUG_CORE, r->log, 0, "resolve cached"); +@@ -506,7 +509,7 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + } + } + +- ctx->next = rn->waiting; ++ last->next = rn->waiting; + rn->waiting = NULL; + + /* unlock name mutex */ +@@ -552,7 +555,7 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + return ngx_resolve_name_locked(r, ctx); + } + +- ctx->next = rn->waiting; ++ last->next = rn->waiting; + rn->waiting = NULL; + + /* unlock name mutex */ +@@ -585,7 +588,7 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + ngx_add_timer(ctx->event, ctx->timeout); + } + +- ctx->next = rn->waiting; ++ last->next = rn->waiting; + rn->waiting = ctx; + ctx->state = NGX_AGAIN; + +@@ -656,8 +659,14 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + ngx_resolver_free(r, rn->name); + ngx_resolver_free(r, rn); + +- ctx->state = NGX_RESOLVE_NXDOMAIN; +- ctx->handler(ctx); ++ do { ++ ctx->state = NGX_RESOLVE_NXDOMAIN; ++ next = ctx->next; ++ ++ ctx->handler(ctx); ++ ++ ctx = next; ++ } while (ctx); + + return NGX_OK; + } diff --git a/debian/patches/0006-Resolver-changed-the-ngx_resolver_create_-_query-arg.patch b/debian/patches/0006-Resolver-changed-the-ngx_resolver_create_-_query-arg.patch new file mode 100644 index 0000000..4364ccf --- /dev/null +++ b/debian/patches/0006-Resolver-changed-the-ngx_resolver_create_-_query-arg.patch @@ -0,0 +1,179 @@ +From: Roman Arutyunyan +Date: Tue, 26 Jan 2016 16:46:48 +0300 +Subject: Resolver: changed the ngx_resolver_create_*_query() arguments. + +No functional changes. + +This is needed by the following change. +--- + src/core/ngx_resolver.c | 57 +++++++++++++++++++++++-------------------------- + 1 file changed, 27 insertions(+), 30 deletions(-) + +diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c +index 3c01584..1899564 100644 +--- a/src/core/ngx_resolver.c ++++ b/src/core/ngx_resolver.c +@@ -59,10 +59,10 @@ static void ngx_resolver_expire(ngx_resolver_t *r, ngx_rbtree_t *tree, + ngx_queue_t *queue); + static ngx_int_t ngx_resolver_send_query(ngx_resolver_t *r, + ngx_resolver_node_t *rn); +-static ngx_int_t ngx_resolver_create_name_query(ngx_resolver_node_t *rn, +- ngx_resolver_ctx_t *ctx); +-static ngx_int_t ngx_resolver_create_addr_query(ngx_resolver_node_t *rn, +- ngx_resolver_ctx_t *ctx); ++static ngx_int_t ngx_resolver_create_name_query(ngx_resolver_t *r, ++ ngx_resolver_node_t *rn, ngx_str_t *name); ++static ngx_int_t ngx_resolver_create_addr_query(ngx_resolver_t *r, ++ ngx_resolver_node_t *rn, ngx_addr_t *addr); + static void ngx_resolver_resend_handler(ngx_event_t *ev); + static time_t ngx_resolver_resend(ngx_resolver_t *r, ngx_rbtree_t *tree, + ngx_queue_t *queue); +@@ -646,7 +646,7 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + ngx_rbtree_insert(&r->name_rbtree, &rn->node); + } + +- rc = ngx_resolver_create_name_query(rn, ctx); ++ rc = ngx_resolver_create_name_query(r, rn, &ctx->name); + + if (rc == NGX_ERROR) { + goto failed; +@@ -873,7 +873,7 @@ ngx_resolve_addr(ngx_resolver_ctx_t *ctx) + ngx_rbtree_insert(tree, &rn->node); + } + +- if (ngx_resolver_create_addr_query(rn, ctx) != NGX_OK) { ++ if (ngx_resolver_create_addr_query(r, rn, &ctx->addr) != NGX_OK) { + goto failed; + } + +@@ -2506,27 +2506,23 @@ ngx_resolver_rbtree_insert_addr6_value(ngx_rbtree_node_t *temp, + + + static ngx_int_t +-ngx_resolver_create_name_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) ++ngx_resolver_create_name_query(ngx_resolver_t *r, ngx_resolver_node_t *rn, ++ ngx_str_t *name) + { + u_char *p, *s; + size_t len, nlen; + ngx_uint_t ident; +-#if (NGX_HAVE_INET6) +- ngx_resolver_t *r; +-#endif + ngx_resolver_qs_t *qs; + ngx_resolver_hdr_t *query; + +- nlen = ctx->name.len ? (1 + ctx->name.len + 1) : 1; ++ nlen = name->len ? (1 + name->len + 1) : 1; + + len = sizeof(ngx_resolver_hdr_t) + nlen + sizeof(ngx_resolver_qs_t); + + #if (NGX_HAVE_INET6) +- r = ctx->resolver; +- +- p = ngx_resolver_alloc(ctx->resolver, r->ipv6 ? len * 2 : len); ++ p = ngx_resolver_alloc(r, r->ipv6 ? len * 2 : len); + #else +- p = ngx_resolver_alloc(ctx->resolver, len); ++ p = ngx_resolver_alloc(r, len); + #endif + if (p == NULL) { + return NGX_ERROR; +@@ -2545,8 +2541,8 @@ ngx_resolver_create_name_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) + + ident = ngx_random(); + +- ngx_log_debug2(NGX_LOG_DEBUG_CORE, ctx->resolver->log, 0, +- "resolve: \"%V\" A %i", &ctx->name, ident & 0xffff); ++ ngx_log_debug2(NGX_LOG_DEBUG_CORE, r->log, 0, ++ "resolve: \"%V\" A %i", name, ident & 0xffff); + + query->ident_hi = (u_char) ((ident >> 8) & 0xff); + query->ident_lo = (u_char) (ident & 0xff); +@@ -2576,11 +2572,11 @@ ngx_resolver_create_name_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) + p--; + *p-- = '\0'; + +- if (ctx->name.len == 0) { ++ if (name->len == 0) { + return NGX_DECLINED; + } + +- for (s = ctx->name.data + ctx->name.len - 1; s >= ctx->name.data; s--) { ++ for (s = name->data + name->len - 1; s >= name->data; s--) { + if (*s != '.') { + *p = *s; + len++; +@@ -2616,8 +2612,8 @@ ngx_resolver_create_name_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) + + ident = ngx_random(); + +- ngx_log_debug2(NGX_LOG_DEBUG_CORE, ctx->resolver->log, 0, +- "resolve: \"%V\" AAAA %i", &ctx->name, ident & 0xffff); ++ ngx_log_debug2(NGX_LOG_DEBUG_CORE, r->log, 0, ++ "resolve: \"%V\" AAAA %i", name, ident & 0xffff); + + query->ident_hi = (u_char) ((ident >> 8) & 0xff); + query->ident_lo = (u_char) (ident & 0xff); +@@ -2634,11 +2630,12 @@ ngx_resolver_create_name_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) + + + static ngx_int_t +-ngx_resolver_create_addr_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) ++ngx_resolver_create_addr_query(ngx_resolver_t *r, ngx_resolver_node_t *rn, ++ ngx_addr_t *addr) + { + u_char *p, *d; + size_t len; +- in_addr_t addr; ++ in_addr_t inaddr; + ngx_int_t n; + ngx_uint_t ident; + ngx_resolver_hdr_t *query; +@@ -2647,7 +2644,7 @@ ngx_resolver_create_addr_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) + struct sockaddr_in6 *sin6; + #endif + +- switch (ctx->addr.sockaddr->sa_family) { ++ switch (addr->sockaddr->sa_family) { + + #if (NGX_HAVE_INET6) + case AF_INET6: +@@ -2664,7 +2661,7 @@ ngx_resolver_create_addr_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) + + sizeof(ngx_resolver_qs_t); + } + +- p = ngx_resolver_alloc(ctx->resolver, len); ++ p = ngx_resolver_alloc(r, len); + if (p == NULL) { + return NGX_ERROR; + } +@@ -2688,11 +2685,11 @@ ngx_resolver_create_addr_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) + + p += sizeof(ngx_resolver_hdr_t); + +- switch (ctx->addr.sockaddr->sa_family) { ++ switch (addr->sockaddr->sa_family) { + + #if (NGX_HAVE_INET6) + case AF_INET6: +- sin6 = (struct sockaddr_in6 *) ctx->addr.sockaddr; ++ sin6 = (struct sockaddr_in6 *) addr->sockaddr; + + for (n = 15; n >= 0; n--) { + p = ngx_sprintf(p, "\1%xd\1%xd", +@@ -2707,11 +2704,11 @@ ngx_resolver_create_addr_query(ngx_resolver_node_t *rn, ngx_resolver_ctx_t *ctx) + + default: /* AF_INET */ + +- sin = (struct sockaddr_in *) ctx->addr.sockaddr; +- addr = ntohl(sin->sin_addr.s_addr); ++ sin = (struct sockaddr_in *) addr->sockaddr; ++ inaddr = ntohl(sin->sin_addr.s_addr); + + for (n = 0; n < 32; n += 8) { +- d = ngx_sprintf(&p[1], "%ud", (addr >> n) & 0xff); ++ d = ngx_sprintf(&p[1], "%ud", (inaddr >> n) & 0xff); + *p = (u_char) (d - &p[1]); + p = d; + } diff --git a/debian/patches/0007-Resolver-fixed-use-after-free-memory-accesses-with-C.patch b/debian/patches/0007-Resolver-fixed-use-after-free-memory-accesses-with-C.patch new file mode 100644 index 0000000..c2f7b07 --- /dev/null +++ b/debian/patches/0007-Resolver-fixed-use-after-free-memory-accesses-with-C.patch @@ -0,0 +1,248 @@ +From: Roman Arutyunyan +Date: Tue, 26 Jan 2016 16:46:59 +0300 +Subject: Resolver: fixed use-after-free memory accesses with CNAME. + +When several requests were waiting for a response, then after getting +a CNAME response only the last request's context had the name updated. +Contexts of other requests had the wrong name. This name was used by +ngx_resolve_name_done() to find the node to remove the request context +from. When the name was wrong, the request could not be properly +cancelled, its context was freed but stayed linked to the node's waiting +list. This happened e.g. when the first request was aborted or timed +out before the resolving completed. When it completed, this triggered +a use-after-free memory access by calling ctx->handler of already freed +request context. The bug manifests itself by +"could not cancel resolving" alerts in error_log. + +When a request was responded with a CNAME, the request context kept +the pointer to the original node's rn->u.cname. If the original node +expired before the resolving timed out or completed with an error, +this would trigger a use-after-free memory access via ctx->name in +ctx->handler(). + +The fix is to keep ctx->name unmodified. The name from context +is no longer used by ngx_resolve_name_done(). Instead, we now keep +the pointer to resolver node to which this request is linked. +Keeping the original name intact also improves logging. +--- + src/core/ngx_resolver.c | 72 +++++++++++++++++++++++-------------------------- + src/core/ngx_resolver.h | 2 ++ + 2 files changed, 35 insertions(+), 39 deletions(-) + +diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c +index 1899564..2bd754c 100644 +--- a/src/core/ngx_resolver.c ++++ b/src/core/ngx_resolver.c +@@ -54,7 +54,7 @@ ngx_int_t ngx_udp_connect(ngx_udp_connection_t *uc); + static void ngx_resolver_cleanup(void *data); + static void ngx_resolver_cleanup_tree(ngx_resolver_t *r, ngx_rbtree_t *tree); + static ngx_int_t ngx_resolve_name_locked(ngx_resolver_t *r, +- ngx_resolver_ctx_t *ctx); ++ ngx_resolver_ctx_t *ctx, ngx_str_t *name); + static void ngx_resolver_expire(ngx_resolver_t *r, ngx_rbtree_t *tree, + ngx_queue_t *queue); + static ngx_int_t ngx_resolver_send_query(ngx_resolver_t *r, +@@ -370,7 +370,7 @@ ngx_resolve_name(ngx_resolver_ctx_t *ctx) + + /* lock name mutex */ + +- rc = ngx_resolve_name_locked(r, ctx); ++ rc = ngx_resolve_name_locked(r, ctx, &ctx->name); + + if (rc == NGX_OK) { + return NGX_OK; +@@ -397,7 +397,6 @@ ngx_resolve_name(ngx_resolver_ctx_t *ctx) + void + ngx_resolve_name_done(ngx_resolver_ctx_t *ctx) + { +- uint32_t hash; + ngx_resolver_t *r; + ngx_resolver_ctx_t *w, **p; + ngx_resolver_node_t *rn; +@@ -419,9 +418,7 @@ ngx_resolve_name_done(ngx_resolver_ctx_t *ctx) + + if (ctx->state == NGX_AGAIN || ctx->state == NGX_RESOLVE_TIMEDOUT) { + +- hash = ngx_crc32_short(ctx->name.data, ctx->name.len); +- +- rn = ngx_resolver_lookup_name(r, &ctx->name, hash); ++ rn = ctx->node; + + if (rn) { + p = &rn->waiting; +@@ -462,20 +459,22 @@ done: + + + static ngx_int_t +-ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) ++ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx, ++ ngx_str_t *name) + { + uint32_t hash; + ngx_int_t rc; ++ ngx_str_t cname; + ngx_uint_t naddrs; + ngx_addr_t *addrs; + ngx_resolver_ctx_t *next, *last; + ngx_resolver_node_t *rn; + +- ngx_strlow(ctx->name.data, ctx->name.data, ctx->name.len); ++ ngx_strlow(name->data, name->data, name->len); + +- hash = ngx_crc32_short(ctx->name.data, ctx->name.len); ++ hash = ngx_crc32_short(name->data, name->len); + +- rn = ngx_resolver_lookup_name(r, &ctx->name, hash); ++ rn = ngx_resolver_lookup_name(r, name, hash); + + if (rn) { + +@@ -549,10 +548,10 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + + if (ctx->recursion++ < NGX_RESOLVER_MAX_RECURSION) { + +- ctx->name.len = rn->cnlen; +- ctx->name.data = rn->u.cname; ++ cname.len = rn->cnlen; ++ cname.data = rn->u.cname; + +- return ngx_resolve_name_locked(r, ctx); ++ return ngx_resolve_name_locked(r, ctx, &cname); + } + + last->next = rn->waiting; +@@ -592,6 +591,11 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + rn->waiting = ctx; + ctx->state = NGX_AGAIN; + ++ do { ++ ctx->node = rn; ++ ctx = ctx->next; ++ } while (ctx); ++ + return NGX_AGAIN; + } + +@@ -630,14 +634,14 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + return NGX_ERROR; + } + +- rn->name = ngx_resolver_dup(r, ctx->name.data, ctx->name.len); ++ rn->name = ngx_resolver_dup(r, name->data, name->len); + if (rn->name == NULL) { + ngx_resolver_free(r, rn); + return NGX_ERROR; + } + + rn->node.key = hash; +- rn->nlen = (u_short) ctx->name.len; ++ rn->nlen = (u_short) name->len; + rn->query = NULL; + #if (NGX_HAVE_INET6) + rn->query6 = NULL; +@@ -646,7 +650,7 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + ngx_rbtree_insert(&r->name_rbtree, &rn->node); + } + +- rc = ngx_resolver_create_name_query(r, rn, &ctx->name); ++ rc = ngx_resolver_create_name_query(r, rn, name); + + if (rc == NGX_ERROR) { + goto failed; +@@ -710,6 +714,11 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx) + + ctx->state = NGX_AGAIN; + ++ do { ++ ctx->node = rn; ++ ctx = ctx->next; ++ } while (ctx); ++ + return NGX_AGAIN; + + failed: +@@ -832,6 +841,7 @@ ngx_resolve_addr(ngx_resolver_ctx_t *ctx) + ctx->next = rn->waiting; + rn->waiting = ctx; + ctx->state = NGX_AGAIN; ++ ctx->node = rn; + + /* unlock addr mutex */ + +@@ -917,6 +927,7 @@ ngx_resolve_addr(ngx_resolver_ctx_t *ctx) + /* unlock addr mutex */ + + ctx->state = NGX_AGAIN; ++ ctx->node = rn; + + return NGX_OK; + +@@ -947,17 +958,11 @@ failed: + void + ngx_resolve_addr_done(ngx_resolver_ctx_t *ctx) + { +- in_addr_t addr; + ngx_queue_t *expire_queue; + ngx_rbtree_t *tree; + ngx_resolver_t *r; + ngx_resolver_ctx_t *w, **p; +- struct sockaddr_in *sin; + ngx_resolver_node_t *rn; +-#if (NGX_HAVE_INET6) +- uint32_t hash; +- struct sockaddr_in6 *sin6; +-#endif + + r = ctx->resolver; + +@@ -986,21 +991,7 @@ ngx_resolve_addr_done(ngx_resolver_ctx_t *ctx) + + if (ctx->state == NGX_AGAIN || ctx->state == NGX_RESOLVE_TIMEDOUT) { + +- switch (ctx->addr.sockaddr->sa_family) { +- +-#if (NGX_HAVE_INET6) +- case AF_INET6: +- sin6 = (struct sockaddr_in6 *) ctx->addr.sockaddr; +- hash = ngx_crc32_short(sin6->sin6_addr.s6_addr, 16); +- rn = ngx_resolver_lookup_addr6(r, &sin6->sin6_addr, hash); +- break; +-#endif +- +- default: /* AF_INET */ +- sin = (struct sockaddr_in *) ctx->addr.sockaddr; +- addr = ntohl(sin->sin_addr.s_addr); +- rn = ngx_resolver_lookup_addr(r, addr); +- } ++ rn = ctx->node; + + if (rn) { + p = &rn->waiting; +@@ -1989,9 +1980,12 @@ ngx_resolver_process_a(ngx_resolver_t *r, u_char *buf, size_t last, + rn->waiting = NULL; + + if (ctx) { +- ctx->name = name; + +- (void) ngx_resolve_name_locked(r, ctx); ++ for (next = ctx; next; next = next->next) { ++ next->node = NULL; ++ } ++ ++ (void) ngx_resolve_name_locked(r, ctx, &name); + } + + ngx_resolver_free(r, rn->query); +diff --git a/src/core/ngx_resolver.h b/src/core/ngx_resolver.h +index 264c8c4..0186e22 100644 +--- a/src/core/ngx_resolver.h ++++ b/src/core/ngx_resolver.h +@@ -161,6 +161,8 @@ struct ngx_resolver_ctx_s { + ngx_uint_t quick; /* unsigned quick:1; */ + ngx_uint_t recursion; + ngx_event_t *event; ++ ++ ngx_resolver_node_t *node; + }; + + diff --git a/debian/patches/0008-Resolver-limited-CNAME-recursion.patch b/debian/patches/0008-Resolver-limited-CNAME-recursion.patch new file mode 100644 index 0000000..dbe95a2 --- /dev/null +++ b/debian/patches/0008-Resolver-limited-CNAME-recursion.patch @@ -0,0 +1,60 @@ +From: Ruslan Ermilov +Date: Tue, 26 Jan 2016 16:47:14 +0300 +Subject: Resolver: limited CNAME recursion. + +Previously, the recursion was only limited for cached responses. +--- + src/core/ngx_resolver.c | 28 ++++++++++++++++++++++------ + 1 file changed, 22 insertions(+), 6 deletions(-) + +diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c +index 2bd754c..25d643d 100644 +--- a/src/core/ngx_resolver.c ++++ b/src/core/ngx_resolver.c +@@ -1976,11 +1976,33 @@ ngx_resolver_process_a(ngx_resolver_t *r, u_char *buf, size_t last, + + ngx_queue_insert_head(&r->name_expire_queue, &rn->queue); + ++ ngx_resolver_free(r, rn->query); ++ rn->query = NULL; ++#if (NGX_HAVE_INET6) ++ rn->query6 = NULL; ++#endif ++ + ctx = rn->waiting; + rn->waiting = NULL; + + if (ctx) { + ++ if (ctx->recursion++ >= NGX_RESOLVER_MAX_RECURSION) { ++ ++ /* unlock name mutex */ ++ ++ do { ++ ctx->state = NGX_RESOLVE_NXDOMAIN; ++ next = ctx->next; ++ ++ ctx->handler(ctx); ++ ++ ctx = next; ++ } while (ctx); ++ ++ return; ++ } ++ + for (next = ctx; next; next = next->next) { + next->node = NULL; + } +@@ -1988,12 +2010,6 @@ ngx_resolver_process_a(ngx_resolver_t *r, u_char *buf, size_t last, + (void) ngx_resolve_name_locked(r, ctx, &name); + } + +- ngx_resolver_free(r, rn->query); +- rn->query = NULL; +-#if (NGX_HAVE_INET6) +- rn->query6 = NULL; +-#endif +- + /* unlock name mutex */ + + return; diff --git a/debian/patches/series b/debian/patches/series index 73f535e..93af595 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1 +1,7 @@ perl-use-dpkg-buildflags.patch +0003-Resolver-fixed-possible-segmentation-fault-on-DNS-fo.patch +0004-Resolver-fixed-crashes-in-timeout-handler.patch +0005-Resolver-fixed-CNAME-processing-for-several-requests.patch +0006-Resolver-changed-the-ngx_resolver_create_-_query-arg.patch +0007-Resolver-fixed-use-after-free-memory-accesses-with-C.patch +0008-Resolver-limited-CNAME-recursion.patch From 1079d2a2777e66b37552e99091e5be9888c526f7 Mon Sep 17 00:00:00 2001 From: Christos Trochalakis Date: Wed, 27 Jan 2016 12:22:12 +0200 Subject: [PATCH 02/10] Release 1.6.2-5+deb8u1 --- debian/changelog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/debian/changelog b/debian/changelog index 62e28fb..f1aaee0 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,11 +1,11 @@ -nginx (1.6.2-5+deb8u1) UNRELEASED; urgency=medium +nginx (1.6.2-5+deb8u1) jessie-security; urgency=high [ Christos Trochalakis ] * Fixes multiple resolver CVEs, CVE-2016-0742, CVE-2016-0746, CVE-2016-0747 Closes: #812806 - -- Christos Trochalakis Wed, 27 Jan 2016 12:19:54 +0200 + -- Christos Trochalakis Wed, 27 Jan 2016 12:22:00 +0200 nginx (1.6.2-5) unstable; urgency=medium From 6322b86a76ef2ec18d64b4c35bd3175981c16496 Mon Sep 17 00:00:00 2001 From: Christos Trochalakis Date: Tue, 31 May 2016 22:55:48 +0300 Subject: [PATCH 03/10] Release 1.6.2-5+deb8u2 --- debian/changelog | 9 +++++ ...pecial-buffers-on-writing-ticket-981.patch | 39 +++++++++++++++++++ debian/patches/series | 1 + 3 files changed, 49 insertions(+) create mode 100644 debian/patches/0009-Core-skip-special-buffers-on-writing-ticket-981.patch diff --git a/debian/changelog b/debian/changelog index f1aaee0..06ee64b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +nginx (1.6.2-5+deb8u2) jessie-security; urgency=high + + [ Christos Trochalakis ] + * Fixes CVE-2016-4450 + NULL pointer dereference while writing client request body. + (Closes: #825960) + + -- Christos Trochalakis Tue, 31 May 2016 22:55:34 +0300 + nginx (1.6.2-5+deb8u1) jessie-security; urgency=high [ Christos Trochalakis ] diff --git a/debian/patches/0009-Core-skip-special-buffers-on-writing-ticket-981.patch b/debian/patches/0009-Core-skip-special-buffers-on-writing-ticket-981.patch new file mode 100644 index 0000000..149dab0 --- /dev/null +++ b/debian/patches/0009-Core-skip-special-buffers-on-writing-ticket-981.patch @@ -0,0 +1,39 @@ +From: Maxim Dounin +Date: Tue, 31 May 2016 05:13:30 +0300 +Subject: Core: skip special buffers on writing (ticket #981). + +A special last buffer with cl->buf->pos set to NULL can be present in +a chain when writing request body if chunked encoding was used. This +resulted in a NULL pointer dereference if it happened to be the only +buffer left after a do...while loop iteration in ngx_write_chain_to_file(). + +The problem originally appeared in nginx 1.3.9 with chunked encoding +support. Additionally, rev. 3832b608dc8d (nginx 1.9.13) changed the +minimum number of buffers to trigger this from IOV_MAX (typically 1024) +to NGX_IOVS_PREALLOCATE (typically 64). + +Fix is to skip such buffers in ngx_chain_to_iovec(), much like it is +done in other places. + +Backported by 1.11.1 for debian. +--- + src/os/unix/ngx_files.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/src/os/unix/ngx_files.c b/src/os/unix/ngx_files.c +index c3ae47f..1970184 100644 +--- a/src/os/unix/ngx_files.c ++++ b/src/os/unix/ngx_files.c +@@ -183,6 +183,12 @@ ngx_write_chain_to_file(ngx_file_t *file, ngx_chain_t *cl, off_t offset, + /* create the iovec and coalesce the neighbouring bufs */ + + while (cl && vec.nelts < IOV_MAX) { ++ ++ if (ngx_buf_special(cl->buf)) { ++ cl = cl->next; ++ continue; ++ } ++ + if (prev == cl->buf->pos) { + iov->iov_len += cl->buf->last - cl->buf->pos; + diff --git a/debian/patches/series b/debian/patches/series index 93af595..f204855 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -5,3 +5,4 @@ perl-use-dpkg-buildflags.patch 0006-Resolver-changed-the-ngx_resolver_create_-_query-arg.patch 0007-Resolver-fixed-use-after-free-memory-accesses-with-C.patch 0008-Resolver-limited-CNAME-recursion.patch +0009-Core-skip-special-buffers-on-writing-ticket-981.patch From 560e74ac0577ce706c7d959c1c146c5cc017d125 Mon Sep 17 00:00:00 2001 From: Christos Trochalakis Date: Tue, 7 Jun 2016 10:46:33 +0300 Subject: [PATCH 04/10] Only build against liblua5.1-0-dev when libluajit is not available Conflicts: debian/changelog Backported from master for jessie --- debian/changelog | 9 +++++++++ debian/control | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/debian/changelog b/debian/changelog index 06ee64b..2d808db 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +nginx (1.6.2-5+deb8u3) UNRELEASED; urgency=high + + [ Christos Trochalakis ] + * debian/control: + Don't allow building against liblua5.1-0-dev on architectures + that libluajit is available. (Closes: #826167) + + -- Christos Trochalakis Tue, 07 Jun 2016 10:47:36 +0300 + nginx (1.6.2-5+deb8u2) jessie-security; urgency=high [ Christos Trochalakis ] diff --git a/debian/control b/debian/control index 36b663e..e356bc0 100644 --- a/debian/control +++ b/debian/control @@ -15,7 +15,8 @@ Build-Depends: autotools-dev, libexpat-dev, libgd2-dev | libgd2-noxpm-dev, libgeoip-dev, - libluajit-5.1-dev [i386 amd64 kfreebsd-i386 armel armhf powerpc powerpcspe mips mipsel] | liblua5.1-0-dev, + libluajit-5.1-dev [i386 amd64 kfreebsd-i386 armel armhf powerpc powerpcspe mips mipsel], + liblua5.1-0-dev [!i386 !amd64 !kfreebsd-i386 !armel !armhf !powerpc !powerpcspe !mips !mipsel], libmhash-dev, libpam0g-dev, libpcre3-dev, From 330e2b21e6c1ee6b6df20a9f2fb98aa868b05c2b Mon Sep 17 00:00:00 2001 From: Christos Trochalakis Date: Wed, 14 Sep 2016 12:23:49 +0300 Subject: [PATCH 05/10] Release 1.6.2-5+deb8u3 --- debian/changelog | 11 ++++++-- debian/control | 1 + debian/nginx-common.NEWS | 15 +++++++++++ debian/nginx-common.config | 36 +++++++++++++++++++++++++ debian/nginx-common.postinst | 35 ++++++++++++++++++------- debian/nginx-common.templates | 13 ++++++++++ debian/po/POTFILES.in | 1 + debian/po/templates.pot | 49 +++++++++++++++++++++++++++++++++++ 8 files changed, 149 insertions(+), 12 deletions(-) create mode 100755 debian/nginx-common.config create mode 100644 debian/nginx-common.templates create mode 100644 debian/po/POTFILES.in create mode 100644 debian/po/templates.pot diff --git a/debian/changelog b/debian/changelog index 2d808db..d49cdd8 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,11 +1,18 @@ -nginx (1.6.2-5+deb8u3) UNRELEASED; urgency=high +nginx (1.6.2-5+deb8u3) jessie-security; urgency=high [ Christos Trochalakis ] + * debian/nginx-common.postinst: + + CVE-2016-1247: Secure log file handling (owner & permissions) + against privilege escalation attacks. /var/log/nginx is now owned + by root:adm. Thanks ro Dawid Golunski for the report. + Changing /var/log/nginx permissions effectively reopens #701112, + since log files can be world-readable. This is a trade-off until + a better log opening solution is implemented upstream (trac:376). * debian/control: Don't allow building against liblua5.1-0-dev on architectures that libluajit is available. (Closes: #826167) - -- Christos Trochalakis Tue, 07 Jun 2016 10:47:36 +0300 + -- Christos Trochalakis Tue, 04 Oct 2016 15:20:33 +0300 nginx (1.6.2-5+deb8u2) jessie-security; urgency=high diff --git a/debian/control b/debian/control index e356bc0..13b7d9f 100644 --- a/debian/control +++ b/debian/control @@ -10,6 +10,7 @@ Uploaders: Jose Parrella , Christos Trochalakis Build-Depends: autotools-dev, debhelper (>= 9), + po-debconf, dh-systemd (>= 1.5), dpkg-dev (>= 1.15.5), libexpat-dev, diff --git a/debian/nginx-common.NEWS b/debian/nginx-common.NEWS index 8b44b63..ab24115 100644 --- a/debian/nginx-common.NEWS +++ b/debian/nginx-common.NEWS @@ -1,3 +1,18 @@ +nginx-common (1.6.2-5+deb8u3) jessie-security; urgency=high + + In order to secure nginx against privilege escalation attacks, we are + changing the way log file owners & permissions are handled so that www-data + is not allowed to symlink a logfile. /var/log/nginx is now owned by root:adm + and its permissions are changed to 0755. The package checks for such symlinks + on existing installations and informs the admin using debconf. + + That unfortunately may come at a cost in terms of privacy. /var/log/nginx is + now world-readable, and nginx hardcodes permissions of non-existing logs to + 0644. On systems running logrotate log files are private after the first + logrotate run, since the new log files are created with 0640 permissions. + + -- Christos Trochalakis Tue, 04 Oct 2016 15:20:33 +0300 + nginx-common (1.6.2-5) unstable; urgency=medium We have disabled SSLv3 in nginx.conf for security reasons (ref: POODLE), diff --git a/debian/nginx-common.config b/debian/nginx-common.config new file mode 100755 index 0000000..a5396cb --- /dev/null +++ b/debian/nginx-common.config @@ -0,0 +1,36 @@ +#!/bin/sh + +set -e + +. /usr/share/debconf/confmodule + +logdir="/var/log/nginx" + +log_symlinks_check() { + # Skip new installations + [ -z "$1" ] && return + + # Skip unaffected installations + dpkg --compare-versions "$1" lt-nl "1.6.2-5+deb8u3" || return + + # Check for unsecure symlinks + linked_logfiles="` find "$logdir" -type l -user www-data -name '*.log' `" + + # Skip if nothing is found + [ -z "$linked_logfiles" ] && return + + db_subst nginx/log-symlinks logfiles $linked_logfiles + db_input high nginx/log-symlinks || true + db_go || true +} + +case "$1" in + configure|reconfigure) + log_symlinks_check "$2" + ;; + *) + ;; +esac + +# vim: set ts=4 sts=4 sw=4 et sta ai : + diff --git a/debian/nginx-common.postinst b/debian/nginx-common.postinst index 351bdf9..2b176f8 100644 --- a/debian/nginx-common.postinst +++ b/debian/nginx-common.postinst @@ -1,6 +1,8 @@ #!/bin/sh set -e +. /usr/share/debconf/confmodule + # Handle naxsi removal dpkg-maintscript-helper rm_conffile \ /etc/nginx/naxsi.rules 1.6.2-2~ -- "$@" @@ -14,18 +16,31 @@ dpkg-maintscript-helper rm_conffile \ case "$1" in configure) logdir="/var/log/nginx" - # Ensure secure permissions (CVE-2013-0337) - # http://bugs.debian.org/701112 - # - # nginx uses 0755 for log files making them world readable, - # we fix that by using 0750 for the log directory. - # - # Allow local admin to override: - # e.g. dpkg-statoverride --add root adm 0755 /var/log/nginx + + # Allow local admin to override if ! dpkg-statoverride --list "$logdir" >/dev/null; then - chown www-data:adm $logdir - chmod 0750 $logdir + chown root:adm $logdir + chmod 0755 $logdir fi + + # Secure default logfiles on fresh installations + if [ -z "$2" ]; then + access_log="$logdir/access.log" + error_log="$logdir/error.log" + + if [ ! -e "$access_log" ]; then + touch "$access_log" + chmod 640 "$access_log" + chown www-data:adm "$access_log" + fi + + if [ ! -e "$error_log" ]; then + touch "$error_log" + chmod 640 "$error_log" + chown www-data:adm "$error_log" + fi + fi + # If a symlink doesn't exist and can be created, then create it. if [ -z $2 ] && [ ! -e /etc/nginx/sites-enabled/default ] && [ -d /etc/nginx/sites-enabled ] && [ -d /etc/nginx/sites-available ]; then diff --git a/debian/nginx-common.templates b/debian/nginx-common.templates new file mode 100644 index 0000000..225762c --- /dev/null +++ b/debian/nginx-common.templates @@ -0,0 +1,13 @@ +Template: nginx/log-symlinks +Type: note +_Description: Possible insecure nginx log files + The following log files under /var/log/nginx directory are symlinks + owned by www-data: + . + ${logfiles} + . + Since nginx 1.4.4-4 /var/log/nginx was owned by www-data. As a result + www-data could symlink log files to sensitive locations, which in turn + could lead to privilege escalation attacks. Although /var/log/nginx + permissions are now fixed it is possible that such insecure links + already exist. So, please make sure to check the above locations. diff --git a/debian/po/POTFILES.in b/debian/po/POTFILES.in new file mode 100644 index 0000000..dcbe15b --- /dev/null +++ b/debian/po/POTFILES.in @@ -0,0 +1 @@ +[type: gettext/rfc822deb] nginx-common.templates diff --git a/debian/po/templates.pot b/debian/po/templates.pot new file mode 100644 index 0000000..bb72383 --- /dev/null +++ b/debian/po/templates.pot @@ -0,0 +1,49 @@ +# Nginx debconf translations +# Copyright (C) 2016 Christos Trochalakis +# This file is distributed under the same license as the nginx package. +# Chrirtos Trochalakis , 2016. +# +#, fuzzy +msgid "" +msgstr "" +"Project-Id-Version: nginx\n" +"Report-Msgid-Bugs-To: nginx@packages.debian.org\n" +"POT-Creation-Date: 2016-10-04 20:03+0300\n" +"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" +"Last-Translator: FULL NAME \n" +"Language-Team: LANGUAGE \n" +"Language: \n" +"MIME-Version: 1.0\n" +"Content-Type: text/plain; charset=CHARSET\n" +"Content-Transfer-Encoding: 8bit\n" + +#. Type: note +#. Description +#: ../nginx-common.templates:1001 +msgid "Possible insecure nginx log files" +msgstr "" + +#. Type: note +#. Description +#: ../nginx-common.templates:1001 +msgid "" +"The following log files under /var/log/nginx directory are symlinks owned by " +"www-data:" +msgstr "" + +#. Type: note +#. Description +#: ../nginx-common.templates:1001 +msgid "${logfiles}" +msgstr "" + +#. Type: note +#. Description +#: ../nginx-common.templates:1001 +msgid "" +"Since nginx 1.4.4-4 /var/log/nginx was owned by www-data. As a result www-" +"data could symlink log files to sensitive locations, which in turn could " +"lead to privilege escalation attacks. Although /var/log/nginx permissions " +"are now fixed it is possible that such insecure links already exist. So, " +"please make sure to check the above locations." +msgstr "" From 9e18152111b9f4f9b04f99ca5c34067f559557a2 Mon Sep 17 00:00:00 2001 From: Christos Trochalakis Date: Fri, 28 Oct 2016 09:00:35 +0300 Subject: [PATCH 06/10] Import Debian patch 1.6.2-5+deb8u4 --- debian/changelog | 8 ++++++++ debian/nginx-common.config | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/debian/changelog b/debian/changelog index d49cdd8..b5390d1 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +nginx (1.6.2-5+deb8u4) jessie-security; urgency=high + + * Non-maintainer upload by the Security Team. + * debian/nginx-common.config: fix return code so script doesn't exit. + Thanks to Marc Deslauriers and Thomas Ward (Closes: #842276) + + -- Salvatore Bonaccorso Thu, 27 Oct 2016 20:22:42 +0200 + nginx (1.6.2-5+deb8u3) jessie-security; urgency=high [ Christos Trochalakis ] diff --git a/debian/nginx-common.config b/debian/nginx-common.config index a5396cb..fd4cc64 100755 --- a/debian/nginx-common.config +++ b/debian/nginx-common.config @@ -11,7 +11,7 @@ log_symlinks_check() { [ -z "$1" ] && return # Skip unaffected installations - dpkg --compare-versions "$1" lt-nl "1.6.2-5+deb8u3" || return + dpkg --compare-versions "$1" lt-nl "1.6.2-5+deb8u3" || return 0 # Check for unsecure symlinks linked_logfiles="` find "$logdir" -type l -user www-data -name '*.log' `" From 1a7d4be9aa91697e87f4b1ed0a483b8c729a1b56 Mon Sep 17 00:00:00 2001 From: Christos Trochalakis Date: Wed, 12 Jul 2017 08:38:05 +0300 Subject: [PATCH 07/10] Handle CVE-2017-7529 Integer overflow in the range filter A security issue was identified in nginx range filter. A specially crafted request might result in an integer overflow and incorrect processing of ranges, potentially resulting in sensitive information leak (CVE-2017-7529). When using nginx with standard modules this allows an attacker to obtain a cache file header if a response was returned from cache. In some configurations a cache file header may contain IP address of the backend server or other sensitive information. Besides, with 3rd party modules it is potentially possible that the issue may lead to a denial of service or a disclosure of a worker process memory. No such modules are currently known though. The issue affects nginx 0.5.6 - 1.13.2. The issue is fixed in nginx 1.13.3, 1.12.1. For older versions, the following configuration can be used as a temporary workaround: max_ranges 1; See http://mailman.nginx.org/pipermail/nginx-announce/2017/000200.html Closes: #868109 --- .../patches/CVE-2017-7529-Range-filter.patch | 46 +++++++++++++++++++ debian/patches/series | 1 + 2 files changed, 47 insertions(+) create mode 100644 debian/patches/CVE-2017-7529-Range-filter.patch diff --git a/debian/patches/CVE-2017-7529-Range-filter.patch b/debian/patches/CVE-2017-7529-Range-filter.patch new file mode 100644 index 0000000..0a34876 --- /dev/null +++ b/debian/patches/CVE-2017-7529-Range-filter.patch @@ -0,0 +1,46 @@ +From 455bd729517f770b2e70a5f51f27c713f2e3973e Mon Sep 17 00:00:00 2001 +From: Maxim Dounin +Date: Tue, 11 Jul 2017 16:06:23 +0300 +Subject: [PATCH] Range filter: protect from total size overflows. + +The overflow can be used to circumvent the restriction on total size of +ranges introduced in c2a91088b0c0 (1.1.2). Additionally, overflow +allows producing ranges with negative start (such ranges can be created +by using a suffix, "bytes=-100"; normally this results in 200 due to +the total size check). These can result in the following errors in logs: + +[crit] ... pread() ... failed (22: Invalid argument) +[alert] ... sendfile() failed (22: Invalid argument) + +When using cache, it can be also used to reveal cache file header. +It is believed that there are no other negative effects, at least with +standard nginx modules. + +In theory, this can also result in memory disclosure and/or segmentation +faults if multiple ranges are allowed, and the response is returned in a +single in-memory buffer. This never happens with standard nginx modules +though, as well as known 3rd party modules. + +Fix is to properly protect from possible overflow when incrementing size. +--- + src/http/modules/ngx_http_range_filter_module.c | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/src/http/modules/ngx_http_range_filter_module.c b/src/http/modules/ngx_http_range_filter_module.c +index 951a00de..59d273b7 100644 +--- a/src/http/modules/ngx_http_range_filter_module.c ++++ b/src/http/modules/ngx_http_range_filter_module.c +@@ -377,6 +377,10 @@ ngx_http_range_parse(ngx_http_request_t *r, ngx_http_range_filter_ctx_t *ctx, + range->start = start; + range->end = end; + ++ if (size > NGX_MAX_OFF_T_VALUE - (end - start)) { ++ return NGX_HTTP_RANGE_NOT_SATISFIABLE; ++ } ++ + size += end - start; + + if (ranges-- == 0) { +-- +2.13.2 + diff --git a/debian/patches/series b/debian/patches/series index f204855..2b14db2 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -6,3 +6,4 @@ perl-use-dpkg-buildflags.patch 0007-Resolver-fixed-use-after-free-memory-accesses-with-C.patch 0008-Resolver-limited-CNAME-recursion.patch 0009-Core-skip-special-buffers-on-writing-ticket-981.patch +CVE-2017-7529-Range-filter.patch From 3dfb47a00c5af5eb7e844826c8510714ee0e8879 Mon Sep 17 00:00:00 2001 From: Christos Trochalakis Date: Wed, 12 Jul 2017 08:46:19 +0300 Subject: [PATCH 08/10] Release 1.6.2-5+deb8u5 --- debian/changelog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/changelog b/debian/changelog index b5390d1..d588f5e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +nginx (1.6.2-5+deb8u5) jessie-security; urgency=high + + * Handle CVE-2017-7529 Integer overflow in the range filter (Closes: #868109) + + -- Christos Trochalakis Wed, 12 Jul 2017 10:29:22 +0300 + nginx (1.6.2-5+deb8u4) jessie-security; urgency=high * Non-maintainer upload by the Security Team. From c7d98a1a4c2bec188ef994304202fcb486a0472d Mon Sep 17 00:00:00 2001 From: Chris Lamb Date: Fri, 9 Nov 2018 10:11:27 +0100 Subject: [PATCH 09/10] Prevent a denial of service vulnerability due to an integer underflow whilst calculating an MP4 header sizes. (Closes: #913090 --- debian/patches/CVE-2018-16845.patch | 18 ++++++++++++++++++ debian/patches/series | 1 + 2 files changed, 19 insertions(+) create mode 100644 debian/patches/CVE-2018-16845.patch diff --git a/debian/patches/CVE-2018-16845.patch b/debian/patches/CVE-2018-16845.patch new file mode 100644 index 0000000..ec04435 --- /dev/null +++ b/debian/patches/CVE-2018-16845.patch @@ -0,0 +1,18 @@ +http://hg.nginx.org/nginx/rev/fdc19a3289c1 + +--- nginx-1.6.2.orig/src/http/modules/ngx_http_mp4_module.c ++++ nginx-1.6.2/src/http/modules/ngx_http_mp4_module.c +@@ -896,6 +896,13 @@ ngx_http_mp4_read_atom(ngx_http_mp4_file + atom_size = ngx_mp4_get_64value(atom_header + 8); + atom_header_size = sizeof(ngx_mp4_atom_header64_t); + ++ if (atom_size < sizeof(ngx_mp4_atom_header64_t)) { ++ ngx_log_error(NGX_LOG_ERR, mp4->file.log, 0, ++ "\"%s\" mp4 atom is too small:%uL", ++ mp4->file.name.data, atom_size); ++ return NGX_ERROR; ++ } ++ + } else { + ngx_log_error(NGX_LOG_ERR, mp4->file.log, 0, + "\"%s\" mp4 atom is too small:%uL", diff --git a/debian/patches/series b/debian/patches/series index 2b14db2..97aca61 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -7,3 +7,4 @@ perl-use-dpkg-buildflags.patch 0008-Resolver-limited-CNAME-recursion.patch 0009-Core-skip-special-buffers-on-writing-ticket-981.patch CVE-2017-7529-Range-filter.patch +CVE-2018-16845.patch From e7ccf43ce6079fd7148dc3e3b049ee083ad77649 Mon Sep 17 00:00:00 2001 From: Chris Lamb Date: Fri, 9 Nov 2018 10:11:52 +0100 Subject: [PATCH 10/10] Release 1.6.2-5+deb8u6. --- debian/changelog | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/debian/changelog b/debian/changelog index d588f5e..d94cd14 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,14 @@ +nginx (1.6.2-5+deb8u6) jessie-security; urgency=high + + * CVE-2018-16845: Prevent a denial of service vulnerability due to an integer + underflow whilst calculating an MP4 header sizes. Previously, there was no + validation for the size of a 64-bit atom in an MP4 file. This could lead to + a CPU hog when the size is 0 or various other problems due to integer + underflow when the calculating atom data size, including segmentation + faults or even worker-process memory disclosure. (Closes: #913090) + + -- Chris Lamb Thu, 08 Nov 2018 18:32:42 +0100 + nginx (1.6.2-5+deb8u5) jessie-security; urgency=high * Handle CVE-2017-7529 Integer overflow in the range filter (Closes: #868109)