Fix memory corruption bugs in BSM record parsing

fetch_newgroups_tok(3): clamp group count to AUDIT_MAX_GROUPS before the
loop to prevent a stack buffer overflow when a crafted record specifies
more than 16 groups.

fetch_execarg_tok(3), fetch_execenv_tok(3): add a bounds check at the
top of the string-walking loop to prevent an out-of-bounds read when the
previous string's nul byte is the last byte of the record buffer.

fetch_sock_unix_tok(3): clamp the memchr search length to the number of
bytes remaining in the buffer to prevent an out-of-bounds read on short
tokens. Also clamp slen to sizeof(path) to prevent a one-byte overflow
when no nul byte is found within the path data.

fetch_socket_tok: fix copy-paste error where the remote address was
written into l_addr instead of r_addr.
Previously reported by: @haginara

Define AU_UNIX_PATH_MAX as 108 (the largest sun_path across all
supported platforms) and use it in au_socketunix_t instead of the
hardcoded 104.

Update fetch_sock_unix_tok to derive its search bound from
sizeof(tok->tt.sockunix.path) so cross-platform records from Solaris and
Linux with paths up to 108 bytes parse correctly without truncation.

REF: https://github.com/openbsm/openbsm/pull/87

Reviewed by:	kevans, markj
Differential Revision:	https://reviews.freebsd.org/D56510

contrib/openbsm/bsm/libbsm.h

@@ -585,13 +585,19 @@ typedef struct {
 	u_int32_t	addr;
 } au_socketinet32_t;
 
+/*
+ * Largest sun_path across all supported platforms (Linux and Solaris use 108,
+ * macOS and FreeBSD use 104).
+ */
+#define	AU_UNIX_PATH_MAX	108
+
 /*
  * socket family           2 bytes
- * path                    104 bytes
+ * path                    up to AU_UNIX_PATH_MAX bytes (NUL terminated)
  */
 typedef struct {
 	u_int16_t	family;
-	char		path[104];
+	char		path[AU_UNIX_PATH_MAX];
 } au_socketunix_t;
 
 /*

contrib/openbsm/libbsm/bsm_io.c

@@ -1867,6 +1867,15 @@ fetch_execarg_tok(tokenstr_t *tok, u_char *buf, int len)
 		return (-1);
 
 	for (i = 0; i < tok->tt.execarg.count; i++) {
+		/*
+		 * Make sure that tok->len has not reached the end of the
+		 * buffer. If the previous string's nul byte was the last byte
+		 * in the buffer, the nul accounting below will have set
+		 * tok->len == len, leaving no room for another string.
+		 */
+		if (tok->len >= (u_int32_t)len) {
+			return (-1);
+		}
 		bptr = buf + tok->len;
 		if (i < AUDIT_MAX_ARGS)
 			tok->tt.execarg.text[i] = (char*)bptr;
@@ -1925,6 +1934,15 @@ fetch_execenv_tok(tokenstr_t *tok, u_char *buf, int len)
 		return (-1);
 
 	for (i = 0; i < tok->tt.execenv.count; i++) {
+		/*
+		 * Make sure that tok->len has not reached the end of the
+		 * buffer. If the previous string's nul byte was the last byte
+		 * in the buffer, the nul accounting below will have set
+		 * tok->len == len, leaving no room for another string.
+		 */
+		if (tok->len >= (u_int32_t)len) {
+			return (-1);
+		}
 		bptr = buf + tok->len;
 		if (i < AUDIT_MAX_ENV)
 			tok->tt.execenv.text[i] = (char*)bptr;
@@ -2037,6 +2055,17 @@ fetch_newgroups_tok(tokenstr_t *tok, u_char *buf, int len)
 	if (err)
 		return (-1);
 
+	/*
+	 * grps.list[] is statically sized and set to AUDIT_MAX_GROUPS. If the
+	 * group count specified in the record is greater than this value just
+	 * clamp/truncate it. Silently truncating a malformed record changes
+	 * what was recorded and could mask tampering. However, a precedent
+	 * has been set in fetch_execarg_tok and fetch_execenv_tok which
+	 * truncate the count under similar circumstances.
+	 */
+	if (tok->tt.grps.no > AUDIT_MAX_GROUPS) {
+		tok->tt.grps.no = AUDIT_MAX_GROUPS;
+	}
 	for (i = 0; i<tok->tt.grps.no; i++) {
 		READ_TOKEN_U_INT32(buf, len, tok->tt.grps.list[i], tok->len,
 		    err);
@@ -3197,27 +3226,36 @@ print_sock_inet128_tok(FILE *fp, tokenstr_t *tok, char *del, int oflags)
 
 /*
  * socket family           2 bytes
- * path                    (up to) 104 bytes + NULL (NULL terminated string).
+ * path                    (up to) AU_UNIX_PATH_MAX bytes (NUL terminated)
  */
 static int
 fetch_sock_unix_tok(tokenstr_t *tok, u_char *buf, int len)
 {
+	size_t remaining, search, pathmax;
 	int err = 0;
 	u_char *p;
 	int slen;
 
-
 	READ_TOKEN_U_INT16(buf, len, tok->tt.sockunix.family, tok->len, err);
 	if (err)
 		return (-1);
 
-	/* slen = strnlen((buf + tok->len), 104) + 1; */
-	p = (u_char *)memchr((const void *)(buf + tok->len), '\0', 104);
-	slen = (p ? (int)(p - (buf + tok->len))  : 104) + 1;
+	/*
+	 * Clamp the search to the bytes remaining in the token and the path
+	 * storage size.  Using sizeof(tok->tt.sockunix.path) rather than a
+	 * literal keeps the bound in sync with au_socketunix_t automatically.
+	 */
+	pathmax = sizeof(tok->tt.sockunix.path);
+	remaining = (size_t)(len - (int)tok->len);
+	search = remaining < pathmax ? remaining : pathmax;
+	p = (u_char *)memchr((const void *)(buf + tok->len), '\0', search);
+	slen = (p ? (int)(p - (buf + tok->len)) + 1 : (int)search);
 
 	READ_TOKEN_BYTES(buf, len, tok->tt.sockunix.path, slen, tok->len, err);
 	if (err)
 		return (-1);
+	/* guarantee NUL termination when no NUL was found in the token data */
+	tok->tt.sockunix.path[pathmax - 1] = '\0';
 
 	return (0);
 }
@@ -3278,7 +3316,7 @@ fetch_socket_tok(tokenstr_t *tok, u_char *buf, int len)
 	if (err)
 		return (-1);
 
-	READ_TOKEN_BYTES(buf, len, &tok->tt.socket.l_addr,
+	READ_TOKEN_BYTES(buf, len, &tok->tt.socket.r_addr,
 	    sizeof(tok->tt.socket.r_addr), tok->len, err);
 	if (err)
 		return (-1);

contrib/openbsm/libbsm/bsm_token.c

@@ -1051,7 +1051,7 @@ au_to_socket_ex(u_short so_domain, u_short so_type,
 /*
  * token ID                1 byte
  * socket family           2 bytes
- * path                    (up to) 104 bytes + NULL  (NULL terminated string)
+ * path                    (up to) AU_UNIX_PATH_MAX bytes (NUL terminated)
  */
 token_t *
 au_to_sock_unix(struct sockaddr_un *so)