134 lines
4.9 KiB
Diff
134 lines
4.9 KiB
Diff
From bd1b7848c66b69bdb1ef25332c90c47d61656437 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
|
|
Date: Thu, 9 Nov 2023 14:51:20 +0100
|
|
Subject: [PATCH] libsepol: enhance saturation check
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
Several values while parsing kernel policies, like symtab sizes or
|
|
string lengths, are checked for saturation. They may not be set to the
|
|
maximum value, to avoid overflows or occupying a reserved value, and
|
|
many of those sizes must not be 0. This is currently handled via the
|
|
two macros is_saturated() and zero_or_saturated().
|
|
|
|
Both macros are tweaked for the fuzzer, because the fuzzer can create
|
|
input with huge sizes. While there is no subsequent data to provide
|
|
the announced sizes, which will be caught later, memory of the requested
|
|
size is allocated, which would lead to OOM reports. Thus the sizes for
|
|
the fuzzer are limited to 2^16. This has the drawback of the fuzzer
|
|
not checking the complete input space.
|
|
|
|
Check the sizes in question for actual enough bytes available in the
|
|
input. This is (only) possible for mapped memory, which the fuzzer
|
|
uses.
|
|
|
|
Application like setools do currently not benefit from this change,
|
|
since they load the policy via a stream. There are currently multiple
|
|
interfaces to load a policy, so reworking them to use mapped memory by
|
|
default might be subject for future work.
|
|
|
|
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
|
|
Acked-by: James Carter <jwcart2@gmail.com>
|
|
|
|
Reference: https://github.com/SELinuxProject/selinux/commit/bd1b7848c66b69bdb1ef25332c90c47d61656437
|
|
Conflict: Context adaptation
|
|
---
|
|
src/avtab.c | 2 +-
|
|
src/policydb.c | 9 ++++++---
|
|
src/private.h | 18 ++++++++++++++++--
|
|
src/services.c | 2 +-
|
|
4 files changed, 24 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/libsepol/src/avtab.c b/libsepol/src/avtab.c
|
|
index 32cd052..e87361b 100644
|
|
--- a/libsepol/src/avtab.c
|
|
+++ b/libsepol/src/avtab.c
|
|
@@ -600,7 +600,7 @@ int avtab_read(avtab_t * a, struct policy_file *fp, uint32_t vers)
|
|
goto bad;
|
|
}
|
|
nel = le32_to_cpu(buf[0]);
|
|
- if (zero_or_saturated(nel)) {
|
|
+ if (zero_or_saturated(nel) || exceeds_available_bytes(fp, nel, sizeof(uint32_t) * 3)) {
|
|
ERR(fp->handle, "table is empty");
|
|
goto bad;
|
|
}
|
|
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
|
|
index c601908..9ce6999 100644
|
|
--- a/libsepol/src/policydb.c
|
|
+++ b/libsepol/src/policydb.c
|
|
@@ -3927,6 +3927,9 @@ static int scope_index_read(scope_index_t * scope_index,
|
|
if (rc < 0)
|
|
return -1;
|
|
scope_index->class_perms_len = le32_to_cpu(buf[0]);
|
|
+ if (is_saturated(scope_index->class_perms_len) ||
|
|
+ exceeds_available_bytes(fp, scope_index->class_perms_len, sizeof(uint32_t) * 3))
|
|
+ return -1;
|
|
if (scope_index->class_perms_len == 0) {
|
|
scope_index->class_perms_map = NULL;
|
|
return 0;
|
|
@@ -4107,7 +4110,8 @@ static int scope_read(policydb_t * p, int symnum, struct policy_file *fp)
|
|
goto cleanup;
|
|
scope->scope = le32_to_cpu(buf[0]);
|
|
scope->decl_ids_len = le32_to_cpu(buf[1]);
|
|
- if (scope->decl_ids_len == 0) {
|
|
+ if (zero_or_saturated(scope->decl_ids_len) ||
|
|
+ exceeds_available_bytes(fp, scope->decl_ids_len, sizeof(uint32_t))) {
|
|
ERR(fp->handle, "invalid scope with no declaration");
|
|
goto cleanup;
|
|
}
|
|
@@ -4397,6 +4401,9 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose)
|
|
if (rc < 0)
|
|
goto bad;
|
|
nprim = le32_to_cpu(buf[0]);
|
|
+ if (is_saturated(nprim) ||
|
|
+ exceeds_available_bytes(fp, nprim, sizeof(uint32_t) * 3))
|
|
+ goto bad;
|
|
nel = le32_to_cpu(buf[1]);
|
|
if (nel && !nprim) {
|
|
ERR(fp->handle, "unexpected items in symbol table with no symbol");
|
|
diff --git a/libsepol/src/private.h b/libsepol/src/private.h
|
|
index a16d957..1993d77 100644
|
|
--- a/libsepol/src/private.h
|
|
+++ b/libsepol/src/private.h
|
|
@@ -44,8 +44,22 @@
|
|
|
|
#define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0]))
|
|
|
|
-#define is_saturated(x) (x == (typeof(x))-1)
|
|
-#define zero_or_saturated(x) ((x == 0) || is_saturated(x))
|
|
+static inline int exceeds_available_bytes(const struct policy_file *fp, size_t x, size_t req_elem_size)
|
|
+{
|
|
+ size_t req_size;
|
|
+
|
|
+ /* Remaining input size is only available for mmap'ed memory */
|
|
+ if (fp->type != PF_USE_MEMORY)
|
|
+ return 0;
|
|
+
|
|
+ if (__builtin_mul_overflow(x, req_elem_size, &req_size))
|
|
+ return 1;
|
|
+
|
|
+ return req_size > fp->len;
|
|
+}
|
|
+
|
|
+#define is_saturated(x) ((x) == (typeof(x))-1)
|
|
+#define zero_or_saturated(x) (((x) == 0) || is_saturated(x))
|
|
|
|
#define spaceship_cmp(a, b) (((a) > (b)) - ((a) < (b)))
|
|
|
|
diff --git a/libsepol/src/services.c b/libsepol/src/services.c
|
|
index 017cfaf..8a80b3a 100644
|
|
--- a/libsepol/src/services.c
|
|
+++ b/libsepol/src/services.c
|
|
@@ -1742,7 +1742,7 @@ int str_read(char **strp, struct policy_file *fp, size_t len)
|
|
int rc;
|
|
char *str;
|
|
|
|
- if (zero_or_saturated(len)) {
|
|
+ if (zero_or_saturated(len) || exceeds_available_bytes(fp, len, sizeof(char))) {
|
|
errno = EINVAL;
|
|
return -1;
|
|
}
|
|
--
|
|
2.33.0
|