[PATCH] apparmor: fix table_size() integer overflow on 32 bit

[PATCH] apparmor: fix table_size() integer overflow on 32 bit

[Maxime Bélair]

table_size() checks ALIGN(sizeof(struct table_header) + len * el_size, 8)
in size_t, where @len is td_lolen, a u32 user-provided value from the
policy blob. On 32 bit kernels both operations can overflow.

Triggering this bug allows to:
 - overflow a 16 byte allocation with up to 4GiB of policy data
 - store the table header at address ZERO_SIZE_PTR (kvzalloc(0))

However, this bug requires CAP_MAC_ADMIN, only affects 32 bit kernels,
and the copy runs until the machine is destroyed, so it gives no
controllable primitive and is therefore not a security issue.

Compute the size with the checked helpers and return 0 when the result is
not representable, so the callers can reject the table. A valid table is
never 0 bytes, the header alone is 12.

64 bit kernels cannot overflow (these structs are 32 bit), and are hence
not affected.

Fixes: e06f75a6a2b4 ("AppArmor: dfa match engine")
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
---
 security/apparmor/include/match.h | 11 ++++++++++-
 security/apparmor/match.c         |  6 +++++-
 security/apparmor/policy_unpack.c |  5 +++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
index f7bd7855f1bd..66485f76d331 100644
--- a/security/apparmor/include/match.h
+++ b/security/apparmor/include/match.h
@@ -12,6 +12,7 @@
 #define __AA_MATCH_H
 
 #include <linux/kref.h>
+#include <linux/overflow.h>
 
 #define DFA_NOMATCH			0
 #define DFA_START			1
@@ -120,7 +121,15 @@ struct aa_dfa {
 
 static inline size_t table_size(size_t len, size_t el_size)
 {
-	return ALIGN(sizeof(struct table_header) + len * el_size, 8);
+	size_t size;
+
+	/* Need check_*_overflow for 32 bit machines */
+	if (check_mul_overflow(len, el_size, &size) ||
+	    check_add_overflow(size, sizeof(struct table_header), &size) ||
+	    size > SIZE_MAX - 7)
+		return 0;
+
+	return ALIGN(size, 8);
 }
 
 #define aa_state_t unsigned int
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index 7713484f6a36..825811d566b9 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -58,7 +58,7 @@ static struct table_header *unpack_table(const char *blob, size_t bsize)
 	if (th.td_lolen == 0)
 		goto out;
 	tsize = table_size(th.td_lolen, th.td_flags);
-	if (bsize < tsize)
+	if (!tsize || bsize < tsize)
 		goto out;
 
 	table = kvzalloc(tsize, GFP_KERNEL);
@@ -281,6 +281,10 @@ static struct table_header *remap_data16_to_data32(struct table_header *old)
 	u32 i;
 
 	tsize = table_size(old->td_lolen, YYTD_DATA32);
+	if (!tsize) {
+		kvfree(old);
+		return NULL;
+	}
 	new = kvzalloc(tsize, GFP_KERNEL);
 	if (!new) {
 		kvfree(old);
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index f1fc48e72d0e..3c744f99a15c 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -1052,6 +1052,11 @@ static int unpack_pdb(struct aa_ext *e, struct aa_policydb **policy,
 		u16 tdflags = pdb->dfa->tables[YYTD_ID_ACCEPT]->td_flags;
 		size_t tsize = table_size(noents, tdflags);
 
+		if (!tsize) {
+			*info = "invalid dfa flags table size";
+			error = -EPROTO;
+			goto fail;
+		}
 		pdb->dfa->tables[YYTD_ID_ACCEPT2] = kvzalloc(tsize, GFP_KERNEL);
 		if (!pdb->dfa->tables[YYTD_ID_ACCEPT2]) {
 			*info = "failed to alloc dfa flags table";
-- 
2.51.0