From 7570718594d846787ca31914e0f8431d7a4cfad1 Mon Sep 17 00:00:00 2001
From: Roman Arutyunyan <arut@nginx.com>
Date: Tue, 4 Jul 2017 18:07:29 +0300
Subject: [PATCH] Resolver: fixed possible use-after-free while resolving SRV.

Resolving an SRV record includes resolving its host names in subrequests.
Previously, if memory allocation failed while reporting a subrequest result
after receiving a response from a DNS server, the SRV resolve handler was
called immediately with the NGX_ERROR state.  However, if the SRV record
included another copy of the resolved name, it was reported once again.
This could trigger the use-after-free memory access after SRV resolve
handler freed the resolve context by calling ngx_resolve_name_done().

Now the SRV resolve handler is called only when all its subrequests are
completed.
---
 src/core/ngx_resolver.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c
index 91f8a5ef3..91231f232 100644
--- a/src/core/ngx_resolver.c
+++ b/src/core/ngx_resolver.c
@@ -3034,25 +3034,15 @@ ngx_resolver_srv_names_handler(ngx_resolver_ctx_t *cctx)
 
         addrs = ngx_resolver_calloc(r, cctx->naddrs * sizeof(ngx_addr_t));
         if (addrs == NULL) {
-            ngx_resolve_name_done(cctx);
-
-            ctx->state = NGX_ERROR;
-            ctx->valid = ngx_time() + (r->valid ? r->valid : 10);
-
-            ctx->handler(ctx);
-            return;
+            srv->state = NGX_ERROR;
+            goto done;
         }
 
         sockaddr = ngx_resolver_alloc(r, cctx->naddrs * sizeof(ngx_sockaddr_t));
         if (sockaddr == NULL) {
             ngx_resolver_free(r, addrs);
-            ngx_resolve_name_done(cctx);
-
-            ctx->state = NGX_ERROR;
-            ctx->valid = ngx_time() + (r->valid ? r->valid : 10);
-
-            ctx->handler(ctx);
-            return;
+            srv->state = NGX_ERROR;
+            goto done;
         }
 
         for (i = 0; i < cctx->naddrs; i++) {
@@ -3069,6 +3059,8 @@ ngx_resolver_srv_names_handler(ngx_resolver_ctx_t *cctx)
         srv->naddrs = cctx->naddrs;
     }
 
+done:
+
     ngx_resolve_name_done(cctx);
 
     if (ctx->count == 0) {
@@ -4254,10 +4246,21 @@ ngx_resolver_report_srv(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx)
     ngx_resolver_addr_t      *addrs;
     ngx_resolver_srv_name_t  *srvs;
 
+    srvs = ctx->srvs;
+    nsrvs = ctx->nsrvs;
+
     naddrs = 0;
 
-    for (i = 0; i < ctx->nsrvs; i++) {
-        naddrs += ctx->srvs[i].naddrs;
+    for (i = 0; i < nsrvs; i++) {
+        if (srvs[i].state == NGX_ERROR) {
+            ctx->state = NGX_ERROR;
+            ctx->valid = ngx_time() + (r->valid ? r->valid : 10);
+
+            ctx->handler(ctx);
+            return;
+        }
+
+        naddrs += srvs[i].naddrs;
     }
 
     if (naddrs == 0) {
@@ -4277,9 +4280,6 @@ ngx_resolver_report_srv(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx)
         return;
     }
 
-    srvs = ctx->srvs;
-    nsrvs = ctx->nsrvs;
-
     i = 0;
     n = 0;