Skip to content

Commit da93007

Browse files
committed
Fix DNS parser issue with compression pointers
The previous version of the parser didn't properly use the entire 14 bit compression field. This meant large DNS packets w/ compression ptrs had the chance of pointing to bogus memory. Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
1 parent e2dd202 commit da93007

File tree

2 files changed

+104
-3
lines changed

2 files changed

+104
-3
lines changed

bazel/repository_locations.bzl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,9 @@ REPOSITORY_LOCATIONS = dict(
185185
sha256 = "c069c0d96137cf005d34411fa67dd3b6f1f8c64af1e7fb2fe0089a41c425acd7",
186186
),
187187
com_github_packetzero_dnsparser = dict(
188-
sha256 = "bdf6c7f56f33725c1c32e672a4779576fb639dd2df565115778eb6be48296431",
189-
strip_prefix = "dnsparser-77398ffc200765db1cea9000d9f550ea99a29f7b",
190-
urls = ["https://github.com/pixie-io/dnsparser/archive/77398ffc200765db1cea9000d9f550ea99a29f7b.tar.gz"],
188+
sha256 = "de1c4270ddaf03c2d25ec02afd4b9b25e0748f84155449a2b68127813abad3a4",
189+
strip_prefix = "dnsparser-362f3988b06b0831683155e110fdac946795c469",
190+
urls = ["https://github.com/ddelnano/dnsparser/archive/362f3988b06b0831683155e110fdac946795c469.tar.gz"],
191191
),
192192
com_github_pgcodekeeper_pgcodekeeper = dict(
193193
urls = ["https://github.com/pgcodekeeper/pgcodekeeper/archive/refs/tags/v5.11.3.tar.gz"],

src/stirling/source_connectors/socket_tracer/protocols/dns/parse_test.cc

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,107 @@ TEST_F(DNSParserTest, PartialRecords) {
461461
}
462462
}
463463

464+
// Regression test: RFC 1035 s4.1.4 compression pointers are 2 bytes with a 14-bit offset.
465+
// The bug was in dnsReadName: when it encountered a compression pointer inside a name, it
466+
// only read the low byte of the 2-byte pointer, discarding the upper 6 bits of the offset.
467+
// This test exercises that code path by constructing a valid DNS response where dnsReadName
468+
// encounters a chained compression pointer whose offset is > 255.
469+
//
470+
// The critical path: answer 19 is a CNAME whose rdata name is "cdn" followed by a compression
471+
// pointer to "example.com" label-encoded at offset > 255. When dnsReadName parses this CNAME
472+
// rdata, it reads "cdn", then hits the compression pointer and recurses — this is the code
473+
// path where the old 1-byte offset read would compute the wrong offset.
474+
TEST_F(DNSParserTest, CompressionPointerOffset14Bit) {
475+
std::vector<uint8_t> pkt;
476+
477+
// --- DNS Header (12 bytes) ---
478+
pkt.push_back(0xAB); pkt.push_back(0xCD); // txid
479+
pkt.push_back(0x81); pkt.push_back(0x80); // flags: standard response
480+
pkt.push_back(0x00); pkt.push_back(0x01); // 1 query
481+
pkt.push_back(0x00); pkt.push_back(0x14); // 20 answers
482+
pkt.push_back(0x00); pkt.push_back(0x00); // 0 authority
483+
pkt.push_back(0x00); pkt.push_back(0x00); // 0 additional
484+
485+
// --- Query section (offset 12) ---
486+
// Name: "www.example.com"
487+
ASSERT_EQ(pkt.size(), 12u);
488+
pkt.push_back(0x03); pkt.insert(pkt.end(), {'w','w','w'});
489+
pkt.push_back(0x07); pkt.insert(pkt.end(), {'e','x','a','m','p','l','e'});
490+
pkt.push_back(0x03); pkt.insert(pkt.end(), {'c','o','m'});
491+
pkt.push_back(0x00);
492+
pkt.push_back(0x00); pkt.push_back(0x01); // type A
493+
pkt.push_back(0x00); pkt.push_back(0x01); // class IN
494+
// Query ends at offset 33.
495+
496+
// --- Answers 1-18: A records (18 * 16 = 288 bytes, offsets 33-320) ---
497+
for (int i = 0; i < 18; i++) {
498+
pkt.push_back(0xC0); pkt.push_back(0x0C); // name: ptr to offset 12
499+
pkt.push_back(0x00); pkt.push_back(0x01); // type A
500+
pkt.push_back(0x00); pkt.push_back(0x01); // class IN
501+
pkt.push_back(0x00); pkt.push_back(0x00);
502+
pkt.push_back(0x00); pkt.push_back(0x3C); // TTL=60
503+
pkt.push_back(0x00); pkt.push_back(0x04); // rdlength=4
504+
pkt.push_back(10); pkt.push_back(0);
505+
pkt.push_back(0); pkt.push_back(static_cast<uint8_t>(i + 1));
506+
}
507+
ASSERT_EQ(pkt.size(), 321u);
508+
509+
// --- Answer 19 (offset 321): A record with inline name that puts "example.com"
510+
// label-encoded at a known offset > 255. ---
511+
// Name: "other.example.com" written inline so "example.com" starts at offset 327.
512+
// offset 321
513+
pkt.push_back(0x05); pkt.insert(pkt.end(), {'o','t','h','e','r'});
514+
// "example" label starts here:
515+
size_t example_offset = pkt.size(); // 327 = 0x147
516+
pkt.push_back(0x07); pkt.insert(pkt.end(), {'e','x','a','m','p','l','e'});
517+
pkt.push_back(0x03); pkt.insert(pkt.end(), {'c','o','m'});
518+
pkt.push_back(0x00);
519+
ASSERT_EQ(example_offset, 327u);
520+
// type A, class IN, TTL, rdlen, addr
521+
pkt.push_back(0x00); pkt.push_back(0x01);
522+
pkt.push_back(0x00); pkt.push_back(0x01);
523+
pkt.push_back(0x00); pkt.push_back(0x00);
524+
pkt.push_back(0x00); pkt.push_back(0x3C);
525+
pkt.push_back(0x00); pkt.push_back(0x04);
526+
pkt.push_back(10); pkt.push_back(0); pkt.push_back(0); pkt.push_back(19);
527+
528+
// --- Answer 20 (offset ~357): CNAME record whose rdata name contains a chained
529+
// compression pointer to offset 327 (0x147) = "example.com".
530+
// rdata name: "cdn" + compression pointer 0xC1 0x47 → "cdn.example.com"
531+
// This is the code path through dnsReadName that triggers the bug. ---
532+
pkt.push_back(0xC0); pkt.push_back(0x0C); // name: ptr to "www.example.com"
533+
pkt.push_back(0x00); pkt.push_back(0x05); // type CNAME
534+
pkt.push_back(0x00); pkt.push_back(0x01); // class IN
535+
pkt.push_back(0x00); pkt.push_back(0x00);
536+
pkt.push_back(0x00); pkt.push_back(0x3C); // TTL=60
537+
// rdata: "cdn" (4 bytes) + compression pointer (2 bytes) = 6 bytes
538+
pkt.push_back(0x00); pkt.push_back(0x06); // rdlength=6
539+
// rdata: label "cdn" then pointer to offset 327 (0xC1 0x47)
540+
pkt.push_back(0x03); pkt.insert(pkt.end(), {'c','d','n'});
541+
pkt.push_back(0xC1); pkt.push_back(0x47); // ptr to offset 0x147 = 327
542+
543+
auto frame_view = CreateStringView<char>(
544+
std::string_view(reinterpret_cast<const char*>(pkt.data()), pkt.size()));
545+
546+
absl::flat_hash_map<stream_id_t, std::deque<Frame>> frames;
547+
ParseResult<stream_id_t> parse_result =
548+
ParseFramesLoop(message_type_t::kResponse, frame_view, &frames);
549+
550+
ASSERT_EQ(parse_result.state, ParseState::kSuccess);
551+
552+
stream_id_t only_key = frames.begin()->first;
553+
ASSERT_EQ(frames[only_key].size(), 1);
554+
Frame& frame = frames[only_key][0];
555+
556+
ASSERT_EQ(frame.records().size(), 20);
557+
558+
// The critical assertion: answer 20's CNAME rdata was parsed by dnsReadName which
559+
// encountered "cdn" then a compression pointer 0xC1 0x47 (offset 327). With the old
560+
// code this would read only 0x47 (71) — wrong offset. With the fix it correctly reads
561+
// 327 and resolves to "cdn.example.com".
562+
EXPECT_EQ(frame.records()[19].cname, "cdn.example.com");
563+
}
564+
464565
} // namespace dns
465566
} // namespace protocols
466567
} // namespace stirling

0 commit comments

Comments
 (0)