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 <[email protected]>
---
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
