Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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@canonical.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>

authored by

Tyler Hicks and committed by
John Johansen
7f3ebcf2 fb7d1bcf

+17 -6
+2 -1
security/apparmor/file.c
··· 47 47 { 48 48 char str[10]; 49 49 50 - aa_perm_mask_to_str(str, aa_file_perm_chrs, map_mask_to_chr_mask(mask)); 50 + aa_perm_mask_to_str(str, sizeof(str), aa_file_perm_chrs, 51 + map_mask_to_chr_mask(mask)); 51 52 audit_log_string(ab, str); 52 53 } 53 54
+2 -1
security/apparmor/include/perms.h
··· 137 137 xcheck(fn_for_each((L1), (P), (FN1)), fn_for_each((L2), (P), (FN2))) 138 138 139 139 140 - void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask); 140 + void aa_perm_mask_to_str(char *str, size_t str_size, const char *chrs, 141 + u32 mask); 141 142 void aa_audit_perm_names(struct audit_buffer *ab, const char * const *names, 142 143 u32 mask); 143 144 void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs,
+13 -4
security/apparmor/lib.c
··· 198 198 /** 199 199 * aa_perm_mask_to_str - convert a perm mask to its short string 200 200 * @str: character buffer to store string in (at least 10 characters) 201 + * @str_size: size of the @str buffer 202 + * @chrs: NUL-terminated character buffer of permission characters 201 203 * @mask: permission mask to convert 202 204 */ 203 - void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask) 205 + void aa_perm_mask_to_str(char *str, size_t str_size, const char *chrs, u32 mask) 204 206 { 205 207 unsigned int i, perm = 1; 208 + size_t num_chrs = strlen(chrs); 206 209 207 - for (i = 0; i < 32; perm <<= 1, i++) { 208 - if (mask & perm) 210 + for (i = 0; i < num_chrs; perm <<= 1, i++) { 211 + if (mask & perm) { 212 + /* Ensure that one byte is left for NUL-termination */ 213 + if (WARN_ON_ONCE(str_size <= 1)) 214 + break; 215 + 209 216 *str++ = chrs[i]; 217 + str_size--; 218 + } 210 219 } 211 220 *str = '\0'; 212 221 } ··· 245 236 246 237 audit_log_format(ab, "\""); 247 238 if ((mask & chrsmask) && chrs) { 248 - aa_perm_mask_to_str(str, chrs, mask & chrsmask); 239 + aa_perm_mask_to_str(str, sizeof(str), chrs, mask & chrsmask); 249 240 mask &= ~chrsmask; 250 241 audit_log_format(ab, "%s", str); 251 242 if (mask & namesmask)