Fix crash when no upstream servers defined.

This is a regession introduced in 3b6df06fb8.

When dnsmasq is started without upstreams (yet), but a
DNS query comes in that needs forwarding dnsmasq now potentially crashes as
the value for "first" variable is undetermined.

A segmentation violation occurs when the index
is out of bounds of  serverarray.

Credits go to pedro0311 <pedro@freshtomato.org>

CHANGELOG

@@ -113,9 +113,11 @@ version 2.91
 	badly with rare broken DNS servers which don't preserve the case
 	of the query in their reply. The first time a reply is returned
 	which matches the query in all respects except case, a warning
-	will be logged. If this coincides with DNS not functioning, it
-	is necessary to disable bit 0x20 encoding with --no-0x20-encode.
-
+	will be logged. In this release, 0x020-encoding is default-off
+	and must be explicitly enabled with --do-0x20-encoding. In future
+	releases it may default on. You can avoid a future release
+	changing the behaviour of an installation with --no-x20-encode.
+	
 	Fix a long-standing problem when two queries which are identical
 	in every repect _except_ case, get combined by dnsmasq. If
 	dnsmasq gets eg, two queries for example.com and Example.com

man/dnsmasq.8

@@ -852,13 +852,17 @@ name on successive queries, for load-balancing. This turns off that
 behaviour, so that the records are always returned in the order
 that they are received from upstream.
 .TP
-.B --no-0x20-encode
-By default, dnsmasq scrambles the case of letters in DNS queries it sends upstream as a security feature.
+.B --do-0x20-encode, --no-0x20-encode
+Dnsmasq can scramble the case of letters in DNS queries it sends upstream as a security feature.
 This technique can interact badly with rare broken DNS servers which don't preserve the case
 of the query in their reply. The first time a reply is returned
 which matches the query in all respects except case, a warning
 will be logged. If this coincides with DNS not functioning, it
-is necessary to disable this scrambling with --no-0x20-encode.
+is necessary to disable the feature. As at version 2.91, 0x20 encoding
+is disabled by default, and must be enabled with --do-0x20-encode. The default
+may change in the future, so to be sure of its status after an upgrade, set --do-0x20-encode
+or --no-0x20-encode in your config. --no-0x20-encode overrides --do-x20-encode or a future default
+0x20-encode enable.
 .TP
 .B --use-stale-cache[=<max TTL excess in s>]
 When set, if a DNS name exists in the cache, but its time-to-live has expired, dnsmasq will return the data anyway. (It attempts to refresh the

src/dnsmasq.h

@@ -280,7 +280,8 @@ struct event_desc {
 #define OPT_LOCALHOST_SERVICE  72
 #define OPT_LOG_PROTO      73
 #define OPT_NO_0x20        74
-#define OPT_LAST           75
+#define OPT_DO_0x20        75
+#define OPT_LAST           76
 
 #define OPTION_BITS (sizeof(unsigned int)*8)
 #define OPTION_SIZE ( (OPT_LAST/OPTION_BITS)+((OPT_LAST%OPTION_BITS)!=0) )

src/forward.c

@@ -354,8 +354,6 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
 	  flags = 0;
 	}
 
-      master = daemon->serverarray[first];
-
       /* don't forward A or AAAA queries for simple names, except the empty name */
       if (!flags &&
 	  option_bool(OPT_NODOTS_LOCAL) &&
@@ -367,7 +365,9 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
       /* Configured answer. */
       if (flags || ede == EDE_NOT_READY)
 	goto reply;
-      
+
+      master = daemon->serverarray[first];
+
       if (!(forward = get_new_frec(now, master, 0)))
 	goto reply;
       /* table full - flags == 0, return REFUSED */
@@ -391,7 +391,7 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
       forward->new_id = get_id();
       header->id = ntohs(forward->new_id);
       
-      forward->frec_src.encode_bitmap = option_bool(OPT_NO_0x20) ? 0 : rand32();
+      forward->frec_src.encode_bitmap = (!option_bool(OPT_NO_0x20) && option_bool(OPT_DO_0x20)) ? rand32() : 0;
       forward->frec_src.encode_bigmap = NULL;
       p = (unsigned char *)(header+1);
       if (!extract_name(header, plen, &p, (char *)&forward->frec_src.encode_bitmap, EXTR_NAME_FLIP, 1))
@@ -3200,12 +3200,12 @@ static struct frec *lookup_frec(char *target, int class, int rrtype, int id, int
   struct dns_header *header;
   int compare_mode = EXTR_NAME_COMPARE;
 
-  /* Only compare case-sensitive when matching frec to a recieved answer,
+  /* Only compare case-sensitive when matching frec to a received answer,
      NOT when looking for a duplicated question. */
   if (flags & FREC_ANSWER)
     {
       flags &= ~FREC_ANSWER;
-      if (!option_bool(OPT_NO_0x20))
+      if (!option_bool(OPT_NO_0x20) && option_bool(OPT_DO_0x20))
 	compare_mode = EXTR_NAME_NOCASE;
     }
   

src/option.c

@@ -194,6 +194,7 @@ struct myoption {
 #define LOPT_DNSSEC_LIMITS 385
 #define LOPT_PXE_OPT       386
 #define LOPT_NO_ENCODE     387
+#define LOPT_DO_ENCODE     388
 
 #ifdef HAVE_GETOPT_LONG
 static const struct option opts[] =  
@@ -249,6 +250,7 @@ static const struct myoption opts[] =
     { "no-negcache", 0, 0, 'N' },
     { "no-round-robin", 0, 0, LOPT_NORR },
     { "no-0x20-encode", 0, 0, LOPT_NO_ENCODE },
+    { "do-0x20-encode", 0, 0, LOPT_DO_ENCODE },
     { "cache-rr", 1, 0, LOPT_CACHE_RR },
     { "addn-hosts", 1, 0, 'H' },
     { "hostsdir", 1, 0, LOPT_HOST_INOTIFY },
@@ -594,6 +596,7 @@ static struct {
   { LOPT_QUIET_TFTP, OPT_QUIET_TFTP, NULL, gettext_noop("Do not log routine TFTP."), NULL },
   { LOPT_NORR, OPT_NORR, NULL, gettext_noop("Suppress round-robin ordering of DNS records."), NULL },
   { LOPT_NO_ENCODE, OPT_NO_0x20, NULL, gettext_noop("Suppress DNS bit 0x20 encoding."), NULL },
+  { LOPT_DO_ENCODE, OPT_DO_0x20, NULL, gettext_noop("Enable DNS bit 0x20 encoding."), NULL },
   { LOPT_NO_IDENT, OPT_NO_IDENT, NULL, gettext_noop("Do not add CHAOS TXT records."), NULL },
   { LOPT_CACHE_RR, ARG_DUP, "<RR-type>", gettext_noop("Cache this DNS resource record type."), NULL },
   { LOPT_MAX_PROCS, ARG_ONE, "<integer>", gettext_noop("Maximum number of concurrent tcp connections."), NULL },

src/rfc1035.c

@@ -35,9 +35,9 @@ int extract_name(struct dns_header *header, size_t plen, unsigned char **pp,
 {
   unsigned char *cp = (unsigned char *)name, *p = *pp, *p1 = NULL;
   unsigned int j, l, namelen = 0, hops = 0;
-  unsigned int bigmap_counter = 0, bigmap_posn = 0, bigmap_size, bitmap;
+  unsigned int bigmap_counter = 0, bigmap_posn = 0, bigmap_size = parm, bitmap = 0;
   int retvalue = 1, case_insens = 1, isExtract = 0, flip = 0, extrabytes = (int)parm;
-  unsigned int *bigmap;
+  unsigned int *bigmap = (unsigned int *)name;
 
   if (func == EXTR_NAME_EXTRACT)
     isExtract = 1, *cp = 0;
@@ -46,9 +46,7 @@ int extract_name(struct dns_header *header, size_t plen, unsigned char **pp,
   else if (func == EXTR_NAME_FLIP)
     {
       flip = 1, extrabytes = 0;
-      bigmap = (unsigned int *)name;
       name = NULL;
-      bigmap_size = parm;
     }
   
   while (1)

src/tftp.c

@@ -360,7 +360,7 @@ void tftp_request(struct listener *listen, time_t now)
     }
   
   p = packet + 2;
-  end = packet + 2 + len;
+  end = packet + len;
   
   if (ntohs(*((unsigned short *)packet)) != OP_RRQ ||
       !(filename = next(&p, end)) ||
@@ -742,15 +742,16 @@ static void free_transfer(struct tftp_transfer *transfer)
 
 static char *next(char **p, char *end)
 {
-  char *ret = *p;
-  size_t len;
+  char *n, *ret = *p;
+  
+  /* Look for end of string, without running off the end of the packet. */
+  for (n = *p; n < end && *n != 0; n++);
 
-  if (*(end-1) != 0 || 
-      *p == end ||
-      (len = strlen(ret)) == 0)
+  /* ran off the end or zero length string - failed */
+  if (n == end || n == ret)
     return NULL;
-
-  *p += len + 1;
+  
+  *p = n + 1;
   return ret;
 }