audit_log_n_string() computes new_len as "slen + 3" (enclosing quotes
plus the NUL terminator) and stores it into an int, while slen is a
size_t. For a sufficiently large slen the addition can overflow and/or
the result be truncated when assigned to the int new_len, so the
"new_len > avail" check can be bypassed and the subsequent
memcpy(ptr, string, slen) can write past the skb tail.
This is the same class of bug that was fixed for the hex sibling in
commit 65dfde57d1e2 ("audit: fix potential integer overflow in
audit_log_n_hex()"); both helpers are reached through
audit_log_n_untrustedstring() with the same length source.
Make new_len a size_t and use check_add_overflow() to catch the
overflow, mirroring the audit_log_n_hex() fix. No functional change for
the in-tree callers, which all pass bounded lengths.
Fixes: 168b7173959f ("AUDIT: Clean up logging of untrusted strings")
Cc: [email protected]
Signed-off-by: Zhan Xusheng <[email protected]>
---
kernel/audit.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 562476937fa7..547ae0cebec9 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2120,7 +2120,8 @@ void audit_log_n_hex(struct audit_buffer *ab, const
unsigned char *buf,
void audit_log_n_string(struct audit_buffer *ab, const char *string,
size_t slen)
{
- int avail, new_len;
+ int avail;
+ size_t new_len;
unsigned char *ptr;
struct sk_buff *skb;
@@ -2130,7 +2131,13 @@ void audit_log_n_string(struct audit_buffer *ab, const
char *string,
BUG_ON(!ab->skb);
skb = ab->skb;
avail = skb_tailroom(skb);
- new_len = slen + 3; /* enclosing quotes + null terminator */
+
+ /* enclosing quotes + null terminator */
+ if (check_add_overflow(slen, (size_t)3, &new_len)) {
+ audit_log_format(ab, "\"?\"");
+ return;
+ }
+
if (new_len > avail) {
avail = audit_expand(ab, new_len);
if (!avail)
--
2.43.0