Skip to content

Commit 01df3bc

Browse files
slandelleslandelle
committed
Fix incorrect name encoding/decoding in DNS records, backport netty/netty#5064 
1 parent 528f103 commit 01df3bc

File tree

4 files changed

+104
-26
lines changed

4 files changed

+104
-26
lines changed

‎netty-bp/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
*/
2828
public class DefaultDnsRecordDecoder implements DnsRecordDecoder {
2929

30+
static final String ROOT = ".";
31+
3032
/**
3133
* Creates a new instance.
3234
*/
@@ -117,16 +119,22 @@ protected String decodeName(ByteBuf in) {
117119
// - https://github.com/netty/netty/issues/5014
118120
// - https://www.ietf.org/rfc/rfc1035.txt , Section 3.1
119121
if (readable == 0) {
120-
return StringUtil.EMPTY_STRING;
122+
return ROOT;
121123
}
124+
122125
final StringBuilder name = new StringBuilder(readable << 1);
123-
for (int len = in.readUnsignedByte(); in.isReadable() && len != 0; len = in.readUnsignedByte()) {
124-
boolean pointer = (len & 0xc0) == 0xc0;
126+
while (in.isReadable()) {
127+
final int len = in.readUnsignedByte();
128+
final boolean pointer = (len & 0xc0) == 0xc0;
125129
if (pointer) {
126130
if (position == -1) {
127131
position = in.readerIndex() + 1;
128132
}
129133

134+
if (!in.isReadable()) {
135+
throw new CorruptedFrameException("truncated pointer in a name");
136+
}
137+
130138
final int next = (len & 0x3f) << 8 | in.readUnsignedByte();
131139
if (next >= end) {
132140
throw new CorruptedFrameException("name has an out-of-range pointer");
@@ -138,18 +146,29 @@ protected String decodeName(ByteBuf in) {
138146
if (checked >= end) {
139147
throw new CorruptedFrameException("name contains a loop.");
140148
}
141-
} else {
149+
} else if (len != 0) {
150+
if (!in.isReadable(len)) {
151+
throw new CorruptedFrameException("truncated label in a name");
152+
}
142153
name.append(in.toString(in.readerIndex(), len, CharsetUtil.UTF_8)).append('.');
143154
in.skipBytes(len);
155+
} else { // len == 0
156+
break;
144157
}
145158
}
159+
146160
if (position != -1) {
147161
in.readerIndex(position);
148162
}
163+
149164
if (name.length() == 0) {
150-
return StringUtil.EMPTY_STRING;
165+
return ROOT;
166+
}
167+
168+
if (name.charAt(name.length() - 1) != '.') {
169+
name.append('.');
151170
}
152171

153-
return name.substring(0, name.length() - 1);
172+
return name.toString();
154173
}
155174
}

‎netty-bp/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import io.netty.handler.codec.UnsupportedMessageTypeException;
2121
import io.netty.util.internal.StringUtil;
2222

23+
import static io.netty.handler.codec.dns.DefaultDnsRecordDecoder.ROOT;
24+
2325
/**
2426
* The default {@link DnsRecordEncoder} implementation.
2527
*
@@ -77,19 +79,24 @@ private void encodeRawRecord(DnsRawRecord record, ByteBuf out) throws Exception
7779
}
7880

7981
protected void encodeName(String name, ByteBuf buf) throws Exception {
80-
String[] parts = StringUtil.split(name, '.');
81-
for (String part: parts) {
82-
final int partLen = part.length();
83-
// We always need to write the length even if its 0.
84-
// See:
85-
// - https://github.com/netty/netty/issues/5014
86-
// - https://www.ietf.org/rfc/rfc1035.txt , Section 3.1
87-
buf.writeByte(partLen);
88-
if (partLen == 0) {
89-
continue;
82+
if (ROOT.equals(name)) {
83+
// Root domain
84+
buf.writeByte(0);
85+
return;
86+
}
87+
88+
final String[] labels = StringUtil.split(name, '.');
89+
for (String label : labels) {
90+
final int labelLen = label.length();
91+
if (labelLen == 0) {
92+
// zero-length label means the end of the name.
93+
break;
9094
}
91-
ByteBufUtil.writeAscii(buf, part);
95+
96+
buf.writeByte(labelLen);
97+
ByteBufUtil.writeAscii(buf, label);
9298
}
99+
93100
buf.writeByte(0); // marks end of name field
94101
}
95102
}

‎netty-bp/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,47 @@
2323

2424
public class DefaultDnsRecordDecoderTest {
2525

26+
@Test
27+
public void testDecodeName() {
28+
testDecodeName("netty.io.", Unpooled.wrappedBuffer(new byte[] {
29+
5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0
30+
}));
31+
}
32+
33+
@Test
34+
public void testDecodeNameWithoutTerminator() {
35+
testDecodeName("netty.io.", Unpooled.wrappedBuffer(new byte[] {
36+
5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o'
37+
}));
38+
}
39+
40+
@Test
41+
public void testDecodeNameWithExtraTerminator() {
42+
// Should not be decoded as 'netty.io..'
43+
testDecodeName("netty.io.", Unpooled.wrappedBuffer(new byte[] {
44+
5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0, 0
45+
}));
46+
}
47+
2648
@Test
2749
public void testDecodeEmptyName() {
28-
testDecodeEmptyName0(Unpooled.buffer().writeByte('0'));
50+
testDecodeName(".", Unpooled.buffer().writeByte(0));
51+
}
52+
53+
@Test
54+
public void testDecodeEmptyNameFromEmptyBuffer() {
55+
testDecodeName(".", Unpooled.EMPTY_BUFFER);
2956
}
3057

3158
@Test
32-
public void testDecodeEmptyNameNonRFC() {
33-
testDecodeEmptyName0(Unpooled.EMPTY_BUFFER);
59+
public void testDecodeEmptyNameFromExtraZeroes() {
60+
testDecodeName(".", Unpooled.wrappedBuffer(new byte[] { 0, 0 }));
3461
}
3562

36-
private static void testDecodeEmptyName0(ByteBuf buffer) {
63+
private static void testDecodeName(String expected, ByteBuf buffer) {
3764
try {
3865
DefaultDnsRecordDecoder decoder = new DefaultDnsRecordDecoder();
39-
Assert.assertEquals(StringUtil.EMPTY_STRING, decoder.decodeName(buffer));
66+
Assert.assertEquals(expected, decoder.decodeName(buffer));
4067
} finally {
4168
buffer.release();
4269
}

‎netty-bp/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoderTest.java

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package io.netty.handler.codec.dns;
1717

1818
import io.netty.buffer.ByteBuf;
19+
import io.netty.buffer.ByteBufUtil;
1920
import io.netty.buffer.Unpooled;
2021
import io.netty.util.internal.StringUtil;
2122
import org.junit.Test;
@@ -24,18 +25,42 @@
2425

2526
public class DefaultDnsRecordEncoderTest {
2627

28+
@Test
29+
public void testEncodeName() throws Exception {
30+
testEncodeName(new byte[] { 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0 }, "netty.io.");
31+
}
32+
33+
@Test
34+
public void testEncodeNameWithoutTerminator() throws Exception {
35+
testEncodeName(new byte[] { 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0 }, "netty.io");
36+
}
37+
38+
@Test
39+
public void testEncodeNameWithExtraTerminator() throws Exception {
40+
testEncodeName(new byte[] { 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0 }, "netty.io..");
41+
}
42+
2743
// Test for https://github.com/netty/netty/issues/5014
2844
@Test
2945
public void testEncodeEmptyName() throws Exception {
46+
testEncodeName(new byte[] { 0 }, StringUtil.EMPTY_STRING);
47+
}
48+
49+
@Test
50+
public void testEncodeRootName() throws Exception {
51+
testEncodeName(new byte[] { 0 }, ".");
52+
}
53+
54+
private static void testEncodeName(byte[] expected, String name) throws Exception {
3055
DefaultDnsRecordEncoder encoder = new DefaultDnsRecordEncoder();
3156
ByteBuf out = Unpooled.buffer();
57+
ByteBuf expectedBuf = Unpooled.wrappedBuffer(expected);
3258
try {
33-
encoder.encodeName(StringUtil.EMPTY_STRING, out);
34-
assertEquals(2, out.readableBytes());
35-
assertEquals(0, out.readByte());
36-
assertEquals(0, out.readByte());
59+
encoder.encodeName(name, out);
60+
assertEquals(expectedBuf, out);
3761
} finally {
3862
out.release();
63+
expectedBuf.release();
3964
}
4065
}
4166
}

0 commit comments

Comments
 (0)