linux-kernel - [PATCH 1/2] apparmor: Check buffer bounds when mapping permissions mask

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1530854701-7348-2-git-send-email-tyhicks@canonical.com>
Date:   Fri,  6 Jul 2018 05:25:00 +0000
From:   Tyler Hicks <tyhicks@...onical.com>
To:     John Johansen <john.johansen@...onical.com>
Cc:     James Morris <jmorris@...ei.org>, Serge Hallyn <serge@...lyn.com>,
        Seth Arnold <seth.arnold@...onical.com>,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH 1/2] apparmor: Check buffer bounds when mapping permissions mask

Don't read past the end of the buffer containing permissions
characters or write past the end of the destination string.

Detected by CoverityScan CID#1415361, 1415376 ("Out-of-bounds access")

Fixes: e53cfe6c7caa ("apparmor: rework perm mapping to a slightly broader set")
Signed-off-by: Tyler Hicks <tyhicks@...onical.com>
---
 security/apparmor/file.c          |  3 ++-
 security/apparmor/include/perms.h |  3 ++-
 security/apparmor/lib.c           | 17 +++++++++++++----
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 224b2fef93ca..4285943f7260 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -47,7 +47,8 @@ static void audit_file_mask(struct audit_buffer *ab, u32 mask)
 {
 	char str[10];
 
-	aa_perm_mask_to_str(str, aa_file_perm_chrs, map_mask_to_chr_mask(mask));
+	aa_perm_mask_to_str(str, sizeof(str), aa_file_perm_chrs,
+			    map_mask_to_chr_mask(mask));
 	audit_log_string(ab, str);
 }
 
diff --git a/security/apparmor/include/perms.h b/security/apparmor/include/perms.h
index 38aa6247d00f..b94ec114d1a4 100644
--- a/security/apparmor/include/perms.h
+++ b/security/apparmor/include/perms.h
@@ -137,7 +137,8 @@ extern struct aa_perms allperms;
 	xcheck(fn_for_each((L1), (P), (FN1)), fn_for_each((L2), (P), (FN2)))
 
 
-void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask);
+void aa_perm_mask_to_str(char *str, size_t str_size, const char *chrs,
+			 u32 mask);
 void aa_audit_perm_names(struct audit_buffer *ab, const char * const *names,
 			 u32 mask);
 void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs,
diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index a7b3f681b80e..7ab368c3789b 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -198,15 +198,24 @@ const char *aa_file_perm_names[] = {
 /**
  * aa_perm_mask_to_str - convert a perm mask to its short string
  * @str: character buffer to store string in (at least 10 characters)
+ * @str_size: size of the @str buffer
+ * @chrs: NUL-terminated character buffer of permission characters
  * @mask: permission mask to convert
  */
-void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask)
+void aa_perm_mask_to_str(char *str, size_t str_size, const char *chrs, u32 mask)
 {
 	unsigned int i, perm = 1;
+	size_t num_chrs = strlen(chrs);
+
+	for (i = 0; i < num_chrs; perm <<= 1, i++) {
+		if (mask & perm) {
+			/* Ensure that one byte is left for NUL-termination */
+			if (WARN_ON_ONCE(str_size <= 1))
+				continue;
 
-	for (i = 0; i < 32; perm <<= 1, i++) {
-		if (mask & perm)
 			*str++ = chrs[i];
+			str_size--;
+		}
 	}
 	*str = '\0';
 }
@@ -236,7 +245,7 @@ void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs,
 
 	audit_log_format(ab, "\"");
 	if ((mask & chrsmask) && chrs) {
-		aa_perm_mask_to_str(str, chrs, mask & chrsmask);
+		aa_perm_mask_to_str(str, sizeof(str), chrs, mask & chrsmask);
 		mask &= ~chrsmask;
 		audit_log_format(ab, "%s", str);
 		if (mask & namesmask)
-- 
2.7.4