From: Lucas Gabriel Vuotto <lucas@sexy.is>
Subject: tcpdump/print-lwres vs b64_ntop
To: tech@openbsd.org
Date: Sat, 15 Feb 2025 17:43:00 +0000

Hello,

I friend of mine is learning some C and needed to use b64_ntop, and
somehow managed to pick the worst possible example usage of b64_ntop in
the src tree.

- dbuf is treated as a string, despite b64_ntop being able to deal with
  binary data
- the encoded length arithmetic is wrong (but it's always >= than the
  corrent encoded length), plus there is no consideration for the final
  NUL byte
- the return value isn't checked and it's used to index a malloc'd
  region

The last two items together can make tcpdump print trash or crash when
l is in the form of 3n + 1, because there is no room for the ending NUL
byte and b64_ntop returns -1.

ok?

Also, this lwres protocol thing came from some BIND9, didn't seem to get
much traction (opinion solely based on how difficult was to find
information about it online) and was removed from BIND9 in 2015 iirc.
Does it make sense to keep it at all?


diff refs/heads/master 2aa3c8b385fa4ed07673c5c475eea151239ba52e
commit - 5c4be1f536db495fb94d472fda393a38f88d284e
commit + 2aa3c8b385fa4ed07673c5c475eea151239ba52e
blob - 7d9805012231e55cd24913df35b4cbf09c5f9828
blob + bcac9d71613a4c09b02036d17015f784a924ea03
--- usr.sbin/tcpdump/print-lwres.c
+++ usr.sbin/tcpdump/print-lwres.c
@@ -268,7 +268,7 @@ lwres_printb64len(const char *p0)
 	u_int8_t *p;
 	u_int16_t l;
 	char *dbuf, *b64buf;
-	int i;
+	int i, b64len;
 
 	p = (u_int8_t *)p0;
 	if (p + 2 > (u_int8_t *)snapend)
@@ -277,11 +277,12 @@ lwres_printb64len(const char *p0)
 	if (p + 2 + l > (u_int8_t *)snapend)
 		goto trunc;
 
-	dbuf = malloc(l + 1);
+	dbuf = malloc(l);
 	if (!dbuf)
 	  return -1;
 
-	b64buf = malloc((l + 2) * 4 / 3);
+	b64len = (l + 2) / 3 * 4 + 1;
+	b64buf = malloc(b64len);
 	if (!b64buf)
 	  {
 	    free(dbuf);
@@ -289,15 +290,17 @@ lwres_printb64len(const char *p0)
 	  }
 
 	memcpy(dbuf, p, l);
-	*(dbuf + l) = (char)0;
 
-	i = b64_ntop (dbuf, l, b64buf, (l + 2) * 4 / 3);
-	b64buf[i] = (char)0;
-	printf ("%s", b64buf);
+	i = b64_ntop (dbuf, l, b64buf, b64len);
+	if (i != -1)
+		printf ("%s", b64buf);
 
 	free (dbuf);
 	free (b64buf);
 
+	if (i == -1)
+		return -1;
+
 	return l + 2;
 
   trunc: