From 0bd87716976f36cfa35d8ddbbb7bc0d700b402e6 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Thu, 4 Jun 2020 17:56:59 +0200
Subject: [PATCH] =?UTF-8?q?Fix=20possible=20StackOverflowError=20when=20tr?=
 =?UTF-8?q?y=20to=20resolve=20authorative=20names=E2=80=A6=20(#10337)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Motivation:

There is a possibility to end up with a StackOverflowError when trying to resolve authorative nameservers because of incorrect wrapping the AuthoritativeDnsServerCache.

Modifications:

Ensure we don't end up with an overflow due wrapping

Result:

Fixes https://github.com/netty/netty/issues/10246
---
 .../netty/resolver/dns/DnsResolveContext.java | 65 ++++++++++++-------
 1 file changed, 42 insertions(+), 23 deletions(-)

diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java
index c5cb784a70..78c9514ced 100644
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java
@@ -498,34 +498,53 @@ abstract class DnsResolveContext<T> {
                 }
             }
         });
-        if (!DnsNameResolver.doResolveAllCached(nameServerName, additionals, resolverPromise, resolveCache(),
+        DnsCache resolveCache = resolveCache();
+        if (!DnsNameResolver.doResolveAllCached(nameServerName, additionals, resolverPromise, resolveCache,
                 parent.resolvedInternetProtocolFamiliesUnsafe())) {
-            final AuthoritativeDnsServerCache authoritativeDnsServerCache = authoritativeDnsServerCache();
             new DnsAddressResolveContext(parent, originalPromise, nameServerName, additionals,
-                                         parent.newNameServerAddressStream(nameServerName),
-                                         resolveCache(), new AuthoritativeDnsServerCache() {
-                @Override
-                public DnsServerAddressStream get(String hostname) {
-                    // To not risk falling into any loop, we will not use the cache while following redirects but only
-                    // on the initial query.
-                    return null;
-                }
+                                         parent.newNameServerAddressStream(nameServerName), resolveCache,
+                                         redirectAuthoritativeDnsServerCache(authoritativeDnsServerCache()), false)
+                    .resolve(resolverPromise);
+        }
+    }
 
-                @Override
-                public void cache(String hostname, InetSocketAddress address, long originalTtl, EventLoop loop) {
-                    authoritativeDnsServerCache.cache(hostname, address, originalTtl, loop);
-                }
+    private static AuthoritativeDnsServerCache redirectAuthoritativeDnsServerCache(
+            AuthoritativeDnsServerCache authoritativeDnsServerCache) {
+        // Don't wrap again to prevent the possibility of an StackOverflowError when wrapping another
+        // RedirectAuthoritativeDnsServerCache.
+        if (authoritativeDnsServerCache instanceof RedirectAuthoritativeDnsServerCache) {
+            return authoritativeDnsServerCache;
+        }
+        return new RedirectAuthoritativeDnsServerCache(authoritativeDnsServerCache);
+    }
 
-                @Override
-                public void clear() {
-                    authoritativeDnsServerCache.clear();
-                }
+    private static final class RedirectAuthoritativeDnsServerCache implements AuthoritativeDnsServerCache {
+        private final AuthoritativeDnsServerCache wrapped;
 
-                @Override
-                public boolean clear(String hostname) {
-                    return authoritativeDnsServerCache.clear(hostname);
-                }
-            }, false).resolve(resolverPromise);
+        RedirectAuthoritativeDnsServerCache(AuthoritativeDnsServerCache authoritativeDnsServerCache) {
+            this.wrapped = authoritativeDnsServerCache;
+        }
+
+        @Override
+        public DnsServerAddressStream get(String hostname) {
+            // To not risk falling into any loop, we will not use the cache while following redirects but only
+            // on the initial query.
+            return null;
+        }
+
+        @Override
+        public void cache(String hostname, InetSocketAddress address, long originalTtl, EventLoop loop) {
+            wrapped.cache(hostname, address, originalTtl, loop);
+        }
+
+        @Override
+        public void clear() {
+            wrapped.clear();
+        }
+
+        @Override
+        public boolean clear(String hostname) {
+            return wrapped.clear(hostname);
         }
     }