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

@@ -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);
 

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)

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;
 }

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);