Skip to content

Fix vizier-pem DNS parser crash due to stack overflow and eventual segfault #2349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ddelnano wants to merge 1 commit into
base: main
Choose a base branch
from
+86 −3
Open

Fix vizier-pem DNS parser crash due to stack overflow and eventual segfault #2349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions bazel/repository_locations.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Comment on lines -188 to +190
Copy link
Copy Markdown
Member Author

@ddelnano ddelnano Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be updated to pixie-io/dnsparser once pixie-io/dnsparser#1 is merged.

),
com_github_pgcodekeeper_pgcodekeeper = dict(
urls = ["https://github.com/pgcodekeeper/pgcodekeeper/archive/refs/tags/v5.11.3.tar.gz"],
Expand Down
83 changes: 83 additions & 0 deletions src/stirling/source_connectors/socket_tracer/protocols/dns/parse_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<char>(CharArrayStringView<uint8_t>(kCompressionPointer14BitFrame));

absl::flat_hash_map<stream_id_t, std::deque<Frame>> frames;
ParseResult<stream_id_t> 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
Expand Down
Loading