Fix crash on receipt of certain malformed DNS requests.

Return INSECURE when validating DNS replies which have RRSIGs, but
when a needed DS record in the trust chain is proved not to exist.
It's allowed for a zone to set up DNSKEY and RRSIG records first, then
add a DS later, completing the chain of trust.

Also, since we don't have the infrastructure to track that these
non-validated replies have RRSIGS, don't cache them, so we don't
provide answers with missing RRSIGS from the cache.

CHANGELOG

@@ -72,7 +72,15 @@ version 2.73
 	    on systems without an RTC, whilst allowing DNS queries before the
 	    clock is valid so that NTP can run. Thanks to
 	    Kevin Darbyshire-Bryant for developing this idea.
-	
+
+	    Add --tftp-no-fail option. Thanks to Stefan Tomanek for
+	    the patch.
+
+	    Fix crash caused by looking up servers.bind, CHAOS text record,
+	    when more than about five --servers= lines are in the dnsmasq
+	    config. This causes memory corruption which causes a crash later.
+	    Thanks to Matt Coddington for sterling work chasing this down.
+
 	
 version 2.72
             Add ra-advrouter mode, for RFC-3775 mobile IPv6 support.
@@ -117,6 +125,9 @@ version 2.72
             Fix problem with --local-service option on big-endian platforms
 	    Thanks to Richard Genoud for the patch.
 
+	    Fix crash on receipt of certain malformed DNS requests. Thanks
+	    to Nick Sampanis for spotting the problem.
+	
 
 version 2.71
             Subtle change to error handling to help DNSSEC validation 

dnsmasq.conf.example

@@ -486,6 +486,9 @@
 # Set the root directory for files available via FTP.
 #tftp-root=/var/ftpd
 
+# Do not abort if the tftp-root is unavailable
+#tftp-no-fail
+
 # Make the TFTP server more secure: with this set, only files owned by
 # the user dnsmasq is running as will be send over the net.
 #tftp-secure

man/dnsmasq.8

@@ -1711,6 +1711,9 @@ Absolute paths (starting with /) are allowed, but they must be within
 the tftp-root. If the optional interface argument is given, the
 directory is only used for TFTP requests via that interface.
 .TP
+.B --tftp-no-fail
+Do not abort startup if specified tftp root directories are inaccessible.
+.TP
 .B --tftp-unique-root
 Add the IP address of the TFTP client as a path component on the end
 of the TFTP-root (in standard dotted-quad format). Only valid if a

src/cache.c

@@ -1367,7 +1367,7 @@ int cache_make_stat(struct txt_record *t)
 		}
 	    port = prettyprint_addr(&serv->addr, daemon->addrbuff);
 	    lenp = p++; /* length */
-	    bytes_avail = (p - buff) + bufflen;
+	    bytes_avail = bufflen - (p - buff );
 	    bytes_needed = snprintf(p, bytes_avail, "%s#%d %u %u", daemon->addrbuff, port, queries, failed_queries);
 	    if (bytes_needed >= bytes_avail)
 	      {
@@ -1381,7 +1381,7 @@ int cache_make_stat(struct txt_record *t)
 		lenp = p - 1;
 		buff = new;
 		bufflen = newlen;
-		bytes_avail = (p - buff) + bufflen;
+		bytes_avail =  bufflen - (p - buff );
 		bytes_needed = snprintf(p, bytes_avail, "%s#%d %u %u", daemon->addrbuff, port, queries, failed_queries);
 	      }
 	    *lenp = bytes_needed;
@@ -1399,6 +1399,20 @@ int cache_make_stat(struct txt_record *t)
   return 1;
 }
 
+/* There can be names in the cache containing control chars, don't 
+   mess up logging or open security holes. */
+static char *sanitise(char *name)
+{
+  unsigned char *r;
+  if (name)
+    for (r = (unsigned char *)name; *r; r++)
+      if (!isprint((int)*r))
+	return "<name unprintable>";
+
+  return name;
+}
+
+
 void dump_cache(time_t now)
 {
   struct server *serv, *serv1;
@@ -1452,9 +1466,9 @@ void dump_cache(time_t now)
 	    *a = 0;
 	    if (strlen(n) == 0 && !(cache->flags & F_REVERSE))
 	      n = "<Root>";
-	    p += sprintf(p, "%-30.30s ", n);
+	    p += sprintf(p, "%-30.30s ", sanitise(n));
 	    if ((cache->flags & F_CNAME) && !is_outdated_cname_pointer(cache))
-	      a = cache_get_cname_target(cache);
+	      a = sanitise(cache_get_cname_target(cache));
 #ifdef HAVE_DNSSEC
 	    else if (cache->flags & F_DS)
 	      {
@@ -1587,6 +1601,8 @@ void log_query(unsigned int flags, char *name, struct all_addr *addr, char *arg)
   if (!option_bool(OPT_LOG))
     return;
 
+  name = sanitise(name);
+
   if (addr)
     {
       if (flags & F_KEYTAG)

src/dnsmasq.c

@@ -58,6 +58,9 @@ int main (int argc, char **argv)
   struct dhcp_context *context;
   struct dhcp_relay *relay;
 #endif
+#ifdef HAVE_TFTP
+  int tftp_prefix_missing = 0;
+#endif
 
 #ifdef LOCALEDIR
   setlocale(LC_ALL, "");
@@ -342,7 +345,7 @@ int main (int argc, char **argv)
 #else
   die(_("DBus not available: set HAVE_DBUS in src/config.h"), NULL, EC_BADCONF);
 #endif
-  
+
   if (daemon->port != 0)
     pre_allocate_sfds();
 
@@ -636,7 +639,7 @@ int main (int argc, char **argv)
 #endif
 
 #ifdef HAVE_TFTP
-      if (option_bool(OPT_TFTP))
+  if (option_bool(OPT_TFTP))
     {
       DIR *dir;
       struct tftp_prefix *p;
@@ -645,19 +648,28 @@ int main (int argc, char **argv)
 	{
 	  if (!((dir = opendir(daemon->tftp_prefix))))
 	    {
-	      send_event(err_pipe[1], EVENT_TFTP_ERR, errno, daemon->tftp_prefix);
-	      _exit(0);
+	      tftp_prefix_missing = 1;
+	      if (!option_bool(OPT_TFTP_NO_FAIL))
+	        {
+	          send_event(err_pipe[1], EVENT_TFTP_ERR, errno, daemon->tftp_prefix);
+	          _exit(0);
+	        }
 	    }
 	  closedir(dir);
 	}
 
       for (p = daemon->if_prefix; p; p = p->next)
 	{
+	  p->missing = 0;
 	  if (!((dir = opendir(p->prefix))))
-	   {
-	     send_event(err_pipe[1], EVENT_TFTP_ERR, errno, p->prefix);
-	     _exit(0);
-	   } 
+	    {
+	      p->missing = 1;
+	      if (!option_bool(OPT_TFTP_NO_FAIL))
+		{
+		  send_event(err_pipe[1], EVENT_TFTP_ERR, errno, p->prefix);
+		  _exit(0);
+		}
+	    }
 	  closedir(dir);
 	}
     }
@@ -773,6 +785,7 @@ int main (int argc, char **argv)
 #ifdef HAVE_TFTP
   if (option_bool(OPT_TFTP))
     {
+      struct tftp_prefix *p;
 #ifdef FD_SETSIZE
       if (FD_SETSIZE < (unsigned)max_fd)
 	max_fd = FD_SETSIZE;
@@ -782,7 +795,14 @@ int main (int argc, char **argv)
 		daemon->tftp_prefix ? _("root is ") : _("enabled"),
 		daemon->tftp_prefix ? daemon->tftp_prefix: "",
 		option_bool(OPT_TFTP_SECURE) ? _("secure mode") : "");
-      
+
+      if (tftp_prefix_missing)
+	my_syslog(MS_TFTP | LOG_WARNING, _("warning: %s inaccessible"), daemon->tftp_prefix);
+
+      for (p = daemon->if_prefix; p; p = p->next)
+	if (p->missing)
+	   my_syslog(MS_TFTP | LOG_WARNING, _("warning: TFTP directory %s inaccessible"), p->prefix);
+
       /* This is a guess, it assumes that for small limits, 
 	 disjoint files might be served, but for large limits, 
 	 a single file will be sent to may clients (the file only needs

src/dnsmasq.h

@@ -240,7 +240,8 @@ struct event_desc {
 #define OPT_LOCAL_SERVICE  49
 #define OPT_LOOP_DETECT    50
 #define OPT_EXTRALOG       51
-#define OPT_LAST           52
+#define OPT_TFTP_NO_FAIL   52
+#define OPT_LAST           53
 
 /* extra flags for my_syslog, we use a couple of facilities since they are known 
    not to occupy the same bits as priorities, no matter how syslog.h is set up. */
@@ -582,6 +583,7 @@ struct hostsfile {
 #define STAT_NO_NS             10
 #define STAT_NEED_DS_NEG       11
 #define STAT_CHASE_CNAME       12
+#define STAT_INSECURE_DS       13
 
 #define FREC_NOREBIND           1
 #define FREC_CHECKING_DISABLED  2
@@ -901,6 +903,7 @@ struct addr_list {
 struct tftp_prefix {
   char *interface;
   char *prefix;
+  int missing;
   struct tftp_prefix *next;
 };
 

src/dnssec.c

@@ -981,7 +981,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch
   
   /* If we've cached that DS provably doesn't exist, result must be INSECURE */
   if (crecp->flags & F_NEG)
-    return STAT_INSECURE;
+    return STAT_INSECURE_DS;
   
   /* NOTE, we need to find ONE DNSKEY which matches the DS */
   for (valid = 0, j = ntohs(header->ancount); j != 0 && !valid; j--) 
@@ -1177,7 +1177,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch
    STAT_NO_DS       It's proved there's no DS here.
    STAT_NO_NS       It's proved there's no DS _or_ NS here.
    STAT_BOGUS       no DS in reply or not signed, fails validation, bad packet.
-   STAT_NEED_DNSKEY DNSKEY records to validate a DS not found, name in keyname
+   STAT_NEED_KEY    DNSKEY records to validate a DS not found, name in keyname
 */
 
 int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int class)
@@ -1208,7 +1208,10 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char
   if (!(p = skip_section(p, ntohs(header->ancount), header, plen)))
     val = STAT_BOGUS;
   
-  if (val == STAT_BOGUS)
+  /* If the key needed to validate the DS is on the same domain as the DS, we'll
+     loop getting nowhere. Stop that now. This can happen of the DS answer comes
+     from the DS's zone, and not the parent zone. */
+  if (val == STAT_BOGUS ||  (val == STAT_NEED_KEY && hostname_isequal(name, keyname)))
     {
       log_query(F_NOEXTRA | F_UPSTREAM, name, NULL, "BOGUS DS");
       return STAT_BOGUS;
@@ -2032,7 +2035,8 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch
   /* NXDOMAIN or NODATA reply, prove that (name, class1, type1) can't exist */
   /* First marshall the NSEC records, if we've not done it previously */
   if (!nsec_type && !(nsec_type = find_nsec_records(header, plen, &nsecs, &nsec_count, qclass)))
-    return STAT_BOGUS; /* No NSECs */
+    return STAT_NO_SIG; /* No NSECs, this is probably a dangling CNAME pointing into
+			   an unsigned zone. Return STAT_NO_SIG to cause this to be proved. */
    
   /* Get name of missing answer */
   if (!extract_name(header, plen, &qname, name, 1, 0))

src/forward.c

@@ -521,7 +521,8 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
 }
 
 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 ad_reqd, int do_bit, int added_pheader, int check_subnet, union mysockaddr *query_source)
+			    int no_cache, int cache_secure, int bogusanswer, int ad_reqd, int do_bit, int added_pheader, 
+			    int check_subnet, union mysockaddr *query_source)
 {
   unsigned char *pheader, *sizep;
   char **sets = 0;
@@ -529,7 +530,8 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
   size_t plen; 
 
   (void)ad_reqd;
-  (void) do_bit;
+  (void)do_bit;
+  (void)bogusanswer;
 
 #ifdef HAVE_IPSET
   if (daemon->ipsets && extract_request(header, n, daemon->namebuff, NULL))
@@ -634,7 +636,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
     }
   
 #ifdef HAVE_DNSSEC
-  if (no_cache && !(header->hb4 & HB4_CD)) 
+  if (bogusanswer && !(header->hb4 & HB4_CD)) 
     {
       if (!option_bool(OPT_DNSSEC_DEBUG))
 	{
@@ -663,6 +665,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
       header->ancount = htons(0);
       header->nscount = htons(0);
       header->arcount = htons(0);
+      header->hb3 &= ~HB3_TC;
     }
   
   /* the bogus-nxdomain stuff, doctor and NXDOMAIN->NODATA munging can all elide
@@ -785,7 +788,7 @@ void reply_query(int fd, int family, time_t now)
      everything is broken */
   if (forward->forwardall == 0 || --forward->forwardall == 1 || RCODE(header) != SERVFAIL)
     {
-      int check_rebind = 0, no_cache_dnssec = 0, cache_secure = 0;
+      int check_rebind = 0, no_cache_dnssec = 0, cache_secure = 0, bogusanswer = 0;
 
       if (option_bool(OPT_NO_REBIND))
 	check_rebind = !(forward->flags & FREC_NOREBIND);
@@ -818,7 +821,13 @@ void reply_query(int fd, int family, time_t now)
 	  else if (forward->flags & FREC_DS_QUERY)
 	    {
 	      status = dnssec_validate_ds(now, header, n, daemon->namebuff, daemon->keyname, forward->class);
-	      if (status == STAT_NO_DS || status == STAT_NO_NS)
+	      /* Provably no DS, everything below is insecure, even if signatures are offered */
+	      if (status == STAT_NO_DS)
+		/* We only cache sigs when we've validated a reply.
+		   Avoid caching a reply with sigs if there's a vaildated break in the 
+		   DS chain, so we don't return replies from cache missing sigs. */
+		status = STAT_INSECURE_DS;
+	      else if (status == STAT_NO_NS)
 		status = STAT_BOGUS;
 	    }
 	  else if (forward->flags & FREC_CHECK_NOSIGN)
@@ -958,8 +967,14 @@ void reply_query(int fd, int family, time_t now)
 		  else if (forward->flags & FREC_DS_QUERY)
 		    {
 		      status = dnssec_validate_ds(now, header, n, daemon->namebuff, daemon->keyname, forward->class);
-		      if (status == STAT_NO_DS || status == STAT_NO_NS)
-			status = STAT_BOGUS;
+		       /* Provably no DS, everything below is insecure, even if signatures are offered */
+		      if (status == STAT_NO_DS)
+			/* We only cache sigs when we've validated a reply.
+			   Avoid caching a reply with sigs if there's a vaildated break in the 
+			   DS chain, so we don't return replies from cache missing sigs. */
+			status = STAT_INSECURE_DS;
+		      else if (status == STAT_NO_NS)
+			status = STAT_BOGUS; 
 		    }
 		  else if (forward->flags & FREC_CHECK_NOSIGN)
 		    {
@@ -984,6 +999,17 @@ void reply_query(int fd, int family, time_t now)
 		}
 	    }
 	  
+	  no_cache_dnssec = 0;
+
+	  if (status == STAT_INSECURE_DS)
+	    {
+	      /* We only cache sigs when we've validated a reply.
+		 Avoid caching a reply with sigs if there's a vaildated break in the 
+		 DS chain, so we don't return replies from cache missing sigs. */
+	      status = STAT_INSECURE;
+	      no_cache_dnssec = 1;
+	    }
+	  
 	  if (status == STAT_TRUNCATED)
 	    header->hb3 |= HB3_TC;
 	  else
@@ -991,19 +1017,23 @@ void reply_query(int fd, int family, time_t now)
 	      char *result;
 	      
 	      if (forward->work_counter == 0)
-		result = "ABANDONED";
+		{
+		  result = "ABANDONED";
+		  status = STAT_BOGUS;
+		}
 	      else
 		result = (status == STAT_SECURE ? "SECURE" : (status == STAT_INSECURE ? "INSECURE" : "BOGUS"));
 	      
 	      log_query(F_KEYTAG | F_SECSTAT, "result", NULL, result);
 	    }
 	  
-	  no_cache_dnssec = 0;
-	  
 	  if (status == STAT_SECURE)
 	    cache_secure = 1;
 	  else if (status == STAT_BOGUS)
-	    no_cache_dnssec = 1;
+	    {
+	      no_cache_dnssec = 1;
+	      bogusanswer = 1;
+	    }
 	}
 #endif     
       
@@ -1013,7 +1043,7 @@ void reply_query(int fd, int family, time_t now)
       else
 	header->hb4 &= ~HB4_CD;
       
-      if ((nn = process_reply(header, now, server, (size_t)n, check_rebind, no_cache_dnssec, cache_secure,
+      if ((nn = process_reply(header, now, server, (size_t)n, check_rebind, no_cache_dnssec, cache_secure, bogusanswer, 
 			      forward->flags & FREC_AD_QUESTION, forward->flags & FREC_DO_QUESTION, 
 			      forward->flags & FREC_ADDED_PHEADER, forward->flags & FREC_HAS_SUBNET, &forward->source)))
 	{
@@ -1416,7 +1446,7 @@ static int do_check_sign(struct frec *forward, int status, time_t now, char *nam
     }
 }
 
-/* Move toward the root, until we find a signed non-existance of a DS, in which case
+/* Move down from the root, until we find a signed non-existance of a DS, in which case
    an unsigned answer is OK, or we find a signed DS, in which case there should be 
    a signature, and the answer is BOGUS */
 static int  tcp_check_for_unsigned_zone(time_t now, struct dns_header *header, size_t plen, int class, char *name, 
@@ -1566,8 +1596,13 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si
   else if (status == STAT_NEED_DS || status == STAT_NEED_DS_NEG)
     {
       new_status = dnssec_validate_ds(now, header, n, name, keyname, class);
-      if (status == STAT_NEED_DS && (new_status == STAT_NO_DS || new_status == STAT_NO_NS))
-	new_status = STAT_BOGUS;
+      if (status == STAT_NEED_DS)
+	{
+	  if (new_status == STAT_NO_DS)
+	    new_status = STAT_INSECURE_DS;
+	  else if (new_status == STAT_NO_NS)
+	    new_status = STAT_BOGUS;
+	}
     }
   else if (status == STAT_CHASE_CNAME)
     new_status = dnssec_chase_cname(now, header, n, name, keyname);
@@ -1626,8 +1661,13 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si
 	      else if (status == STAT_NEED_DS || status == STAT_NEED_DS_NEG)
 		{
 		  new_status = dnssec_validate_ds(now, header, n, name, keyname, class);
-		  if (status == STAT_NEED_DS  && (new_status == STAT_NO_DS || new_status == STAT_NO_NS))
-		    new_status = STAT_BOGUS; /* Validated no DS */
+		  if (status == STAT_NEED_DS)
+		    {
+		      if (new_status == STAT_NO_DS)
+			new_status = STAT_INSECURE_DS;
+		      else if (new_status == STAT_NO_NS)
+			new_status = STAT_BOGUS; /* Validated no DS */
+		    }
 		}
 	      else if (status == STAT_CHASE_CNAME)
 		new_status = dnssec_chase_cname(now, header, n, name, keyname);
@@ -1648,7 +1688,7 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si
 		goto another_tcp_key;
 	    }
 	}
-
+      
       free(packet);
     }
   return new_status;
@@ -1669,7 +1709,7 @@ unsigned char *tcp_request(int confd, time_t now,
   int local_auth = 0;
 #endif
   int checking_disabled, ad_question, do_bit, added_pheader = 0;
-  int check_subnet, no_cache_dnssec = 0, cache_secure = 0;
+  int check_subnet, no_cache_dnssec = 0, cache_secure = 0, bogusanswer = 0;
   size_t m;
   unsigned short qtype;
   unsigned int gotname;
@@ -1937,16 +1977,31 @@ unsigned char *tcp_request(int confd, time_t now,
 			  int status = tcp_key_recurse(now, STAT_TRUNCATED, header, m, 0, daemon->namebuff, daemon->keyname, last_server, &keycount);
 			  char *result;
 
+			  if (status == STAT_INSECURE_DS)
+			    {
+			      /* We only cache sigs when we've validated a reply.
+				 Avoid caching a reply with sigs if there's a vaildated break in the 
+				 DS chain, so we don't return replies from cache missing sigs. */
+			      status = STAT_INSECURE;
+			      no_cache_dnssec = 1;
+			    }
+			  
 			  if (keycount == 0)
-			    result = "ABANDONED";
+			    {
+			      result = "ABANDONED";
+			      status = STAT_BOGUS;
+			    }
 			  else
 			    result = (status == STAT_SECURE ? "SECURE" : (status == STAT_INSECURE ? "INSECURE" : "BOGUS"));
 			  
 			  log_query(F_KEYTAG | F_SECSTAT, "result", NULL, result);
 			  
 			  if (status == STAT_BOGUS)
-			    no_cache_dnssec = 1;
-			  
+			    {
+			      no_cache_dnssec = 1;
+			      bogusanswer = 1;
+			    }
+
 			  if (status == STAT_SECURE)
 			    cache_secure = 1;
 			}
@@ -1980,7 +2035,7 @@ unsigned char *tcp_request(int confd, time_t now,
 #endif
 
 		      m = process_reply(header, now, last_server, (unsigned int)m, 
-					option_bool(OPT_NO_REBIND) && !norebind, no_cache_dnssec,
+					option_bool(OPT_NO_REBIND) && !norebind, no_cache_dnssec, bogusanswer,
 					cache_secure, ad_question, do_bit, added_pheader, check_subnet, &peer_addr); 
 		      
 		      break;

src/option.c

@@ -153,6 +153,7 @@ struct myoption {
 #define LOPT_DHOPT_INOTIFY 341
 #define LOPT_HOST_INOTIFY  342
 #define LOPT_DNSSEC_STAMP  343
+#define LOPT_TFTP_NO_FAIL  344
 
 #ifdef HAVE_GETOPT_LONG
 static const struct option opts[] =  
@@ -235,6 +236,7 @@ static const struct myoption opts[] =
     { "dhcp-ignore-names", 2, 0, LOPT_NO_NAMES },
     { "enable-tftp", 2, 0, LOPT_TFTP },
     { "tftp-secure", 0, 0, LOPT_SECURE },
+    { "tftp-no-fail", 0, 0, LOPT_TFTP_NO_FAIL },
     { "tftp-unique-root", 0, 0, LOPT_APREF },
     { "tftp-root", 1, 0, LOPT_PREFIX },
     { "tftp-max", 1, 0, LOPT_TFTP_MAX },
@@ -419,6 +421,7 @@ static struct {
   { LOPT_PREFIX, ARG_DUP, "<dir>[,<iface>]", gettext_noop("Export files by TFTP only from the specified subtree."), NULL },
   { LOPT_APREF, OPT_TFTP_APREF, NULL, gettext_noop("Add client IP address to tftp-root."), NULL },
   { LOPT_SECURE, OPT_TFTP_SECURE, NULL, gettext_noop("Allow access only to files owned by the user running dnsmasq."), NULL },
+  { LOPT_TFTP_NO_FAIL, OPT_TFTP_NO_FAIL, NULL, gettext_noop("Do not terminate the service if TFTP directories are inaccessible."), NULL },
   { LOPT_TFTP_MAX, ARG_ONE, "<integer>", gettext_noop("Maximum number of conncurrent TFTP transfers (defaults to %s)."), "#" },
   { LOPT_NOBLOCK, OPT_TFTP_NOBLOCK, NULL, gettext_noop("Disable the TFTP blocksize extension."), NULL },
   { LOPT_TFTP_LC, OPT_TFTP_LC, NULL, gettext_noop("Convert TFTP filenames to lowercase"), NULL },

src/rfc1035.c

@@ -128,8 +128,8 @@ int extract_name(struct dns_header *header, size_t plen, unsigned char **pp,
 	    if (isExtract)
 	      {
 		unsigned char c = *p;
-		if (isascii(c) && !iscntrl(c) && c != '.')
-		  *cp++ = *p;
+		if (c != 0 && c != '.')
+		  *cp++ = c;
 		else
 		  return 0;
 	      }
@@ -1198,7 +1198,10 @@ unsigned int extract_request(struct dns_header *header, size_t qlen, char *name,
 size_t setup_reply(struct dns_header *header, size_t qlen,
 		struct all_addr *addrp, unsigned int flags, unsigned long ttl)
 {
-  unsigned char *p = skip_questions(header, qlen);
+  unsigned char *p;
+
+  if (!(p = skip_questions(header, qlen)))
+    return 0;
   
   /* clear authoritative and truncated flags, set QR flag */
   header->hb3 = (header->hb3 & ~(HB3_AA | HB3_TC)) | HB3_QR;
@@ -1214,7 +1217,7 @@ size_t setup_reply(struct dns_header *header, size_t qlen,
     SET_RCODE(header, NOERROR); /* empty domain */
   else if (flags == F_NXDOMAIN)
     SET_RCODE(header, NXDOMAIN);
-  else if (p && flags == F_IPV4)
+  else if (flags == F_IPV4)
     { /* we know the address */
       SET_RCODE(header, NOERROR);
       header->ancount = htons(1);
@@ -1222,7 +1225,7 @@ size_t setup_reply(struct dns_header *header, size_t qlen,
       add_resource_record(header, NULL, NULL, sizeof(struct dns_header), &p, ttl, NULL, T_A, C_IN, "4", addrp);
     }
 #ifdef HAVE_IPV6
-  else if (p && flags == F_IPV6)
+  else if (flags == F_IPV6)
     {
       SET_RCODE(header, NOERROR);
       header->ancount = htons(1);

src/tftp.c

@@ -236,7 +236,7 @@ void tftp_request(struct listener *listen, time_t now)
       if (ioctl(listen->tftpfd, SIOCGIFMTU, &ifr) != -1)
 	mtu = ifr.ifr_mtu;      
     }
-  
+
   if (name)
     {
       /* check for per-interface prefix */