diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 6c5236fff83..d15847cf825 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -185,9 +185,9 @@ REPOSITORY_LOCATIONS = dict( sha256 = "c069c0d96137cf005d34411fa67dd3b6f1f8c64af1e7fb2fe0089a41c425acd7", ), com_github_packetzero_dnsparser = dict( - sha256 = "bdf6c7f56f33725c1c32e672a4779576fb639dd2df565115778eb6be48296431", - strip_prefix = "dnsparser-77398ffc200765db1cea9000d9f550ea99a29f7b", - urls = ["https://github.com/pixie-io/dnsparser/archive/77398ffc200765db1cea9000d9f550ea99a29f7b.tar.gz"], + sha256 = "de1c4270ddaf03c2d25ec02afd4b9b25e0748f84155449a2b68127813abad3a4", + strip_prefix = "dnsparser-362f3988b06b0831683155e110fdac946795c469", + urls = ["https://github.com/ddelnano/dnsparser/archive/362f3988b06b0831683155e110fdac946795c469.tar.gz"], ), com_github_pgcodekeeper_pgcodekeeper = dict( urls = ["https://github.com/pgcodekeeper/pgcodekeeper/archive/refs/tags/v5.11.3.tar.gz"], diff --git a/src/stirling/source_connectors/socket_tracer/protocols/dns/parse_test.cc b/src/stirling/source_connectors/socket_tracer/protocols/dns/parse_test.cc index e5379ec7fa3..bfb0878b521 100644 --- a/src/stirling/source_connectors/socket_tracer/protocols/dns/parse_test.cc +++ b/src/stirling/source_connectors/socket_tracer/protocols/dns/parse_test.cc @@ -461,6 +461,89 @@ TEST_F(DNSParserTest, PartialRecords) { } } +// Regression test: RFC 1035 s4.1.4 compression pointers are 2 bytes with a 14-bit offset. +// The bug was in dnsReadName: when it encountered a compression pointer inside a name, it +// only read the low byte of the 2-byte pointer, discarding the upper 6 bits of the offset. +// This test exercises that code path by constructing a valid DNS response where dnsReadName +// encounters a chained compression pointer whose offset is > 255. +// +// The critical path: answer 20 is a CNAME whose rdata name is "cdn" followed by a compression +// pointer to "example.com" label-encoded at offset 327 (0x147). When dnsReadName parses this +// CNAME rdata, it reads "cdn", then hits the compression pointer and recurses — this is the +// code path where the old 1-byte offset read would compute the wrong offset. +// +// Domain Name System (response) +// Transaction ID: 0xabcd +// Flags: 0x8180 Standard query response, No error +// Questions: 1 +// Answer RRs: 20 +// Authority RRs: 0 +// Additional RRs: 0 +// Queries +// www.example.com: type A, class IN +// Answers +// Answers 1-18: www.example.com: type A, class IN, addr 10.0.0.1 .. 10.0.0.18 +// Answer 19: other.example.com: type A, class IN, addr 10.0.0.19 +// (inline name places "example.com" at offset 327 = 0x147) +// Answer 20: www.example.com: type CNAME, class IN, cname cdn.example.com +// (rdata: "cdn" + compression pointer 0xC1 0x47 to offset 327) +constexpr uint8_t kCompressionPointer14BitFrame[] = { + 0xab, 0xcd, 0x81, 0x80, 0x00, 0x01, 0x00, 0x14, 0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77, + 0x07, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x03, 0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, + 0x01, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x01, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x02, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x03, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x04, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x05, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x06, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x07, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x08, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x09, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x0a, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x0b, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x0c, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x0d, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x0e, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x0f, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x10, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x11, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, 0x00, + 0x12, 0x05, 0x6f, 0x74, 0x68, 0x65, 0x72, 0x07, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x03, + 0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x04, 0x0a, 0x00, + 0x00, 0x13, 0xc0, 0x0c, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x06, 0x03, 0x63, + 0x64, 0x6e, 0xc1, 0x47}; + +TEST_F(DNSParserTest, CompressionPointerOffset14Bit) { + auto frame_view = + CreateStringView(CharArrayStringView(kCompressionPointer14BitFrame)); + + absl::flat_hash_map> frames; + ParseResult parse_result = + ParseFramesLoop(message_type_t::kResponse, frame_view, &frames); + + ASSERT_EQ(parse_result.state, ParseState::kSuccess); + + stream_id_t only_key = + frames.begin()->first; // Grab the first (and only) key. DNS has no notion of streams. + ASSERT_EQ(frames[only_key].size(), 1); + Frame& first_frame = frames[only_key][0]; + + EXPECT_EQ(first_frame.header.txid, 0xabcd); + EXPECT_EQ(first_frame.header.flags, 0x8180); + EXPECT_EQ(first_frame.header.num_queries, 1); + EXPECT_EQ(first_frame.header.num_answers, 20); + EXPECT_EQ(first_frame.header.num_auth, 0); + EXPECT_EQ(first_frame.header.num_addl, 0); + + ASSERT_EQ(first_frame.records().size(), 20); + + // The critical assertion: answer 20's CNAME rdata was parsed by dnsReadName which + // encountered "cdn" then a compression pointer 0xC1 0x47 (offset 327). With the old + // code this would read only 0x47 (71) — wrong offset. With the fix it correctly reads + // 327 and resolves to "cdn.example.com". + EXPECT_EQ(first_frame.records()[19].cname, "cdn.example.com"); +} + } // namespace dns } // namespace protocols } // namespace stirling