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

[Tyler Hicks]

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 <tyhi...@canonical.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