Fix 1-byte buffer overflow in relay_reply4()

Potential SIGSEGV when using DHCPv4-relay.

Thanks to Asim Viladi Oglu Manizada for finding this.

src/cache.c

@@ -1083,6 +1083,9 @@ int cache_recv_insert(time_t now, int fd)
 	     !read_write(fd, (unsigned char *)&validatecount, sizeof(validatecount), RW_READ) ||
 	     !read_write(fd, (unsigned char *)&validatecountp, sizeof(validatecountp), RW_READ)))
 	  return 0;
+
+	if (op == PIPE_OP_KILLED)
+	  my_syslog(LOG_INFO, _("TCP process for DNSSEC validation timed out"));
 	
 	/* There's a tiny chance that the frec may have been freed 
 	   and reused before the TCP process returns. Detect that with

src/dhcp-common.c

@@ -882,7 +882,7 @@ char *option_string(int prot, unsigned int opt, unsigned char *val, int opt_len,
 			 buf[j++] = c;
 		     }
 		    i = l;
-		    if (val[i] != 0 && j < buf_len)
+		    if (i < opt_len && val[i] != 0 && j < buf_len)
 		      buf[j++] = '.';
 		  }
 	      }
@@ -892,24 +892,23 @@ char *option_string(int prot, unsigned int opt, unsigned char *val, int opt_len,
 		unsigned char *p;
 
 		i = 0, j = 0;
-		while (1)
+		while (i + 2 <= opt_len)
 		  {
 		    p = &val[i];
 		    GETSHORT(len, p);
+		    if (i + 2 + len > opt_len)
+		      break; /* malformed: body extends beyond option */
 		    for (k = 0; k < len && j < buf_len; k++)
 		      {
 		       char c = *p++;
 		       if (isprint((unsigned char)c))
 			 buf[j++] = c;
 		     }
-		    i += len +2;
-		    if (i >= opt_len)
-		      break;
-
-		    if (j < buf_len)
+		    i += len + 2;
+		    if (i < opt_len && j < buf_len)
 		      buf[j++] = ',';
 		  }
-	      }	      
+	      }
 #endif
 	    else if ((ot[o].size & (OT_DEC | OT_TIME)) && opt_len != 0)
 	      {

src/dnsmasq.c

@@ -1385,8 +1385,6 @@ static void sig_handler(int sig)
 		  read_write(daemon->pipe_to_parent, (unsigned char *)(&daemon->forward_to_tcp), sizeof(daemon->forward_to_tcp), RW_WRITE);
 		  read_write(daemon->pipe_to_parent, (unsigned char *)(&daemon->forward_to_tcp->uid), sizeof(daemon->forward_to_tcp->uid), RW_WRITE);
 
-		  my_syslog(LOG_INFO, _("TCP process for DNSSEC validation timed out"));
-
 		  _exit(0);
 		}
 	    }

src/domain.c

@@ -175,8 +175,8 @@ int is_rev_synth(int flag, union all_addr *addr, char *name)
 	       *p = '-';
 	 }
        
-       strncat(name, ".", MAXDNAME);
-       strncat(name, c->domain, MAXDNAME);
+       strncat(name, ".", MAXDNAME - strlen(name) - 1);
+       strncat(name, c->domain, MAXDNAME - strlen(name) - 1);
 
        return 1;
      }
@@ -201,16 +201,16 @@ int is_rev_synth(int flag, union all_addr *addr, char *name)
 	   for (i = 0; i < 16; i += 2)
 	     {
 	       sprintf(frag, "%s%02x%02x",  i == 0 ? "" : "-", addr->addr6.s6_addr[i], addr->addr6.s6_addr[i+1]);
-	       strncat(name, frag, MAXDNAME);
+	       strncat(name, frag, MAXDNAME - strlen(name) - 1);
 	     }
 	 }
 
-       strncat(name, ".", MAXDNAME);
-       strncat(name, c->domain, MAXDNAME);
-       
+       strncat(name, ".", MAXDNAME - strlen(name) - 1);
+       strncat(name, c->domain, MAXDNAME - strlen(name) - 1);
+
        return 1;
      }
-   
+
    return 0;
 }
 

src/dump.c

@@ -85,7 +85,8 @@ void dump_init(void)
       /* count existing records */
       while (read_write(daemon->dumpfd, (void *)&pcap_header, sizeof(pcap_header), RW_READ))
 	{
-	  lseek(daemon->dumpfd, pcap_header.incl_len, SEEK_CUR);
+	  if (lseek(daemon->dumpfd, pcap_header.incl_len, SEEK_CUR) == (off_t)-1)
+	    break;
 	  packet_count++;
 	}
     }

src/edns0.c

@@ -469,6 +469,8 @@ int check_source(struct dns_header *header, size_t plen, unsigned char *pseudohe
      {
        GETSHORT(code, p);
        GETSHORT(len, p);
+       if (i + 4 + len > rdlen)
+	 break; /* malformed: option body extends beyond RDATA */
        if (code == EDNS0_OPTION_CLIENT_SUBNET)
 	 {
 	   if (peer)

src/forward.c

@@ -1205,10 +1205,10 @@ void reply_query(int fd, time_t now)
 	  server = daemon->serverarray[serv];
 	  if (server->sfd && server->sfd->fd == fd)
 	    break;
-
-	  if (serv == last)
-	    return;
 	}
+
+      if (serv == last)
+	return;
     }
   
   /* spoof check: answer must come from known server, also
@@ -2175,9 +2175,12 @@ static ssize_t tcp_talk(int first, int last, int start, struct dns_header *heade
 	     someone might be attempting to insert bogus values into the cache by 
 	     sending replies containing questions and bogus answers.
 	     Try another server, or give up */
-	  p = (unsigned char *)(((struct dns_header *)recvbuff->iov_base)+1);
-	  if (extract_name(((struct dns_header *)recvbuff->iov_base), rsize, &p, daemon->namebuff, EXTR_NAME_COMPARE, 4) != 1)
+	  struct dns_header *header = (struct dns_header *)recvbuff->iov_base;
+	  p = (unsigned char *)(header+1);
+	  if (rsize < (unsigned int)sizeof(struct dns_header) || !(header->hb3 & HB3_QR) || ntohs(header->qdcount) != 1 ||
+	      extract_name(header, rsize, &p, daemon->namebuff, EXTR_NAME_COMPARE, 4) != 1)
 	    continue;
+	  
 	  GETSHORT(rtype, p); 
 	  GETSHORT(rclass, p);
       
@@ -2465,18 +2468,18 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff,
 	      !read_write(confd, (unsigned char *)daemon->packet, size, RW_READ))
 	    break;
 	  
-	  if (size < (int)sizeof(struct dns_header))
+	  /* header == query */
+	  header = (struct dns_header *)daemon->packet;
+
+	  if (size < (int)sizeof(struct dns_header) || (header->hb3 & HB3_QR))
 	    continue;
 	  
 	  /* Make sure we have a buffer big enough for the largest answer. */
 	  expand_buf(bigbuff, 65536 + MAXDNAME + RRFIXEDSZ);
 	  out_header = bigbuff->iov_base;
 	  
-	  /* header == query */
-	  header = (struct dns_header *)daemon->packet;
-
 	  /* Add edns0 pheader to query */
-	  size = add_edns0_config(header, size, ((unsigned char *) header) + daemon->edns_pktsz, &peer_addr, now, &cacheable);
+	  size = add_edns0_config(header, size, ((unsigned char *) header) + daemon->packet_buff_sz, &peer_addr, now, &cacheable);
 
 	  /* Clear buffer to avoid risk of information disclosure. */
 	  memset(bigbuff->iov_base, 0, bigbuff->iov_len);

src/helper.c

@@ -740,9 +740,11 @@ static unsigned char *grab_extradata_lua(unsigned char *buf, unsigned char *end,
   if (!buf || (buf == end))
     return NULL;
 
-  for (next = buf; *next != 0; next++)
+  for (next = buf; ; next++)
     if (next == end)
       return NULL;
+    else if (*next == 0)
+      break;
   
   if (next != buf)
     {

src/lease.c

@@ -73,7 +73,9 @@ static int read_leases(time_t now, FILE *leasestream)
 		if (lease)
 		  {
 		    opt_len = parse_hex(daemon->packet, (unsigned char *)daemon->packet, 255, NULL, NULL);
-		    
+		    if (opt_len < 0)
+		      continue;
+
 		    if (strcmp(daemon->dhcp_buff3, "vendorclass") == 0)
 		      lease_set_vendorclass(lease, (unsigned char *)daemon->packet, opt_len);
 		    else if (strcmp(daemon->dhcp_buff3, "agent-info") == 0)
@@ -99,6 +101,8 @@ static int read_leases(time_t now, FILE *leasestream)
 	    
 	    
 	    hw_len = parse_hex(daemon->dhcp_buff2, (unsigned char *)daemon->dhcp_buff2, DHCP_CHADDR_MAX, NULL, &hw_type);
+	    if (hw_len < 0)
+	      hw_len = 0;
 	    /* For backwards compatibility, no explicit MAC address type means ether. */
 	    if (hw_type == 0 && hw_len != 0)
 	      hw_type = ARPHRD_ETHER; 
@@ -131,7 +135,11 @@ static int read_leases(time_t now, FILE *leasestream)
 	  die (_("too many stored leases"), NULL, EC_MISC);
 
 	if (strcmp(daemon->packet, "*") != 0)
-	  clid_len = parse_hex(daemon->packet, (unsigned char *)daemon->packet, 255, NULL, NULL);
+	  {
+	    clid_len = parse_hex(daemon->packet, (unsigned char *)daemon->packet, 255, NULL, NULL);
+	    if (clid_len < 0)
+	      clid_len = 0;
+	  }
 	
 	lease_set_hwaddr(lease, (unsigned char *)daemon->dhcp_buff2, (unsigned char *)daemon->packet, 
 			 hw_len, hw_type, clid_len, now, 0);

src/rfc1035.c

@@ -317,7 +317,7 @@ unsigned char *skip_name(unsigned char *ansp, struct dns_header *header, size_t
       else if (label_type == 0x40)
 	{
 	  /* Extended label type */
-	  unsigned int count;
+	  unsigned int count, llen;
 	  
 	  if (!CHECK_LEN(header, ansp, plen, 2))
 	    return NULL;
@@ -328,9 +328,12 @@ unsigned char *skip_name(unsigned char *ansp, struct dns_header *header, size_t
 	  count = *(ansp++); /* Bits in bitstring */
 	  
 	  if (count == 0) /* count == 0 means 256 bits */
-	    ansp += 32;
+	    llen = 32;
 	  else
-	    ansp += ((count-1)>>3)+1;
+	    llen = ((count-1)>>3)+1;
+
+	  if (!ADD_RDLEN(header, ansp, plen, llen))
+	    return NULL;
 	}
       else
 	{ /* label type == 0 Bottom six bits is length */

src/rfc2131.c

@@ -597,7 +597,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
       if (mess->file[0])
 	{
 	  memcpy(daemon->dhcp_buff2, mess->file, sizeof(mess->file));
-	  daemon->dhcp_buff2[sizeof(mess->file) + 1] = 0; /* ensure zero term. */
+	  daemon->dhcp_buff2[sizeof(mess->file)] = 0; /* ensure zero term. */
 	  id.net = (char *)daemon->dhcp_buff2;
 	  id.next = netid;
 	  netid = &id;
@@ -3248,7 +3248,7 @@ unsigned int relay_reply4(struct dhcp_packet *mess, size_t sz, char *arrival_int
 
 	      /* delete agent info before return RFC 3046 para 2.1 */
 	      *opt = OPTION_END;
-	      memset(opt + 1, 0, option_len(opt) + 2);
+	      memset(opt + 1, 0, option_len(opt) + 1);
 	    }
 	}
       else if (mess->giaddr.s_addr == relay->local.addr4.s_addr)

src/rfc3315.c

@@ -2037,9 +2037,15 @@ static void log6_opts(int nest, unsigned int xid, void *start_opts, void *end_op
 	}
       else if (type == OPTION6_STATUS_CODE)
 	{
-	  int len = sprintf(daemon->namebuff, "%u ", opt6_uint(opt, 0, 2));
-	  memcpy(daemon->namebuff + len, opt6_ptr(opt, 2), opt6_len(opt)-2);
-	  daemon->namebuff[len + opt6_len(opt) - 2] = 0;
+	  if (opt6_len(opt) >= 2)
+	    {
+	      int slen = opt6_len(opt) - 2;
+	      int len = sprintf(daemon->namebuff, "%u ", opt6_uint(opt, 0, 2));
+	      if (slen > (MAXDNAME * 2) - len - 1)
+		slen = (MAXDNAME * 2) - len - 1;
+	      memcpy(daemon->namebuff + len, opt6_ptr(opt, 2), slen);
+	      daemon->namebuff[len + slen] = 0;
+	    }
 	  optname = "status";
 	}
       else

src/tftp.c

@@ -545,8 +545,13 @@ static struct tftp_file *check_tftp_fileperm(char *packet, ssize_t *len, char *p
   int fd = -1;
 
   /* trick to ban moving out of the subtree */
-  if (prefix && strstr(namebuff, "/../"))
-    goto perm;
+  if (prefix)
+    {
+      char *suspect = strstr(namebuff, "/..");
+
+      if (suspect && (suspect[3] == '/' || suspect[3] == 0))
+        goto perm;
+    }
   
   if ((fd = open(namebuff, O_RDONLY)) == -1)
     {

src/util.c

@@ -105,8 +105,6 @@ u32 rand32(void)
 
 u64 rand64(void)
 {
-  static int outleft = 0;
-
   if (outleft < 2)
     {
       if (!++in[0]) if (!++in[1]) if (!++in[2]) ++in[3];
@@ -581,6 +579,7 @@ void prettyprint_time(char *buf, unsigned int t)
   else
     {
       unsigned int x, p = 0;
+      buf[0] = '\0';
        if ((x = t/86400))
 	p += sprintf(&buf[p], "%ud", x);
        if ((x = (t/3600)%24))
@@ -991,7 +990,7 @@ int expand_workspace_real(const char *func, unsigned int line, unsigned char ***
   if (!(p = whine_realloc_real("expand_workspace", func, line, *wkspc, new * sizeof(unsigned char *))))
     return 0;
 
-  memset(p+old, 0, new-old);
+  memset(p+old, 0, (new-old) * sizeof(unsigned char *));
   
   *wkspc = p;
   *szp = new;