mirror of
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-03-22 07:27:12 +08:00
apparmor: fix side-effect bug in match_char() macro usage
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: 074c1cd798 ("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>
This commit is contained in:
committed by
John Johansen
parent
3060394149
commit
8756b68eda
@@ -463,13 +463,18 @@ aa_state_t aa_dfa_match_len(struct aa_dfa *dfa, aa_state_t start,
|
||||
if (dfa->tables[YYTD_ID_EC]) {
|
||||
/* Equivalence class table defined */
|
||||
u8 *equiv = EQUIV_TABLE(dfa);
|
||||
for (; len; len--)
|
||||
match_char(state, def, base, next, check,
|
||||
equiv[(u8) *str++]);
|
||||
for (; len; len--) {
|
||||
u8 c = equiv[(u8) *str];
|
||||
|
||||
match_char(state, def, base, next, check, c);
|
||||
str++;
|
||||
}
|
||||
} else {
|
||||
/* default is direct to next state */
|
||||
for (; len; len--)
|
||||
match_char(state, def, base, next, check, (u8) *str++);
|
||||
for (; len; len--) {
|
||||
match_char(state, def, base, next, check, (u8) *str);
|
||||
str++;
|
||||
}
|
||||
}
|
||||
|
||||
return state;
|
||||
@@ -503,13 +508,18 @@ aa_state_t aa_dfa_match(struct aa_dfa *dfa, aa_state_t start, const char *str)
|
||||
/* Equivalence class table defined */
|
||||
u8 *equiv = EQUIV_TABLE(dfa);
|
||||
/* default is direct to next state */
|
||||
while (*str)
|
||||
match_char(state, def, base, next, check,
|
||||
equiv[(u8) *str++]);
|
||||
while (*str) {
|
||||
u8 c = equiv[(u8) *str];
|
||||
|
||||
match_char(state, def, base, next, check, c);
|
||||
str++;
|
||||
}
|
||||
} else {
|
||||
/* default is direct to next state */
|
||||
while (*str)
|
||||
match_char(state, def, base, next, check, (u8) *str++);
|
||||
while (*str) {
|
||||
match_char(state, def, base, next, check, (u8) *str);
|
||||
str++;
|
||||
}
|
||||
}
|
||||
|
||||
return state;
|
||||
|
||||
Reference in New Issue
Block a user