audit: fix potential integer overflow in audit_log_n_string()

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.

Cc: stable@vger.kernel.org
Fixes: 168b7173959f ("AUDIT: Clean up logging of untrusted strings")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/kernel/audit.c b/kernel/audit.c
index 562476937fa793def6c7b3e8893a46dc8966a81e..9412af9144bc9a8db057f553a276c4256798d95d 100644 (file)
--- 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, 3, &new_len)) {
+               audit_log_format(ab, "?");
+               return;
+       }
+
        if (new_len > avail) {
                avail = audit_expand(ab, new_len);
                if (!avail)