From 7f3ebcf2b1395e0248e56146041e1e5625fd2f23 Mon Sep 17 00:00:00 2001
From: Tyler Hicks <tyhicks@canonical.com>
Date: Fri Jul 6 05:25:00 2018 +0000
Subject: [PATCH] apparmor: Check buffer bounds when mapping permissions mask
Git-commit: 7f3ebcf2b1395e0248e56146041e1e5625fd2f23
References: git-fixes
Patch-mainline: v4.19-rc1
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@canonical.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
security/apparmor/file.c | 3 ++-
security/apparmor/include/perms.h | 3 ++-
security/apparmor/lib.c | 17 +++++++++++++----
3 files changed, 17 insertions(+), 6 deletions(-)
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -46,7 +46,8 @@ static void audit_file_mask(struct audit
{
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);
}
--- 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 **names, u32 mask);
void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs,
u32 chrsmask, const char **names, u32 namesmask);
--- 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))
+ break;
- for (i = 0; i < 32; perm <<= 1, i++) {
- if (mask & perm)
*str++ = chrs[i];
+ str_size--;
+ }
}
*str = '\0';
}
@@ -235,7 +244,7 @@ void aa_audit_perm_mask(struct audit_buf
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)