From 702ebbc19b6c1c7a95e06ba1abd8a62cc48e6907 Mon Sep 17 00:00:00 2001
From: Adam <Adam@sigterm.info>
Date: Sun, 20 Jul 2014 05:24:18 -0400
Subject: [PATCH] Don't spin from malformed dns packets containing loops

Motivation:

DNS packets that contain pointers in a loop will cause
DnsResponseDecoder.readName() to infinite loop.

Modifications:

Fixed DnsResponseDecoder.readName() to detect when packets have loops
and return null instead.

Result:

It is no longer possible to cause Netty to infinite loop by sending it malformed
DNS packets with a loop.
---
 .../handler/codec/dns/DnsResponseDecoder.java   |  7 +++++++
 .../handler/codec/dns/DnsResponseTest.java      | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/codec-dns/src/main/java/io/netty/handler/codec/dns/DnsResponseDecoder.java b/codec-dns/src/main/java/io/netty/handler/codec/dns/DnsResponseDecoder.java
index b8f613b52d..ea89379534 100644
--- a/codec-dns/src/main/java/io/netty/handler/codec/dns/DnsResponseDecoder.java
+++ b/codec-dns/src/main/java/io/netty/handler/codec/dns/DnsResponseDecoder.java
@@ -86,6 +86,8 @@ public class DnsResponseDecoder extends MessageToMessageDecoder<DatagramPacket>
      */
     private static String readName(ByteBuf buf) {
         int position = -1;
+        int checked = 0;
+        int length = buf.writerIndex();
         StringBuilder name = new StringBuilder();
         for (int len = buf.readUnsignedByte(); buf.isReadable() && len != 0; len = buf.readUnsignedByte()) {
             boolean pointer = (len & 0xc0) == 0xc0;
@@ -94,6 +96,11 @@ public class DnsResponseDecoder extends MessageToMessageDecoder<DatagramPacket>
                     position = buf.readerIndex() + 1;
                 }
                 buf.readerIndex((len & 0x3f) << 8 | buf.readUnsignedByte());
+                // check for loops
+                checked += 2;
+                if (checked >= length) {
+                    return null;
+                }
             } else {
                 name.append(buf.toString(buf.readerIndex(), len, CharsetUtil.UTF_8)).append('.');
                 buf.skipBytes(len);
diff --git a/codec-dns/src/test/java/io/netty/handler/codec/dns/DnsResponseTest.java b/codec-dns/src/test/java/io/netty/handler/codec/dns/DnsResponseTest.java
index 28efbe5063..45895c17a1 100644
--- a/codec-dns/src/test/java/io/netty/handler/codec/dns/DnsResponseTest.java
+++ b/codec-dns/src/test/java/io/netty/handler/codec/dns/DnsResponseTest.java
@@ -19,9 +19,12 @@ import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.embedded.EmbeddedChannel;
 import io.netty.channel.socket.DatagramPacket;
+import io.netty.handler.codec.DecoderException;
 
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import java.net.InetSocketAddress;
 
@@ -41,6 +44,8 @@ public class DnsResponseTest {
                     111, 109, 0, 0, 16, 0, 1, -64, 12, 0, 16, 0, 1, 0, 0, 84, 75, 0, 12, 11, 118, 61, 115, 112, 102,
                     49, 32, 45, 97, 108, 108 } };
 
+    private static final byte[] malformedLoopPacket = { 0, 4, -127, -128, 0, 1, 0, 0, 0, 0, 0, 0, -64, 12, 0, 1, 0, 1 };
+
     @Test
     public void readResponseTest() throws Exception {
         EmbeddedChannel embedder = new EmbeddedChannel(new DnsResponseDecoder());
@@ -65,4 +70,16 @@ public class DnsResponseTest {
             decoded.release();
         }
     }
+
+    @Rule
+    public ExpectedException exception = ExpectedException.none();
+
+    @Test
+    public void readMalormedResponseTest() throws Exception {
+        EmbeddedChannel embedder = new EmbeddedChannel(new DnsResponseDecoder());
+        ByteBuf packet = embedder.alloc().buffer(512).writeBytes(malformedLoopPacket);
+        exception.expect(DecoderException.class);
+        exception.expectMessage("java.lang.NullPointerException: name");
+        embedder.writeInbound(new DatagramPacket(packet, null, new InetSocketAddress(0)));
+    }
 }