[PATCH] apparmor: bounds-check the OWNER companion perms entry at load time

[PATCH] apparmor: bounds-check the OWNER companion perms entry at load time

[Qihang]

For OWNER-flagged DFA states, aa_lookup_condperms() may access
perms[index + 1] for non-owner lookups. verify_dfa_accept_index()
only checks that index is within the perms table, allowing the
companion entry to be read past the end.

Reject OWNER states whose companion perms entry is out of bounds.

Fixes: 6f442d42c0d89 ("apparmor: fix profile verification and enable it")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
 security/apparmor/policy_unpack.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index d9dcff167c480..e53f92b34725f 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -1485,8 +1485,24 @@ static int verify_header(struct aa_ext *e, int required, const char **ns)
 static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size)
 {
 	int i;
+
+	/* the OWNER companion entry needs one extra table slot */
+	if (table_size <= 0)
+		return false;
+
 	for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
-		if (ACCEPT_TABLE(dfa)[i] >= table_size)
+		int limit = table_size;
+
+		/*
+		 * OWNER-flagged states are indexed as perms pairs:
+		 * aa_lookup_condperms() returns &perms[index + 1]
+		 * for non-owner lookups, so the companion entry must
+		 * stay in bounds too.
+		 */
+		if (dfa->tables[YYTD_ID_ACCEPT2] &&
+		    (ACCEPT_TABLE2(dfa)[i] & ACCEPT_FLAG_OWNER))
+			limit--;
+		if (ACCEPT_TABLE(dfa)[i] >= limit)
 			return false;
 	}
 	return true;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] apparmor: bounds-check the OWNER companion perms entry at load time

[Qihang]

Hi John,

Just a quick ping on this patch.

Thanks, Qihang