Date: Thu, 23 Apr 2026 20:29:03 +0000 From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Cc: Christian S.J. Peron <csjp@FreeBSD.org> Subject: git: a46205a100b3 - main - Fix memory corruption bugs in BSM record parsing Message-ID: <69ea810f.236d5.50248d4c@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=a46205a100b3201a60aaca26e4ac8097e1e136a7 commit a46205a100b3201a60aaca26e4ac8097e1e136a7 Author: Christian S.J. Peron <csjp@FreeBSD.org> AuthorDate: 2026-04-23 20:26:50 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2026-04-23 20:26:50 +0000 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 | 10 ++++++-- contrib/openbsm/libbsm/bsm_io.c | 50 +++++++++++++++++++++++++++++++++----- contrib/openbsm/libbsm/bsm_token.c | 2 +- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/contrib/openbsm/bsm/libbsm.h b/contrib/openbsm/bsm/libbsm.h index 2614a84f01e2..f0ea3943254d 100644 --- a/contrib/openbsm/bsm/libbsm.h +++ b/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; /* diff --git a/contrib/openbsm/libbsm/bsm_io.c b/contrib/openbsm/libbsm/bsm_io.c index f0b3e4a1a2df..323f4ed337f8 100644 --- a/contrib/openbsm/libbsm/bsm_io.c +++ b/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); diff --git a/contrib/openbsm/libbsm/bsm_token.c b/contrib/openbsm/libbsm/bsm_token.c index 682836f43870..8f55945cb33b 100644 --- a/contrib/openbsm/libbsm/bsm_token.c +++ b/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)home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?69ea810f.236d5.50248d4c>