Fix memory corruption bugs in BSM record parsing

[csjp]

Summary

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

[markj]

contrib/openbsm/libbsm/bsm_io.c

3249

IMHO it is a bit strange that len is a signed int here and in the other parsing functions. Shouldn't it be a size_t?

[csjp]

So.. I went down a wild rabbit hole here. len is int here becauase for some
reason we used int for all the fetch_*(3) functions. Upon deeper analysis:

1. The macros cast len to u_int32_t anyway READ_TOKEN_U_INT16 and READ_TOKEN_BYTES both do (u_int32_t)(len) internally, so a signed negative value would just wrap to a large unsigned number and the bounds check would still fail (conservatively).
2. The negative-len guard lives in au_fetch_tok(), the public entry point, which explicitly checks if (len <= 0) return (-1) before dispatching to any fetch_* function. So by the time fetch_sock_unix_tok() is called, len is already guaranteed to be > 0.
3. Line 3401 remaining = (size_t)(len - (int)tok->len) is the only place the sign matters, and a negative result there would wrap to a huge size_t, but it would immediately be clamped by pathmax on the next line in my fix.

That said, it's still cleaner to avoid the signed/unsigned arithmetic entirely.
I thought maybe we got it from the original Solaris/SunOS BSM libary but it
turns out that au_fetch_tok(3) wasn't even part of the original Solaris public
libbsm API (illumos classifies it as private/internal)

But I would agree we could safely switch this to size_t and simplify some code

Best,
Christian S.J. Peron