From 014e909f787e808bb35daa546d3f8f3663918de2 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Thu, 12 Feb 2026 16:23:42 +0000 Subject: [PATCH] Rework storage allocation for domain names. CVE-2026-2291 This is the stable release fix for CVE-2026-2291 Dnsmasq recieves and sends domain names within DNS packets in RFC-1035 format, a string of counted labels terminated by a zero length label. Internally to dnsmasq, domain names are represented as zero-terminated C strings with the labels seperated by '.' characters, like the normal presentation format for domain names. The RFC-1035 limit for a domain name in wire format is 255 octets, and when converted to presentation format, this makes a string of maximum 253 characters. However dnsmasq needs to be able to represent any 8-bit character within a label, and this causes problems with /000 (which would terminate the c-string early) and '.' (which would terminate a label early). To avoid this, dnsmasq escapes both of these characters, which means that they take two bytes in the string. The domain name with must charaters in it has four labels and the 255 octet wire length limits these four labels to a total of 250 characters. The other five octets are the four label length bytes and the zero length byte terminator. If all 250 characters need escaping that makes the maximum length of the dnsmasq internal format 503 characters (250 escaped characters in labels, amd three dots between four labels) Since this format is zero-terminated c-string, buffers have to be allocated as 504 bytes. This arrangement grew in a somewhat ad-hoc manner and early dnsmasq didn't do the escaping trick, and didn't really differentiate between the lengths of the wire format and the presentation format, since they were very similar. It also, for reasons lost in time, inherited a defintion for MAXDNAME from early BIND headers, set at 1025 bytes. This value was used as the buffer size for both wire and presentation formats. This patch makes everything consistent. It declares two constants MAXDNAME 255 /* max size of wire format domain name */ MAXDNAMESTR 503 /* max size of internal format created from a 255 octet wire format name */ All internal name buffers are allocated as MAXDNAMESTR+1 bytes, to hold a maximum size internal format name and zero termination. Names parsed out of incoming packets are rejected if their wire format is greater than 255 bytes, which ensures that their internal representation cannot exceed 503 characters. Names inserted into outgoing packets are failed similarly if they exceed 255 bytes. (this would in theory be possible for a legal internal-representaion name if it has at least one unescaped character in a label. --- CHANGELOG | 12 +++++++++++- src/cache.c | 8 ++++++-- src/config.h | 2 +- src/dbus.c | 2 +- src/dhcp-common.c | 4 ++-- src/dhcp.c | 2 +- src/dns-protocol.h | 32 ++++++++++++++++++++++++++++++-- src/dnsmasq.c | 9 ++------- src/dnsmasq.h | 8 ++++---- src/dnssec.c | 2 +- src/domain.c | 18 +++++++++--------- src/helper.c | 2 +- src/lease.c | 2 +- src/network.c | 4 ++-- src/option.c | 22 +++++++++------------- src/radv.c | 4 ++-- src/rfc1035.c | 19 ++++++++++++------- src/rfc2131.c | 6 +++--- src/rfc3315.c | 6 +++--- src/rrfilter.c | 26 ++++++++++---------------- src/tftp.c | 14 +++++++------- src/util.c | 24 ++++++++++++++++++------ 22 files changed, 136 insertions(+), 92 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b74f0d6..4acc376 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -33,7 +33,17 @@ version 2.93 This means that the permissions used when creating the watches are the same as used for accessing watched files, which makes more sense and avoids odd and confusing error conditions. - + + Rework storage allocation for domain names. This fixes a security + bug that can cause heap-overwrite with long domain names. + CVE-2026-2291 covers this and a simple patch for existing + releases was released with the CVE. This patch reworks the whole + code base to make it cleaner and less liable to future + confusions. Either upgrading to 2.93 or applying the patch + to earlier versions is sufficient to close the security hole. + Thanks to Andrew S. Fasano for spotting this problem in the + first place. + version 2.92 Redesign the interaction between DNSSEC validation and per-domain diff --git a/src/cache.c b/src/cache.c index f85b7a8..32192c1 100644 --- a/src/cache.c +++ b/src/cache.c @@ -22,6 +22,7 @@ static struct crec *new_chain = NULL; static int insert_error; static union bigname *big_free = NULL; static int bignames_left, hash_size; + struct nameblock { struct nameblock *next; unsigned int last, index; @@ -221,6 +222,9 @@ static char *store_name(unsigned int namelen, unsigned int index) struct nameblock *block; char *ret = NULL; + if (namelen > NAMEBLOCK_CHARS) + return NULL; + for (block = hostblocks; block; block = block->next) if (block->index == index && NAMEBLOCK_CHARS - block->last >= namelen) break; @@ -1418,7 +1422,7 @@ static int gettok(FILE *f, char *token) return eatspace(f); } - if (count < (MAXDNAME - 1)) + if (count < (MAXDNAMESTR - 1)) { token[count++] = c; token[count] = 0; @@ -1793,7 +1797,7 @@ void cache_add_dhcp_entry(char *host_name, int prot, /* Name in hosts, address doesn't match */ if (fail_crec) { - inet_ntop(prot, &fail_crec->addr, daemon->namebuff, MAXDNAME); + inet_ntop(prot, &fail_crec->addr, daemon->namebuff, MAXDNAMESTR); my_syslog(MS_DHCP | LOG_WARNING, _("not giving name %s to the DHCP lease of %s because " "the name exists in %s with address %s"), diff --git a/src/config.h b/src/config.h index bdd8acb..a010c8c 100644 --- a/src/config.h +++ b/src/config.h @@ -43,7 +43,7 @@ #define PING_CACHE_TIME 30 /* Ping test assumed to be valid this long. */ #define DECLINE_BACKOFF 600 /* disable DECLINEd static addresses for this long */ #define DHCP_PACKET_MAX 16384 /* hard limit on DHCP packet size */ -#define SMALLDNAME 50 /* most domain names are smaller than this */ +#define SMALLDNAME 75 /* most domain names are smaller than this */ #define CNAME_CHAIN 10 /* chains longer than this atr dropped for loop protection */ #define DNSSEC_MIN_TTL 60 /* DNSKEY and DS records in cache last at least this long */ #define HOSTSFILE "/etc/hosts" diff --git a/src/dbus.c b/src/dbus.c index afdadf2..aa59eb8 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -736,7 +736,7 @@ static void add_dict_entry(DBusMessageIter *container, const char *key, const ch static void add_dict_int(DBusMessageIter *container, const char *key, const unsigned int val) { - snprintf(daemon->namebuff, MAXDNAME, "%u", val); + snprintf(daemon->namebuff, MAXDNAMESTR, "%u", val); add_dict_entry(container, key, daemon->namebuff); } diff --git a/src/dhcp-common.c b/src/dhcp-common.c index a14c415..ef8fc44 100644 --- a/src/dhcp-common.c +++ b/src/dhcp-common.c @@ -278,9 +278,9 @@ void log_tags(struct dhcp_netid *netid, u32 xid) if (!n) { - strncat (s, netid->net, (MAXDNAME-1) - strlen(s)); + strncat (s, netid->net, MAXDNAMESTR - strlen(s)); if (netid->next) - strncat (s, ", ", (MAXDNAME-1) - strlen(s)); + strncat (s, ", ", MAXDNAMESTR - strlen(s)); } } my_syslog(MS_DHCP | LOG_INFO, _("%u tags: %s"), xid, s); diff --git a/src/dhcp.c b/src/dhcp.c index 7b25835..3f8dc29 100644 --- a/src/dhcp.c +++ b/src/dhcp.c @@ -958,7 +958,7 @@ void dhcp_read_ethers(void) up = &config->next; } - while (fgets(buff, MAXDNAME, f)) + while (fgets(buff, MAXDNAMESTR, f)) { char *host = NULL; diff --git a/src/dns-protocol.h b/src/dns-protocol.h index 9ef4083..43dddcf 100644 --- a/src/dns-protocol.h +++ b/src/dns-protocol.h @@ -22,8 +22,34 @@ #define IN6ADDRSZ 16 #define INADDRSZ 4 +/* Dnsmasq handles domains names internally as NULL-terminated C strings. + The '.' characters in these strings are significant as label-seperators. + '.' and /0 characters _within_ labels are escaped and take up two + characters to allow labels to contain arbitrary characters. + + The maximum length of a domain name in wire format is 255 bytes. + and the maximum length of this when coverted to a