Skip to content

Commit b73c1df

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 853ce31 commit b73c1df

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
@@ -408,13 +408,18 @@ aa_state_t aa_dfa_match_len(struct aa_dfa *dfa, aa_state_t start,
408408
if (dfa->tables[YYTD_ID_EC]) {
409409
/* Equivalence class table defined */
410410
u8 *equiv = EQUIV_TABLE(dfa);
411-
for (; len; len--)
412-
match_char(state, def, base, next, check,
413-
equiv[(u8) *str++]);
411+
for (; len; len--) {
412+
u8 c = equiv[(u8) *str];
413+
414+
match_char(state, def, base, next, check, c);
415+
str++;
416+
}
414417
} else {
415418
/* default is direct to next state */
416-
for (; len; len--)
417-
match_char(state, def, base, next, check, (u8) *str++);
419+
for (; len; len--) {
420+
match_char(state, def, base, next, check, (u8) *str);
421+
str++;
422+
}
418423
}
419424

420425
return state;
@@ -448,13 +453,18 @@ aa_state_t aa_dfa_match(struct aa_dfa *dfa, aa_state_t start, const char *str)
448453
/* Equivalence class table defined */
449454
u8 *equiv = EQUIV_TABLE(dfa);
450455
/* default is direct to next state */
451-
while (*str)
452-
match_char(state, def, base, next, check,
453-
equiv[(u8) *str++]);
456+
while (*str) {
457+
u8 c = equiv[(u8) *str];
458+
459+
match_char(state, def, base, next, check, c);
460+
str++;
461+
}
454462
} else {
455463
/* default is direct to next state */
456-
while (*str)
457-
match_char(state, def, base, next, check, (u8) *str++);
464+
while (*str) {
465+
match_char(state, def, base, next, check, (u8) *str);
466+
str++;
467+
}
458468
}
459469

460470
return state;

0 commit comments

Comments
 (0)