[PATCH] apparmor: fix out-of-bounds read in unix socket address audit

[PATCH] apparmor: fix out-of-bounds read in unix socket address audit

[Henry Martin]

audit_unix_addr() logs pathname unix addresses with
audit_log_untrustedstring(), which sizes the string with strlen().
The sockaddr passed to the LSM hook is not NUL-terminated:
__sys_bind() copies only addrlen bytes from userspace into an
on-stack buffer and leaves the tail uninitialized. A pathname
address without a NUL byte within addrlen makes strlen() run past
the stack object, and adjacent kernel stack contents end up in the
audit record. E.g. on mainline (05441d697f7f, KASAN build):

    BUG: KASAN: stack-out-of-bounds in strlen+0x93/0xa0
    Read of size 1 at addr ffffc9000287fe98 by task poc/1004
    CPU: 3 UID: 1000 PID: 1004 Comm: poc
    Call Trace:
     strlen+0x93/0xa0
     audit_log_untrustedstring+0x19/0x30
     audit_net_cb+0x3cb/0x760
     aa_check_perms+0x19d/0x340
     aa_unix_bind_perm+0x73a/0xb60
     security_socket_bind+0x2c/0x60
     __sys_bind+0x103/0x200

Bound the string by the address length with strnlen(), matching
the abstract-address branch.

This issue was discovered by Tencent CodeBuddy Security.

Cc: stable@vger.kernel.org
Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
 security/apparmor/net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/apparmor/net.c b/security/apparmor/net.c
index 136d14226b97d..cc8dcc5bd619c 100644
--- a/security/apparmor/net.c
+++ b/security/apparmor/net.c
@@ -86,7 +86,8 @@ static void audit_unix_addr(struct audit_buffer *ab, const char *str,
 		audit_log_format(ab, " %s=none", str);
 	} else if (addr->sun_path[0]) {
 		audit_log_format(ab, " %s=", str);
-		audit_log_untrustedstring(ab, addr->sun_path);
+		audit_log_n_untrustedstring(ab, addr->sun_path,
+					    strnlen(addr->sun_path, len));
 	} else {
 		audit_log_format(ab, " %s=\"@", str);
 		if (audit_string_contains_control(&addr->sun_path[1], len - 1))
-- 
2.43.7

Re: [PATCH] apparmor: fix out-of-bounds read in unix socket address audit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] apparmor: fortify_panic() in audit_unix_addr() on 108-byte unix
  path
--

commit d5771717eed480bc696396976af119ef6974188d
Author: Henry Martin <bsdhenrymartin@gmail.com>

apparmor: fix out-of-bounds read in unix socket address audit

This commit replaces audit_log_untrustedstring() with
audit_log_n_untrustedstring() and strnlen() when logging pathname AF_UNIX
socket addresses in audit_unix_addr().

> diff --git a/security/apparmor/net.c b/security/apparmor/net.c
> index a333e6aff9262..10f888c83b897 100644
> --- a/security/apparmor/net.c
> +++ b/security/apparmor/net.c
> @@ -85,7 +85,8 @@ static void audit_unix_addr(struct audit_buffer *ab, const char *str,
>  		audit_log_format(ab, " %s=none", str);
>  	} else if (addr->sun_path[0]) {
>  		audit_log_format(ab, " %s=", str);
> -		audit_log_untrustedstring(ab, addr->sun_path);
> +		audit_log_n_untrustedstring(ab, addr->sun_path,
> +					    strnlen(addr->sun_path, len));

[Severity: High]
Can strnlen(addr->sun_path, len) trigger a fortify_panic() when auditing a
socket bound to a 108-byte (UNIX_PATH_MAX) filesystem pathname?

For a 108-byte pathname, unix_mkname_bsd() writes a trailing NUL byte one
byte past sun_path[107] and returns 111 (108 + offsetof(struct sockaddr_un,
sun_path) + 1), which unix_create_addr() stores in u->addr->len.  When
audit_unix_sk_addr() later passes u->addr->name and u->addr->len to
audit_unix_addr(), len is unix_addr_len(111) = 109.

With CONFIG_FORTIFY_SOURCE=y, strnlen(addr->sun_path, 109) sees p_size =
sizeof(addr->sun_path) = 108, maxlen = 109, and ret = 108, which satisfies
(p_size <= ret && maxlen != ret) and calls fortify_panic().  Clamping the
bound to min_t(size_t, len, sizeof(addr->sun_path)) avoids passing a
maxlen larger than the sun_path array.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106185532.3338123-1-bsdhenrymartin@gmail.com?part=1