From 6bb107af1e50e89f1b4559de8efeef705ce4bc13 Mon Sep 17 00:00:00 2001
From: mikbergl <mikael.berglund@tele2.com>
Date: Thu, 26 Jan 2023 16:36:50 +0100
Subject: [PATCH] Fix NPE in SimpleResolver

When reading a response that is REFUSED with no more data. The code could crash while comparing the DNS query with the response.

This change skips the comparison if the response is denied.
---
 .../java/org/xbill/DNS/SimpleResolver.java    |  6 ++
 .../xbill/DNS/SimpleResolverDeniedTest.java   | 61 +++++++++++++++++++
 2 files changed, 67 insertions(+)
 create mode 100644 src/test/java/org/xbill/DNS/SimpleResolverDeniedTest.java

diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java
index 4a564bb1..e3406c26 100644
--- a/src/main/java/org/xbill/DNS/SimpleResolver.java
+++ b/src/main/java/org/xbill/DNS/SimpleResolver.java
@@ -402,6 +402,12 @@ CompletableFuture<Message> sendAsync(Message query, boolean forceTcp, Executor e
             return f;
           }
 
+          if (response.getQuestion() == null) {
+            f.completeExceptionally(
+                new WireParseException("invalid message: question section missing"));
+            return f;
+          }
+
           // validate name, class and type (rfc5452#section-9.1)
           if (!query.getQuestion().getName().equals(response.getQuestion().getName())) {
             f.completeExceptionally(
diff --git a/src/test/java/org/xbill/DNS/SimpleResolverDeniedTest.java b/src/test/java/org/xbill/DNS/SimpleResolverDeniedTest.java
new file mode 100644
index 00000000..c0f7fdd9
--- /dev/null
+++ b/src/test/java/org/xbill/DNS/SimpleResolverDeniedTest.java
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: BSD-3-Clause
+package org.xbill.DNS;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.time.Duration;
+import java.util.concurrent.CompletableFuture;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+class SimpleResolverDeniedTest {
+
+  @Test
+  void emptyResponseShouldThrowWireParseException() throws IOException {
+
+    Name zone = Name.fromString("example.");
+    Message query = Message.newUpdate(zone);
+    Record record =
+        new CNAMERecord(Name.fromString("www", zone), DClass.IN, 300, Name.fromString("example."));
+    query.addRecord(record, Section.UPDATE);
+
+    try (MockedStatic<NioUdpClient> udpClient = Mockito.mockStatic(NioUdpClient.class)) {
+      udpClient
+          .when(
+              () ->
+                  NioUdpClient.sendrecv(
+                      any(),
+                      any(InetSocketAddress.class),
+                      any(byte[].class),
+                      anyInt(),
+                      any(Duration.class)))
+          .thenAnswer(
+              a -> {
+                Message qparsed = new Message(a.getArgument(2, byte[].class));
+
+                int id = qparsed.getHeader().getID();
+                Message response = new Message(id);
+                response.getHeader().setRcode(Rcode.REFUSED);
+                byte[] rbytes = response.toWire(Message.MAXLENGTH);
+
+                // This was the exact format returned by denying server
+                assertArrayEquals(
+                    rbytes,
+                    new byte[] {(byte) (id >>> 8), (byte) id, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0});
+
+                CompletableFuture<byte[]> f = new CompletableFuture<>();
+                f.complete(rbytes);
+                return f;
+              });
+
+      SimpleResolver simpleResolver = new SimpleResolver("127.0.0.1");
+
+      assertThrows(WireParseException.class, () -> simpleResolver.send(query));
+    }
+  }
+}