139 lines
5.0 KiB
Diff
139 lines
5.0 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
|
|
---
|
|
libsepol/src/avtab.c | 2 +-
|
|
libsepol/src/policydb.c | 9 ++++++---
|
|
libsepol/src/private.h | 22 ++++++++++++++++------
|
|
libsepol/src/services.c | 2 +-
|
|
4 files changed, 24 insertions(+), 11 deletions(-)
|
|
|
|
diff --git a/libsepol/src/avtab.c b/libsepol/src/avtab.c
|
|
index 7c2328b7..b2fa8d85 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 bc7bc9dc..6ba4f916 100644
|
|
--- a/libsepol/src/policydb.c
|
|
+++ b/libsepol/src/policydb.c
|
|
@@ -3857,7 +3857,8 @@ 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))
|
|
+ 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;
|
|
@@ -4036,7 +4037,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 (zero_or_saturated(scope->decl_ids_len)) {
|
|
+ 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;
|
|
}
|
|
@@ -4315,7 +4317,8 @@ 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))
|
|
+ if (is_saturated(nprim) ||
|
|
+ exceeds_available_bytes(fp, nprim, sizeof(uint32_t) * 3))
|
|
goto bad;
|
|
nel = le32_to_cpu(buf[1]);
|
|
if (nel && !nprim) {
|
|
diff --git a/libsepol/src/private.h b/libsepol/src/private.h
|
|
index 1833b497..1500bbc2 100644
|
|
--- a/libsepol/src/private.h
|
|
+++ b/libsepol/src/private.h
|
|
@@ -44,13 +44,23 @@
|
|
|
|
#define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0]))
|
|
|
|
-#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
|
|
-# define is_saturated(x) (x == (typeof(x))-1 || (x) > (1U << 16))
|
|
-#else
|
|
-# define is_saturated(x) (x == (typeof(x))-1)
|
|
-#endif
|
|
+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 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 51bd56a0..aa1ad52c 100644
|
|
--- a/libsepol/src/services.c
|
|
+++ b/libsepol/src/services.c
|
|
@@ -1748,7 +1748,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.27.0
|