diff options
| author | Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> | 2026-01-29 17:08:25 +0100 |
|---|---|---|
| committer | John Johansen <john.johansen@canonical.com> | 2026-03-09 16:05:43 -0700 |
| commit | 8756b68edae37ff546c02091989a4ceab3f20abd (patch) | |
| tree | f76e80d43854ce57adf76a9e373cb5a9d16bbccb /security | |
| parent | 306039414932c80f8420695a24d4fe10c84ccfb2 (diff) | |
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: 074c1cd798cb ("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>
Diffstat (limited to 'security')
| -rw-r--r-- | security/apparmor/match.c | 30 |
1 files changed, 20 insertions, 10 deletions
diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 8fa0a1494acd..30c46b119869 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -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; |