On 8/9/26 07:29, Fabrice Derepas wrote:
verify_tags() validates the tagset table unpacked from a policy blob. For each set it reads a count and checks that advancing the index by that count stays inside sets.table[]:u32 cnt = tags->sets.table[i]; if (i+cnt >= tags->sets.size) { i, cnt and sets.size are all u32, so i+cnt is evaluated modulo 2^32. sets.table[] is filled by unpack_tagsets() with aa_unpack_u32(), so every entry is a raw unbounded 32-bit word taken from the policy blob, and verify_tags() is the function that is supposed to validate it. A count close to U32_MAX makes the sum wrap to a small value, the guard passes, and the inner loop then walks sets.table[++i] past the end of the kcalloc(size, sizeof(u32)) allocation. Note that sets.size is bounded by 65535, because unpack_tagsets() reads it with aa_unpack_array() as a u16, so the wrap cannot be reached by growing the table; it is reached purely through the attacker-supplied count. With sets.size = 2 and sets.table = { 0, 0xffffffff }: i = 0: cnt = 0, guard 0 + 0 >= 2 is false, inner loop does not run i = 1: cnt = 0xffffffff, guard (1 + 0xffffffff) mod 2^32 == 0 >= 2 is false, so the guard is bypassed and the inner loop reads sets.table[2] -- one element past a two element allocation The walk continues until an out-of-bounds value happens to be >= hdrs.size or the access faults, so a crafted policy yields an out-of-bounds read on the policy load path (aa_replace_profiles -> aa_unpack -> unpack_policydb -> unpack_tags -> verify_tags). unpack_tags() runs before the perms and DFA tables are unpacked, so no other table needs to be well formed to reach it. Policy load is gated by aa_may_manage_policy(), which checks CAP_MAC_ADMIN relative to the subject's own user namespace rather than the init user namespace, so with the default unprivileged_userns_apparmor_policy=1 the path is reachable from an unprivileged task in a matched-level nested namespace, not only by a globally privileged one. Perform the addition in u64 so that it cannot wrap, restoring the intended i + cnt < sets.size guarantee. Fixes: 3d28e2397af7 ("apparmor: add support loading per permission tagging") Signed-off-by: Fabrice Derepas <[email protected]>
Acked-by: John Johansen <[email protected]>
--- security/apparmor/policy_unpack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index d9dcff1..15b5e92 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -730,7 +730,7 @@ static bool verify_tags(struct aa_tags_struct *tags, const char **info) /* count followed by count indexes into hdrs */ u32 cnt = tags->sets.table[i];- if (i+cnt >= tags->sets.size) {+ if ((u64)i + cnt >= tags->sets.size) { AA_DEBUG(DEBUG_UNPACK, "tagset too large %d+%d > sets.table[%d]", i, cnt, tags->sets.size);
