Re-implement print_mac() to do its own buffer managment.

print_mac() used to output into caller-supplied buffer with no
length checking. The various callers supplied different
length buffers and did their own, ad-hoc, length checking before
calling print_mac(). This was an accident waiting to happen and
it had actually happened in couple of places, with buffer
overflows possible.

print_mac() now returns the address of its own buffer, which
is grown as required.

Thanks to Nicholas Carlini <npc@anthropic.com> for spotting this problem.

src/dbus.c

@@ -1055,7 +1055,7 @@ void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname)
   DBusConnection *connection = (DBusConnection *)daemon->dbus;
   DBusMessage* message = NULL;
   DBusMessageIter args;
-  char *action_str, *mac = daemon->namebuff;
+  char *action_str, *mac;
   unsigned char *p;
   int i;
 
@@ -1068,7 +1068,7 @@ void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname)
 #ifdef HAVE_DHCP6
    if (lease->flags & (LEASE_TA | LEASE_NA))
      {
-       print_mac(mac, lease->clid, lease->clid_len);
+       mac = print_mac(lease->clid, lease->clid_len);
        inet_ntop(AF_INET6, &lease->addr6, daemon->addrbuff, ADDRSTRLEN);
      }
    else
@@ -1076,7 +1076,7 @@ void emit_dbus_signal(int action, struct dhcp_lease *lease, char *hostname)
      {
        p = extended_hwaddr(lease->hwaddr_type, lease->hwaddr_len,
 			   lease->hwaddr, lease->clid_len, lease->clid, &i);
-       print_mac(mac, p, i);
+       mac = print_mac(p, i);
        inet_ntop(AF_INET, &lease->addr, daemon->addrbuff, ADDRSTRLEN);
      }
 

src/dhcp-common.c

@@ -936,11 +936,11 @@ char *option_string(int prot, unsigned int opt, unsigned char *val, int opt_len,
 	  trunc = 1;
 	  opt_len = 14;
 	}
-      print_mac(buf, val, opt_len);
+
+      *buf = 0;
+      strncat(buf, print_mac(val, opt_len), buf_len);
       if (trunc)
 	strncat(buf, "...", buf_len - strlen(buf));
-    
-
     }
 
   return ot[o].name ? ot[o].name : "";

src/dnsmasq.h

@@ -1538,7 +1538,7 @@ int parse_hex(char *in, unsigned char *out, int maxlen,
 	      unsigned int *wildcard_mask, int *mac_type);
 int memcmp_masked(unsigned char *a, unsigned char *b, int len, 
 		  unsigned int mask);
-char *print_mac(char *buff, unsigned char *mac, int len);
+char *print_mac(unsigned char *mac, int len);
 int read_write(int fd, unsigned char *packet, int size, int rw);
 int read_writev(int fd, struct iovec *iov, int iovcnt, int rw);
 void close_fds(long max_fd, int spare1, int spare2, int spare3);

src/edns0.c

@@ -277,14 +277,16 @@ 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 */
+   OPT_STRIP_MAC = MAC is removed
+   Handle only six byte MACs. */
 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;
   unsigned char mac[DHCP_CHADDR_MAX];
-  char encode[18]; /* handle 6 byte MACs ONLY */
-
+  char encode[9] = { 0 };
+  char *encoded = encode;
+  
   if ((option_bool(OPT_MAC_B64) || option_bool(OPT_MAC_HEX)) && (maclen = find_mac(l3, mac, 1, now)) == 6)
     {
       if (option_bool(OPT_STRIP_MAC))
@@ -292,7 +294,7 @@ static size_t add_dns_client(struct dns_header *header, size_t plen, size_t outl
        *cacheablep = 0;
     
        if (option_bool(OPT_MAC_HEX))
-	 print_mac(encode, mac, maclen);
+	 encoded = print_mac(mac, maclen);
        else
 	 {
 	   encoder(mac, encode);
@@ -304,7 +306,7 @@ static size_t add_dns_client(struct dns_header *header, size_t plen, size_t outl
     replace = 2;
 
   if (replace != 0 || maclen == 6)
-    plen = add_pseudoheader(header, plen, outlen, EDNS0_OPTION_NOMDEVICEID, (unsigned char *)encode, strlen(encode), 0, replace);
+    plen = add_pseudoheader(header, plen, outlen, EDNS0_OPTION_NOMDEVICEID, (unsigned char *)encoded, strlen(encoded), 0, replace);
 
   return plen;
 }

src/radv.c

@@ -215,11 +215,8 @@ void icmp6_packet(time_t now)
 	  if (opt_sz == 0 || opt_sz > rem)
 	    return; /* Bad packet */
 	  
-	  if (p[0] == ICMP6_OPT_SOURCE_MAC && ((opt_sz - 2) * 3 - 1 < MAXDNAMESTR))
-	    {
-	      print_mac(daemon->namebuff, &p[2], opt_sz - 2);
-	      mac = daemon->namebuff;
-	    }
+	  if (p[0] == ICMP6_OPT_SOURCE_MAC)
+	    mac = print_mac(&p[2], opt_sz - 2);
 	}
       
       if (!option_bool(OPT_QUIET_RA))

src/rfc2131.c

@@ -1313,7 +1313,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 		  unsigned char *mac = extended_hwaddr(ltmp->hwaddr_type, ltmp->hwaddr_len,
 						       ltmp->hwaddr, ltmp->clid_len, ltmp->clid, &len);
 		  my_syslog(MS_DHCP | LOG_WARNING, _("not using configured address %s because it is leased to %s"),
-			    daemon->addrbuff, print_mac(daemon->namebuff, mac, len));
+			    daemon->addrbuff, print_mac(mac, len));
 		}
 	      else
 		{
@@ -1537,7 +1537,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 		{
 		  inet_ntop(AF_INET, &ltmp->addr, daemon->addrbuff, ADDRSTRLEN);
 		  my_syslog(MS_DHCP | LOG_INFO, _("abandoning lease to %s of %s"),
-			    print_mac(daemon->namebuff, ltmp->hwaddr, ltmp->hwaddr_len), 
+			    print_mac(ltmp->hwaddr, ltmp->hwaddr_len), 
 			    daemon->addrbuff);
 		  lease = ltmp;
 		}
@@ -1918,16 +1918,18 @@ static void add_extradata_opt(struct dhcp_lease *lease, unsigned char *opt)
 static void log_packet(char *type, void *addr, unsigned char *ext_mac, 
 		       int mac_len, char *interface, char *string, char *err, u32 xid)
 {
+  char *mac = "";
+
   if (!err && !option_bool(OPT_LOG_OPTS) && option_bool(OPT_QUIET_DHCP))
     return;
   
-  daemon->addrbuff[0] = daemon->namebuff[0] = 0;
+  daemon->addrbuff[0] = 0;
   
   if (addr)
     inet_ntop(AF_INET, addr, daemon->addrbuff, ADDRSTRLEN);
   
   if (ext_mac)
-    print_mac(daemon->namebuff, ext_mac, mac_len);
+    mac = print_mac(ext_mac, mac_len);
   
   if (option_bool(OPT_LOG_OPTS))
     my_syslog(MS_DHCP | LOG_INFO, "%u %s(%s) %s%s%s%s%s%s",
@@ -1936,7 +1938,7 @@ static void log_packet(char *type, void *addr, unsigned char *ext_mac,
 	      interface, 
 	      daemon->addrbuff,
 	      addr ? " " : "",
-	      daemon->namebuff,
+	      mac,
 	      ext_mac ? " " : "",
 	      string ? string : "",
 	      err ? err : "");
@@ -1946,16 +1948,16 @@ static void log_packet(char *type, void *addr, unsigned char *ext_mac,
 	      interface, 
 	      daemon->addrbuff,
 	      addr ? " " : "",
-	      daemon->namebuff,
+	      mac,
 	      ext_mac ? " " : "",
 	      string ? string : "",
 	      err ? err : "");
   
 #ifdef HAVE_UBUS
   if (!strcmp(type, "DHCPACK"))
-    ubus_event_bcast("dhcp.ack", daemon->namebuff, addr ? daemon->addrbuff : NULL, string, interface);
+    ubus_event_bcast("dhcp.ack", mac, addr ? daemon->addrbuff : NULL, string, interface);
   else if (!strcmp(type, "DHCPRELEASE"))
-    ubus_event_bcast("dhcp.release", daemon->namebuff, addr ? daemon->addrbuff : NULL, string, interface);
+    ubus_event_bcast("dhcp.release", mac, addr ? daemon->addrbuff : NULL, string, interface);
 #endif
 }
 

src/rfc3315.c

@@ -474,10 +474,8 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu
   if (state->mac_len != 0)
     {
       if (option_bool(OPT_LOG_OPTS))
-	{
-	  print_mac(daemon->dhcp_buff, state->mac, state->mac_len);
-	  my_syslog(MS_DHCP | LOG_INFO, _("%u client MAC address: %s"), state->xid, daemon->dhcp_buff);
-	}
+	my_syslog(MS_DHCP | LOG_INFO, _("%u client MAC address: %s"), state->xid,
+		  print_mac(state->mac, state->mac_len));
 
       for (mac_opt = daemon->dhcp_macs; mac_opt; mac_opt = mac_opt->next)
 	if ((unsigned)mac_opt->hwaddr_len == state->mac_len &&
@@ -2077,14 +2075,6 @@ static void log6_quiet(struct state *state, char *type, struct in6_addr *addr, c
 
 static void log6_packet(struct state *state, char *type, struct in6_addr *addr, char *string)
 {
-  int clid_len = state->clid_len;
-
-  /* avoid buffer overflow */
-  if (clid_len > 100)
-    clid_len = 100;
-  
-  print_mac(daemon->namebuff, state->clid, clid_len);
-
   if (addr)
     {
       inet_ntop(AF_INET6, addr, daemon->dhcp_buff2, DHCP_BUFF_SZ - 1);
@@ -2099,14 +2089,14 @@ static void log6_packet(struct state *state, char *type, struct in6_addr *addr,
 	      type,
 	      state->iface_name, 
 	      daemon->dhcp_buff2,
-	      daemon->namebuff,
+	      print_mac(state->clid, state->clid_len),
 	      string ? string : "");
   else
     my_syslog(MS_DHCP | LOG_INFO, "%s(%s) %s%s %s",
 	      type,
 	      state->iface_name, 
 	      daemon->dhcp_buff2,
-	      daemon->namebuff,
+	      print_mac(state->clid, state->clid_len),
 	      string ? string : "");
 }
 

src/util.c

@@ -692,18 +692,24 @@ int memcmp_masked(unsigned char *a, unsigned char *b, int len, unsigned int mask
   return count;
 }
 
-char *print_mac(char *buff, unsigned char *mac, int len)
+char *print_mac(unsigned char *mac, int len)
 {
-  char *p = buff;
-  int i;
-   
-  if (len == 0)
-    sprintf(p, "<null>");
-  else
-    for (i = 0; i < len; i++)
-      p += sprintf(p, "%.2x%s", mac[i], (i == len - 1) ? "" : ":");
+  static struct iovec buff = { NULL, 0 };
   
-  return buff;
+  /* each byte is two digits plus ':' except that last
+     which is two digits plus terminator. */
+  if (len == 0 || !expand_buf(&buff, len*3))
+    return "<null>";
+  else
+    {
+      char *p =  buff.iov_base;
+      int i;
+
+      for (i = 0; i < len; i++)
+	p += sprintf(p, "%.2x%s", mac[i], (i == len - 1) ? "" : ":");
+    }
+  
+  return buff.iov_base;
 }
 
 /* rc is return from sendto and friends.