Fix crash with empty DHCP SNAME option.

dhcp-option=66,

sets the servername to nothing, and causes a segfault at DHCP
transaction time. There may be other ways to provoke this, for
instance by using an empty filename.

The patch make safe_strncpy() even safer, by handing a NULL src argument.

Thanks to Jeff Allen for spotting and reporting this.

src/auth.c

@@ -591,7 +591,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
   if (auth && zone)
     {
       char *authname;
-      int newoffset, offset = 0;
+      int newoffset = ansp - (unsigned char *)header, offset = 0;
 
       if (!subnet)
 	authname = zone->domain;
@@ -631,8 +631,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	}
       
       /* handle NS and SOA in auth section or for explicit queries */
-       newoffset = ansp - (unsigned char *)header;
-       if (((anscount == 0 && !ns) || soa) &&
+      if (((anscount == 0 && !ns) || soa) &&
 	  add_resource_record(header, limit, &trunc, 0, &ansp, 
 			      daemon->auth_ttl, NULL, T_SOA, C_IN, "ddlllll",
 			      authname, daemon->authserver,  daemon->hostmaster,
@@ -650,11 +649,10 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
       if (anscount != 0 || ns)
 	{
 	  struct name_list *secondary;
-	  
+	 	  
 	  /* Only include the machine running dnsmasq if it's acting as an auth server */
 	  if (daemon->authinterface)
 	    {
-	      newoffset = ansp - (unsigned char *)header;
 	      if (add_resource_record(header, limit, &trunc, -offset, &ansp, 
 				      daemon->auth_ttl, NULL, T_NS, C_IN, "d", offset == 0 ? authname : NULL, daemon->authserver))
 		{
@@ -669,9 +667,11 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 
 	  if (!subnet)
 	    for (secondary = daemon->secondary_forward_server; secondary; secondary = secondary->next)
-	      if (add_resource_record(header, limit, &trunc, offset, &ansp, 
-				      daemon->auth_ttl, NULL, T_NS, C_IN, "d", secondary->name))
+	      if (add_resource_record(header, limit, &trunc, -offset, &ansp, 
+				      daemon->auth_ttl, NULL, T_NS, C_IN, "d", offset == 0 ? authname : NULL, secondary->name))
 		{
+		  if (offset == 0) 
+		    offset = newoffset;
 		  if (ns) 
 		    anscount++;
 		  else

src/cache.c

@@ -1600,7 +1600,7 @@ void cache_reload(void)
   
 #ifdef HAVE_DNSSEC
   for (ds = daemon->ds; ds; ds = ds->next)
-    if ((cache = get_config_crec()))
+    if (ds->name && (cache = get_config_crec()))
       {
 	
 	if (!(cache->addr.ds.keydata = blockdata_alloc(ds->digest, ds->digestlen)))

src/dnsmasq.c

@@ -946,9 +946,27 @@ int main (int argc, char **argv)
 	my_syslog(LOG_INFO, _("DNSSEC signature timestamps not checked until system time valid"));
 
       for (ds = daemon->ds; ds; ds = ds->next)
-	my_syslog(LOG_INFO,
-		  ds->digestlen == 0 ? _("configured with negative trust anchor for %s") : _("configured with trust anchor for %s keytag %u"),
-		  ds->name[0] == 0 ? "<root>" : ds->name, ds->keytag);
+	{
+	  struct ds_config *ds1;
+	  
+	  for (ds1 = ds->next; ds1; ds1 = ds1->next)
+	    if (strcmp(ds->name, ds1->name) == 0 &&
+		ds->digestlen == ds1->digestlen &&
+		(ds->digestlen == 0 ||
+		 (ds->algo == ds1->algo &&
+		  ds->keytag == ds1->keytag &&
+		  ds->digest_type == ds1->digest_type &&
+		  memcmp(ds->digest, ds1->digest, ds->digestlen) == 0)))
+	      {
+		ds->name = NULL; /* Mark as duplicate */
+		break;
+	      }
+	  
+	  if (ds->name)
+	    my_syslog(LOG_INFO,
+		      ds->digestlen == 0 ? _("configured with negative trust anchor for %s") : _("configured with trust anchor for %s keytag %u"),
+		      ds->name[0] == 0 ? "<root>" : ds->name, ds->keytag);
+	}
     }
 #endif
 

src/dnsmasq.h

@@ -1497,11 +1497,13 @@ unsigned char *do_rfc1035_name(unsigned char *p, char *sval, char *limit);
 void *safe_malloc(size_t size);
 void safe_strncpy(char *dest, const char *src, size_t size);
 void safe_pipe(int *fd, int read_noblock);
+#define malloc_log(x, y) malloc_log_real(__func__, __LINE__, (x), (y))
 #define whine_malloc(x) whine_malloc_real(__func__, __LINE__, (x))
 #define whine_realloc(x, y) whine_realloc_real(NULL, __func__, __LINE__, (x), (y))
 #define expand_buf(x, y) expand_buf_real(__func__, __LINE__, (x), (y))
 #define expand_workspace(x, y, z) expand_workspace_real(__func__, __LINE__, (x), (y), (z))
 #define free(x) free_real(__func__, __LINE__, (x))
+void malloc_log_real(const char *func, unsigned int line, void *mem, size_t size);
 void free_real(const char *func, unsigned int line, void *ptr);
 void *whine_malloc_real(const char *func, unsigned int line, size_t size);
 void *whine_realloc_real(const char *wrapper, const char *func, unsigned int line, void *ptr, size_t size);

src/rfc1035.c

@@ -1427,6 +1427,11 @@ int check_for_ignored_address(struct dns_header *header, size_t qlen)
   return check_bad_address(header, qlen, daemon->ignore_addr, NULL, NULL);
 }
 
+/* Nameoffset > 0 means that the name of the new record already exists at the given offset,
+   so use a "jump" to that.
+   Nameoffset == 0 means use the first variable argument as the name of the new record.
+   nameoffset < 0 means use the first variable argument as the start of the new record name,
+   then "jump" to -nameoffset to complete it. */
 int add_resource_record(struct dns_header *header, char *limit, int *truncp, int nameoffset, unsigned char **pp, 
 			unsigned long ttl, int *offset, unsigned short type, unsigned short class, char *format, ...)
 {

src/util.c

@@ -266,6 +266,10 @@ char *canonicalise(char *in, int *nomem)
 	  
 	  return NULL;
 	}
+
+      /* IDN library doesnt call our malloc wrapper, so log this by steam */
+      if (ret)
+	malloc_log(ret, strlen(ret)+1);
       
       return ret;
     }
@@ -323,13 +327,15 @@ void *safe_malloc(size_t size)
 }
 
 /* Ensure limited size string is always terminated.
- * Can be replaced by (void)strlcpy() on some platforms */
+   Can be replaced by (void)strlcpy() on some platforms.
+   src may be NULL in which case we return an empty string. */
 void safe_strncpy(char *dest, const char *src, size_t size)
 {
   if (size != 0)
     {
-      dest[size-1] = '\0';
-      strncpy(dest, src, size-1);
+      dest[0] = dest[size-1] = '\0';
+      if (src)
+	strncpy(dest, src, size-1);
     }
 }
 
@@ -993,6 +999,12 @@ int expand_workspace_real(const char *func, unsigned int line, unsigned char ***
   return 1;
 }
 
+void malloc_log_real(const char *func, unsigned int line, void *mem, size_t size)
+{
+  if (mem && daemon->log_malloc)
+    my_syslog(LOG_INFO, _("malloc: %s:%u %zu bytes at %x"), func, line, size, hash_ptr(mem));
+}
+
 #undef free
 void free_real(const char *func, unsigned int line, void *ptr)
 {