[nginx] Resolver: fixed possible use-after-free while resolving SRV.

Roman Arutyunyan arut at nginx.com
Tue Jul 4 15:26:23 UTC 2017


details:   http://hg.nginx.org/nginx/rev/80224192163c
branches:  
changeset: 7048:80224192163c
user:      Roman Arutyunyan <arut at nginx.com>
date:      Tue Jul 04 18:07:29 2017 +0300
description:
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.

diffstat:

 src/core/ngx_resolver.c |  38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)

diffs (76 lines):

diff -r 3fef8c5caa75 -r 80224192163c src/core/ngx_resolver.c
--- a/src/core/ngx_resolver.c	Wed Mar 15 15:55:35 2017 -0700
+++ b/src/core/ngx_resolver.c	Tue Jul 04 18:07:29 2017 +0300
@@ -3034,25 +3034,15 @@ ngx_resolver_srv_names_handler(ngx_resol
 
         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_resol
         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 *
     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 *
         return;
     }
 
-    srvs = ctx->srvs;
-    nsrvs = ctx->nsrvs;
-
     i = 0;
     n = 0;
 


More information about the nginx-devel mailing list