From 8fef1e84b157200389b65292009eeaba1f6d600b Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 16 Sep 2026 15:13:29 +0200 Subject: [PATCH] dns-packet: limit the number of jumps during packet decompression So far we didn't limit the number of jumps when decompressing a DNS packet. This can lead to some extra work with specially crafted packets that send a staircase of thousands of descending compression pointers, which can then be amplified by referencing such staircase from thousands of names. Given that legitimate DNS packets need only a handful of jumps in the compression chains, let's limit the number of jumps per a single name to 256 (the same limit as, for example, Unbound currently has). Reported by opensec-intelligence (OpenSec). (cherry picked from commit efa16c7f31575d53ac7b90cbeb359f0d8a026246) --- src/resolve/test-dns-packet-extract.c | 113 ++++++++++++++++++++++++++ src/shared/dns-packet.c | 5 +- src/shared/dns-packet.h | 3 + 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/resolve/test-dns-packet-extract.c b/src/resolve/test-dns-packet-extract.c index 28a510dea6..2c053c299f 100644 --- a/src/resolve/test-dns-packet-extract.c +++ b/src/resolve/test-dns-packet-extract.c @@ -4846,4 +4846,117 @@ TEST(overlong_domain) { "0123456789012345678901234567890123456789012345678901234567890"); } +/* ================================================================ + * compression-pointer decompression limits + * ================================================================ */ + +/* Build a response whose first record stores a staircase of strictly descending compression pointers + * as opaque RDATA, and whose remaining records each own a name that points at the head of that + * staircase */ +static size_t append_pointer_staircase(DnsPacket *packet, size_t staircase_pointers, size_t names) { + _cleanup_free_ uint8_t *buf = NULL; + size_t rdata_base, rdlength, head = 0, ancount, total, off; + + assert(packet); + assert(!packet->extracted); + assert(staircase_pointers > 0); + + /* Absolute offset where record 1's RDATA begins: + * header(12) + name(1, root) + type(2) + class(2) + ttl(4) + rdlength(2) */ + rdata_base = DNS_PACKET_HEADER_SIZE + 1 + 10; + ASSERT_LE(rdata_base + 2 * staircase_pointers, (size_t) DNS_COMPRESSION_OFFSET_MAX); + + /* RDATA holds a root-label terminator, a pad byte, then the pointer staircase */ + rdlength = 2 + 2 * staircase_pointers; + + ancount = 1 + names; + total = rdata_base + rdlength + names * (2 + 10); + ASSERT_LE(total, (size_t) DNS_PACKET_SIZE_MAX); + + ASSERT_NOT_NULL(buf = malloc0(total)); + + /* Header: response, QDCOUNT=0, ANCOUNT=ancount. */ + buf[2] = BIT_QR | BIT_RD; + buf[3] = BIT_RA | DNS_RCODE_SUCCESS; + buf[6] = (uint8_t) (ancount >> 8); + buf[7] = (uint8_t) ancount; + + off = DNS_PACKET_HEADER_SIZE; + buf[off++] = 0x00; /* record 1 name = root */ + buf[off++] = 0xff; buf[off++] = 0x00; /* type = 65280 (private use, opaque RDATA) */ + buf[off++] = 0x00; buf[off++] = 0x01; /* class = IN */ + off += 4; /* ttl = 0 */ + buf[off++] = (uint8_t) (rdlength >> 8); + buf[off++] = (uint8_t) rdlength; + + /* Build a pointer staircase with a chain of pointers that all reference each other all the + * way back to the root-label terminator */ + ASSERT_EQ(off, rdata_base); + buf[off] = 0x00; /* rdata[0]: root-label chain terminator */ + for (size_t cursor = 2; cursor + 1 < rdlength; cursor += 2) { + size_t target = rdata_base + cursor - 2; /* previous slot in the staircase */ + + buf[off + cursor] = (uint8_t) (0xc0 | (target >> 8)); + buf[off + cursor + 1] = (uint8_t) (target & 0xff); + head = rdata_base + cursor; /* highest (last) pointer is the chain head */ + } + off += rdlength; + + /* Add a bunch of names that all point to the pointer staircase we just created */ + for (size_t i = 0; i < names; i++) { + buf[off++] = (uint8_t) (0xc0 | (head >> 8)); /* name = pointer to staircase head */ + buf[off++] = (uint8_t) (head & 0xff); + buf[off++] = 0xff; buf[off++] = 0x00; /* type = 65280 */ + buf[off++] = 0x00; buf[off++] = 0x01; /* class = IN */ + off += 4; /* ttl = 0 */ + off += 2; /* rdlength = 0 */ + } + + ASSERT_EQ(off, total); + + /* dns_packet_new() bumps the returned packet's internal size to DNS_PACKET_HEADER_SIZE on + * initialization, but here we try to append a complete packet (with our own header) to it, so let's + * truncate it to 0 before doing so, so our new header is at the start of the packet */ + dns_packet_truncate(packet, 0); + ASSERT_OK(dns_packet_append_blob(packet, buf, total, NULL)); + ASSERT_EQ(packet->size, total); + return total; +} + +/* A single name that walks the pointer staircase past the DNS_COMPRESSION_JUMPS_MAX limit */ +TEST(compression_pointer_chain_exceeds_per_name_cap) { + _cleanup_(dns_packet_unrefp) DnsPacket *packet = NULL; + size_t total; + + ASSERT_OK(dns_packet_new(&packet, DNS_PROTOCOL_DNS, 0, DNS_PACKET_SIZE_MAX)); + ASSERT_NOT_NULL(packet); + + total = append_pointer_staircase(packet, DNS_COMPRESSION_JUMPS_MAX + 50, 1); + ASSERT_GE(total, DNS_COMPRESSION_JUMPS_MAX + 1); + + ASSERT_ERROR(dns_packet_extract(packet), EBADMSG); +} + +/* A chain exactly at the per-name limit is fine; a chain one pointer longer should be rejected */ +TEST(compression_pointer_chain_per_name_cap_boundary) { + _cleanup_(dns_packet_unrefp) DnsPacket *packet = NULL; + + ASSERT_OK(dns_packet_new(&packet, DNS_PROTOCOL_DNS, 0, DNS_PACKET_SIZE_MAX)); + ASSERT_NOT_NULL(packet); + + /* Exactly at the per-name limit */ + append_pointer_staircase(packet, DNS_COMPRESSION_JUMPS_MAX - 1, 1); + ASSERT_OK(dns_packet_extract(packet)); + ASSERT_EQ(dns_answer_size(packet->answer), 2u); + ASSERT_OK(dns_packet_patch_ttls(packet, now(CLOCK_BOOTTIME))); + + packet = dns_packet_unref(packet); + ASSERT_OK(dns_packet_new(&packet, DNS_PROTOCOL_DNS, 0, DNS_PACKET_SIZE_MAX)); + ASSERT_NOT_NULL(packet); + + /* One pointer past the per-name limit */ + append_pointer_staircase(packet, DNS_COMPRESSION_JUMPS_MAX, 1); + ASSERT_ERROR(dns_packet_extract(packet), EBADMSG); +} + DEFINE_TEST_MAIN(LOG_DEBUG) diff --git a/src/shared/dns-packet.c b/src/shared/dns-packet.c index bb3b8f9235..a0508b1c68 100644 --- a/src/shared/dns-packet.c +++ b/src/shared/dns-packet.c @@ -1583,7 +1583,7 @@ int dns_packet_read_name( assert(p); _cleanup_(rewind_dns_packet) DnsPacketRewinder rewinder = REWINDER_INIT(p); - size_t after_rindex = 0, jump_barrier = p->rindex; + size_t after_rindex = 0, jump_barrier = p->rindex, jumps = 0; _cleanup_free_ char *name = NULL; bool first = true; size_t n = 0, m = 0; @@ -1643,6 +1643,9 @@ int dns_packet_read_name( if (ptr < DNS_PACKET_HEADER_SIZE || ptr >= jump_barrier) return -EBADMSG; + if (jumps++ >= DNS_COMPRESSION_JUMPS_MAX) + return -EBADMSG; + if (after_rindex == 0) after_rindex = p->rindex; diff --git a/src/shared/dns-packet.h b/src/shared/dns-packet.h index 5eab4ffab1..1bf577a7ce 100644 --- a/src/shared/dns-packet.h +++ b/src/shared/dns-packet.h @@ -51,6 +51,9 @@ assert_cc(sizeof(DnsPacketHeader) == 12); /* With EDNS0 we can use larger packets, default to 1232, which is what is commonly used */ #define DNS_PACKET_UNICAST_SIZE_LARGE_MAX 1232u +/* Maximum number of compression pointers we are willing to follow while decoding a single name */ +#define DNS_COMPRESSION_JUMPS_MAX 256U + typedef struct DnsPacket { unsigned n_ref; DnsProtocol protocol;