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.
This commit is contained in:
Adam 2014-07-20 05:24:18 -04:00 committed by Norman Maurer
parent b83df4c6b3
commit 702ebbc19b
2 changed files with 24 additions and 0 deletions

View File

@ -86,6 +86,8 @@ public class DnsResponseDecoder extends MessageToMessageDecoder<DatagramPacket>
*/ */
private static String readName(ByteBuf buf) { private static String readName(ByteBuf buf) {
int position = -1; int position = -1;
int checked = 0;
int length = buf.writerIndex();
StringBuilder name = new StringBuilder(); StringBuilder name = new StringBuilder();
for (int len = buf.readUnsignedByte(); buf.isReadable() && len != 0; len = buf.readUnsignedByte()) { for (int len = buf.readUnsignedByte(); buf.isReadable() && len != 0; len = buf.readUnsignedByte()) {
boolean pointer = (len & 0xc0) == 0xc0; boolean pointer = (len & 0xc0) == 0xc0;
@ -94,6 +96,11 @@ public class DnsResponseDecoder extends MessageToMessageDecoder<DatagramPacket>
position = buf.readerIndex() + 1; position = buf.readerIndex() + 1;
} }
buf.readerIndex((len & 0x3f) << 8 | buf.readUnsignedByte()); buf.readerIndex((len & 0x3f) << 8 | buf.readUnsignedByte());
// check for loops
checked += 2;
if (checked >= length) {
return null;
}
} else { } else {
name.append(buf.toString(buf.readerIndex(), len, CharsetUtil.UTF_8)).append('.'); name.append(buf.toString(buf.readerIndex(), len, CharsetUtil.UTF_8)).append('.');
buf.skipBytes(len); buf.skipBytes(len);

View File

@ -19,9 +19,12 @@ import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled; import io.netty.buffer.Unpooled;
import io.netty.channel.embedded.EmbeddedChannel; import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.channel.socket.DatagramPacket; import io.netty.channel.socket.DatagramPacket;
import io.netty.handler.codec.DecoderException;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException;
import java.net.InetSocketAddress; 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, 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 } }; 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 @Test
public void readResponseTest() throws Exception { public void readResponseTest() throws Exception {
EmbeddedChannel embedder = new EmbeddedChannel(new DnsResponseDecoder()); EmbeddedChannel embedder = new EmbeddedChannel(new DnsResponseDecoder());
@ -65,4 +70,16 @@ public class DnsResponseTest {
decoded.release(); 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)));
}
} }