From 18a139f48fc4f54178d3ca28ebc4be1539289034 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sun, 17 May 2026 13:06:17 +0100 Subject: [PATCH] Change "limit" arg to add_pseudoheader() to a length. Also ripple effects for most of its callers. This fixes an OOB write caused by bad pointer arithmetic calculating the pointer-to-end-of-packet used before. Thanks to Donghyeon Jeong for spotting the bug. --- src/dnsmasq.h | 8 ++++---- src/dnssec.c | 4 ++-- src/edns0.c | 37 +++++++++++++++++++------------------ src/forward.c | 38 +++++++++++++++++++------------------- 4 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 4eb2f25..bdee8d0 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -1468,7 +1468,7 @@ int in_zone(struct auth_zone *zone, char *name, char **cut); /* dnssec.c */ #ifdef HAVE_DNSSEC -size_t dnssec_generate_query(struct dns_header *header, unsigned char *end, char *name, int class, int id, int type); +size_t dnssec_generate_query(struct dns_header *header, size_t outlen, char *name, int class, int id, int type); int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int class, int *validate_count); int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char *name, @@ -1939,11 +1939,11 @@ void from_wire(char *name); /* edns0.c */ unsigned char *find_pseudoheader(struct dns_header *header, size_t plen, size_t *len, unsigned char **p, int *is_sign, int *is_last); -size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *limit, +size_t add_pseudoheader(struct dns_header *header, size_t plen, size_t out_size, int optno, unsigned char *opt, size_t optlen, int set_do, int replace); -size_t add_do_bit(struct dns_header *header, size_t plen, unsigned char *limit); +size_t add_do_bit(struct dns_header *header, size_t plen, size_t outlen); void edns0_needs_mac(union mysockaddr *addr, time_t now); -size_t add_edns0_config(struct dns_header *header, size_t plen, unsigned char *limit, +size_t add_edns0_config(struct dns_header *header, size_t plen, size_t outlen, union mysockaddr *source, time_t now, int *cacheable); int check_source(struct dns_header *header, size_t plen, unsigned char *pseudoheader, union mysockaddr *peer); diff --git a/src/dnssec.c b/src/dnssec.c index a503fcd..4c37d0c 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -2332,7 +2332,7 @@ int dnskey_keytag(int alg, int flags, unsigned char *key, int keylen) } } -size_t dnssec_generate_query(struct dns_header *header, unsigned char *end, char *name, +size_t dnssec_generate_query(struct dns_header *header, size_t outlen, char *name, int class, int id, int type) { unsigned char *p; @@ -2356,7 +2356,7 @@ size_t dnssec_generate_query(struct dns_header *header, unsigned char *end, char PUTSHORT(type, p); PUTSHORT(class, p); - return add_do_bit(header, p - (unsigned char *)header, end); + return add_do_bit(header, p - (unsigned char *)header, outlen); } int errflags_to_ede(int status) diff --git a/src/edns0.c b/src/edns0.c index e1621e8..30158b8 100644 --- a/src/edns0.c +++ b/src/edns0.c @@ -100,13 +100,14 @@ unsigned char *find_pseudoheader(struct dns_header *header, size_t plen, size_t replace == 1 ->replace existing or add option replace == 2 ->relpace existing option only. */ -size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *limit, +size_t add_pseudoheader(struct dns_header *header, size_t plen, size_t out_size, int optno, unsigned char *opt, size_t optlen, int set_do, int replace) { unsigned char *lenp = NULL, *datap = NULL, *p, *udp_len, *buff = NULL; int rdlen = 0, is_sign, is_last; unsigned short flags = set_do ? 0x8000 : 0, rcode = 0; - + unsigned char *limit = ((unsigned char *)header) + out_size; + p = find_pseudoheader(header, plen, NULL, &udp_len, &is_sign, &is_last); if (is_sign) @@ -247,9 +248,9 @@ size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *l return p - (unsigned char *)header; } -size_t add_do_bit(struct dns_header *header, size_t plen, unsigned char *limit) +size_t add_do_bit(struct dns_header *header, size_t plen, size_t outlen) { - return add_pseudoheader(header, plen, (unsigned char *)limit, 0, NULL, 0, 1, 0); + return add_pseudoheader(header, plen, outlen, 0, NULL, 0, 1, 0); } static unsigned char char64(unsigned char c) @@ -277,7 +278,7 @@ void edns0_needs_mac(union mysockaddr *addr, time_t now) /* OPT_ADD_MAC = MAC is added (if available) OPT_ADD_MAC + OPT_STRIP_MAC = MAC is replaced, if not available, it is only removed OPT_STRIP_MAC = MAC is removed */ -static size_t add_dns_client(struct dns_header *header, size_t plen, unsigned char *limit, +static size_t add_dns_client(struct dns_header *header, size_t plen, size_t outlen, union mysockaddr *l3, time_t now, int *cacheablep) { int replace = 0, maclen = 0; @@ -303,7 +304,7 @@ static size_t add_dns_client(struct dns_header *header, size_t plen, unsigned ch replace = 2; if (replace != 0 || maclen == 6) - plen = add_pseudoheader(header, plen, limit, EDNS0_OPTION_NOMDEVICEID, (unsigned char *)encode, strlen(encode), 0, replace); + plen = add_pseudoheader(header, plen, outlen, EDNS0_OPTION_NOMDEVICEID, (unsigned char *)encode, strlen(encode), 0, replace); return plen; } @@ -312,7 +313,7 @@ static size_t add_dns_client(struct dns_header *header, size_t plen, unsigned ch /* OPT_ADD_MAC = MAC is added (if available) OPT_ADD_MAC + OPT_STRIP_MAC = MAC is replaced, if not available, it is only removed OPT_STRIP_MAC = MAC is removed */ -static size_t add_mac(struct dns_header *header, size_t plen, unsigned char *limit, +static size_t add_mac(struct dns_header *header, size_t plen, size_t outlen, union mysockaddr *l3, time_t now, int *cacheablep) { int maclen = 0, replace = 0; @@ -328,7 +329,7 @@ static size_t add_mac(struct dns_header *header, size_t plen, unsigned char *lim replace = 2; if (replace != 0 || maclen != 0) - plen = add_pseudoheader(header, plen, limit, EDNS0_OPTION_MAC, mac, maclen, 0, replace); + plen = add_pseudoheader(header, plen, outlen, EDNS0_OPTION_MAC, mac, maclen, 0, replace); return plen; } @@ -409,7 +410,7 @@ static size_t calc_subnet_opt(struct subnet_opt *opt, union mysockaddr *source, /* OPT_CLIENT_SUBNET = client subnet is added OPT_CLIENT_SUBNET + OPT_STRIP_ECS = client subnet is replaced OPT_STRIP_ECS = client subnet is removed */ -static size_t add_source_addr(struct dns_header *header, size_t plen, unsigned char *limit, +static size_t add_source_addr(struct dns_header *header, size_t plen, size_t outlen, union mysockaddr *source, int *cacheable) { /* http://tools.ietf.org/html/draft-vandergaast-edns-client-subnet-02 */ @@ -439,7 +440,7 @@ static size_t add_source_addr(struct dns_header *header, size_t plen, unsigned c return plen; } - return add_pseudoheader(header, plen, (unsigned char *)limit, EDNS0_OPTION_CLIENT_SUBNET, (unsigned char *)&opt, len, 0, replace); + return add_pseudoheader(header, plen, outlen, EDNS0_OPTION_CLIENT_SUBNET, (unsigned char *)&opt, len, 0, replace); } int check_source(struct dns_header *header, size_t plen, unsigned char *pseudoheader, union mysockaddr *peer) @@ -516,7 +517,7 @@ struct umbrella_opt { u8 fields[4 * UMBRELLA_TYPESZ + UMBRELLA_ORGSZ + IN6ADDRSZ + UMBRELLA_DEVICESZ + UMBRELLA_ASSETSZ]; }; -static size_t add_umbrella_opt(struct dns_header *header, size_t plen, unsigned char *limit, union mysockaddr *source, int *cacheable) +static size_t add_umbrella_opt(struct dns_header *header, size_t plen, size_t outlen, union mysockaddr *source, int *cacheable) { *cacheable = 0; @@ -548,28 +549,28 @@ static size_t add_umbrella_opt(struct dns_header *header, size_t plen, unsigned PUTLONG(daemon->umbrella_asset, u); } - return add_pseudoheader(header, plen, (unsigned char *)limit, EDNS0_OPTION_UMBRELLA, (unsigned char *)&opt, u - (u8 *)&opt, 0, 1); + return add_pseudoheader(header, plen, outlen, EDNS0_OPTION_UMBRELLA, (unsigned char *)&opt, u - (u8 *)&opt, 0, 1); } /* Set *check_subnet if we add a client subnet option, which needs to checked in the reply. Set *cacheable to zero if we add an option which the answer may depend on. */ -size_t add_edns0_config(struct dns_header *header, size_t plen, unsigned char *limit, +size_t add_edns0_config(struct dns_header *header, size_t plen, size_t outlen, union mysockaddr *source, time_t now, int *cacheable) { *cacheable = 1; - plen = add_mac(header, plen, limit, source, now, cacheable); - plen = add_dns_client(header, plen, limit, source, now, cacheable); + plen = add_mac(header, plen, outlen, source, now, cacheable); + plen = add_dns_client(header, plen, outlen, source, now, cacheable); if (daemon->dns_client_id) - plen = add_pseudoheader(header, plen, limit, EDNS0_OPTION_NOMCPEID, + plen = add_pseudoheader(header, plen, outlen, EDNS0_OPTION_NOMCPEID, (unsigned char *)daemon->dns_client_id, strlen(daemon->dns_client_id), 0, 1); if (option_bool(OPT_UMBRELLA)) - plen = add_umbrella_opt(header, plen, limit, source, cacheable); + plen = add_umbrella_opt(header, plen, outlen, source, cacheable); - plen = add_source_addr(header, plen, limit, source, cacheable); + plen = add_source_addr(header, plen, outlen, source, cacheable); return plen; } diff --git a/src/forward.c b/src/forward.c index bc23227..ee85cb1 100644 --- a/src/forward.c +++ b/src/forward.c @@ -375,7 +375,7 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr, #ifdef HAVE_DNSSEC if (option_bool(OPT_DNSSEC_VALID)) { - plen = add_do_bit(header, plen, ((unsigned char *) header) + daemon->edns_pktsz); + plen = add_do_bit(header, plen, daemon->edns_pktsz); /* For debugging, set Checking Disabled, otherwise, have the upstream check too, this allows it to select auth servers when one is returning bad data. */ @@ -514,7 +514,7 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr, forwarded = 0; /* Advertise the size of UDP reply we can accept. */ - plen = add_pseudoheader(header, plen, (unsigned char *)(header + daemon->edns_pktsz), 0, NULL, 0, 0, 0); + plen = add_pseudoheader(header, plen, daemon->edns_pktsz, 0, NULL, 0, 0, 0); /* check for send errors here (no route to host) if we fail to send to all nameservers, send back an error @@ -597,9 +597,9 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr, u16 swap = htons((u16)ede); if (ede != EDE_UNSET) - plen = add_pseudoheader(header, plen, (unsigned char *)(header + replylimit), EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, 0, 0); + plen = add_pseudoheader(header, plen, replylimit, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, 0, 0); else - plen = add_pseudoheader(header, plen, (unsigned char *)(header + replylimit), 0, NULL, 0, 0, 0); + plen = add_pseudoheader(header, plen, replylimit, 0, NULL, 0, 0, 0); } #if defined(HAVE_CONNTRACK) && defined(HAVE_UBUS) @@ -695,7 +695,7 @@ static struct ipsets *domain_find_sets(struct ipsets *setlist, const char *domai static size_t process_reply(struct dns_header *header, time_t now, struct server *server, size_t n, int check_rebind, int no_cache, int cache_secure, int bogusanswer, int ad_reqd, int do_bit, int added_pheader, - union mysockaddr *query_source, unsigned char *limit, int ede) + union mysockaddr *query_source, size_t outlen, int ede) { unsigned char *pheader, *sizep; struct ipsets *ipsets = NULL, *nftsets = NULL; @@ -879,7 +879,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server if (pheader && ede != EDE_UNSET) { u16 swap = htons((u16)ede); - n = add_pseudoheader(header, n, limit, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 1); + n = add_pseudoheader(header, n, outlen, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 1); } if (RCODE(header) == NXDOMAIN) @@ -1027,7 +1027,7 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, allocation of a new one: third arg of get_new_frec() does that. */ if ((serverind = dnssec_server(forward->sentto, daemon->keyname, STAT_ISEQUAL(status, STAT_NEED_DS), NULL, NULL)) != -1 && (server = daemon->serverarray[serverind]) && - (nn = dnssec_generate_query(header, ((unsigned char *) header) + daemon->edns_pktsz, + (nn = dnssec_generate_query(header, daemon->edns_pktsz, daemon->keyname, forward->class, get_id(), STAT_ISEQUAL(status, STAT_NEED_KEY) ? T_DNSKEY : T_DS)) && (fd = allocate_rfd(&rfds, server)) != -1 && @@ -1442,7 +1442,7 @@ void return_reply(time_t now, struct frec *forward, struct dns_header *header, s if ((nn = process_reply(header, now, forward->sentto, (size_t)n, check_rebind, no_cache_dnssec, cache_secure, bogusanswer, forward->flags & FREC_AD_QUESTION, forward->flags & FREC_DO_QUESTION, !(forward->flags & FREC_HAS_PHEADER), &forward->frec_src.source, - ((unsigned char *)header) + daemon->edns_pktsz, ede))) + daemon->edns_pktsz, ede))) { struct frec_src *src, *prev; int do_trunc; @@ -1945,7 +1945,7 @@ void receive_query(struct listener *listen, time_t now) { int cacheable; - n = add_edns0_config(header, n, ((unsigned char *)header) + daemon->edns_pktsz, &source_addr, now, &cacheable); + n = add_edns0_config(header, n, daemon->edns_pktsz, &source_addr, now, &cacheable); saved_question = blockdata_alloc((char *) header, (size_t)n); if (!cacheable) @@ -1979,12 +1979,12 @@ void receive_query(struct listener *listen, time_t now) { u16 swap = htons(ede); - m = add_pseudoheader(header, m, ((unsigned char *) header) + daemon->edns_pktsz, - EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); + m = add_pseudoheader(header, m, daemon->edns_pktsz, EDNS0_OPTION_EDE, + (unsigned char *)&swap, 2, do_bit, 0); } else - m = add_pseudoheader(header, m, ((unsigned char *) header) + daemon->edns_pktsz, - 0, NULL, 0, do_bit, 0); + m = add_pseudoheader(header, m, daemon->edns_pktsz, 0, + NULL, 0, do_bit, 0); } #ifdef HAVE_DUMPFILE @@ -2326,7 +2326,7 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si query_header = (struct dns_header *)daemon->packet; daemon->srv_save = NULL; - m = dnssec_generate_query(query_header, ((unsigned char *)query_header) + daemon->edns_pktsz, keyname, class, 0, + m = dnssec_generate_query(query_header, daemon->edns_pktsz, keyname, class, 0, STAT_ISEQUAL(new_status, STAT_NEED_KEY) ? T_DNSKEY : T_DS); if ((start = dnssec_server(server, keyname, STAT_ISEQUAL(new_status, STAT_NEED_DS), &first, &last)) == -1 || @@ -2479,7 +2479,7 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff, out_header = bigbuff->iov_base; /* Add edns0 pheader to query */ - size = add_edns0_config(header, size, ((unsigned char *) header) + daemon->packet_buff_sz, &peer_addr, now, &cacheable); + size = add_edns0_config(header, size, daemon->packet_buff_sz, &peer_addr, now, &cacheable); /* Clear buffer to avoid risk of information disclosure. */ memset(bigbuff->iov_base, 0, bigbuff->iov_len); @@ -2620,7 +2620,7 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff, #ifdef HAVE_DNSSEC if (option_bool(OPT_DNSSEC_VALID)) { - size = add_do_bit(header, size, ((unsigned char *) header) + daemon->edns_pktsz); + size = add_do_bit(header, size, daemon->edns_pktsz); /* For debugging, set Checking Disabled, otherwise, have the upstream check too, this allows it to select auth servers when one is returning bad data. */ @@ -2715,7 +2715,7 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff, m = process_reply(out_header, now, serv, (unsigned int)m, option_bool(OPT_NO_REBIND) && !norebind, no_cache_dnssec, cache_secure, bogusanswer, - ad_reqd, do_bit, !have_pseudoheader, &peer_addr, ((unsigned char *)out_header) + 65536, ede); + ad_reqd, do_bit, !have_pseudoheader, &peer_addr, 65536, ede); /* process_reply() adds pheader itself */ have_pseudoheader = 0; @@ -2747,9 +2747,9 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff, u16 swap = htons((u16)ede); if (ede != EDE_UNSET) - m = add_pseudoheader(out_header, m, ((unsigned char *) out_header) + 65536, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); + m = add_pseudoheader(out_header, m, 65536, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); else - m = add_pseudoheader(out_header, m, ((unsigned char *) out_header) + 65536, 0, NULL, 0, do_bit, 0); + m = add_pseudoheader(out_header, m, 65536, 0, NULL, 0, do_bit, 0); } check_log_writer(1);