Skip to content

Commit 0510d1b

Browse files
m-pellizzergregkh
authored andcommitted
apparmor: fix side-effect bug in match_char() macro usage
commit 8756b68 upstream. The match_char() macro evaluates its character parameter multiple times when traversing differential encoding chains. When invoked with *str++, the string pointer advances on each iteration of the inner do-while loop, causing the DFA to check different characters at each iteration and therefore skip input characters. This results in out-of-bounds reads when the pointer advances past the input buffer boundary. [ 94.984676] ================================================================== [ 94.985301] BUG: KASAN: slab-out-of-bounds in aa_dfa_match+0x5ae/0x760 [ 94.985655] Read of size 1 at addr ffff888100342000 by task file/976 [ 94.986319] CPU: 7 UID: 1000 PID: 976 Comm: file Not tainted 6.19.0-rc7-next-20260127 #1 PREEMPT(lazy) [ 94.986322] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 94.986329] Call Trace: [ 94.986341] <TASK> [ 94.986347] dump_stack_lvl+0x5e/0x80 [ 94.986374] print_report+0xc8/0x270 [ 94.986384] ? aa_dfa_match+0x5ae/0x760 [ 94.986388] kasan_report+0x118/0x150 [ 94.986401] ? aa_dfa_match+0x5ae/0x760 [ 94.986405] aa_dfa_match+0x5ae/0x760 [ 94.986408] __aa_path_perm+0x131/0x400 [ 94.986418] aa_path_perm+0x219/0x2f0 [ 94.986424] apparmor_file_open+0x345/0x570 [ 94.986431] security_file_open+0x5c/0x140 [ 94.986442] do_dentry_open+0x2f6/0x1120 [ 94.986450] vfs_open+0x38/0x2b0 [ 94.986453] ? may_open+0x1e2/0x2b0 [ 94.986466] path_openat+0x231b/0x2b30 [ 94.986469] ? __x64_sys_openat+0xf8/0x130 [ 94.986477] do_file_open+0x19d/0x360 [ 94.986487] do_sys_openat2+0x98/0x100 [ 94.986491] __x64_sys_openat+0xf8/0x130 [ 94.986499] do_syscall_64+0x8e/0x660 [ 94.986515] ? count_memcg_events+0x15f/0x3c0 [ 94.986526] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986540] ? handle_mm_fault+0x1639/0x1ef0 [ 94.986551] ? vma_start_read+0xf0/0x320 [ 94.986558] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986561] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986563] ? fpregs_assert_state_consistent+0x50/0xe0 [ 94.986572] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986574] ? arch_exit_to_user_mode_prepare+0x9/0xb0 [ 94.986587] ? srso_alias_return_thunk+0x5/0xfbef5 [ 94.986588] ? irqentry_exit+0x3c/0x590 [ 94.986595] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 94.986597] RIP: 0033:0x7fda4a79c3ea Fix by extracting the character value before invoking match_char, ensuring single evaluation per outer loop. Fixes: 074c1cd ("apparmor: dfa move character match into a macro") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d42b2b6 commit 0510d1b

1 file changed

Lines changed: 20 additions & 10 deletions

File tree

security/apparmor/match.c

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -463,13 +463,18 @@ aa_state_t aa_dfa_match_len(struct aa_dfa *dfa, aa_state_t start,
463463
if (dfa->tables[YYTD_ID_EC]) {
464464
/* Equivalence class table defined */
465465
u8 *equiv = EQUIV_TABLE(dfa);
466-
for (; len; len--)
467-
match_char(state, def, base, next, check,
468-
equiv[(u8) *str++]);
466+
for (; len; len--) {
467+
u8 c = equiv[(u8) *str];
468+
469+
match_char(state, def, base, next, check, c);
470+
str++;
471+
}
469472
} else {
470473
/* default is direct to next state */
471-
for (; len; len--)
472-
match_char(state, def, base, next, check, (u8) *str++);
474+
for (; len; len--) {
475+
match_char(state, def, base, next, check, (u8) *str);
476+
str++;
477+
}
473478
}
474479

475480
return state;
@@ -503,13 +508,18 @@ aa_state_t aa_dfa_match(struct aa_dfa *dfa, aa_state_t start, const char *str)
503508
/* Equivalence class table defined */
504509
u8 *equiv = EQUIV_TABLE(dfa);
505510
/* default is direct to next state */
506-
while (*str)
507-
match_char(state, def, base, next, check,
508-
equiv[(u8) *str++]);
511+
while (*str) {
512+
u8 c = equiv[(u8) *str];
513+
514+
match_char(state, def, base, next, check, c);
515+
str++;
516+
}
509517
} else {
510518
/* default is direct to next state */
511-
while (*str)
512-
match_char(state, def, base, next, check, (u8) *str++);
519+
while (*str) {
520+
match_char(state, def, base, next, check, (u8) *str);
521+
str++;
522+
}
513523
}
514524

515525
return state;

0 commit comments

Comments
 (0)