diff mbox series

[RFC,v2,09/36] libsepol: use reallocarray wrapper to avoid overflows

Message ID 20211105154542.38434-10-cgzones@googlemail.com (mailing list archive)
State Superseded
Headers show
Series libsepol: add fuzzer for reading binary policies | expand

Commit Message

Christian Göttsche Nov. 5, 2021, 3:45 p.m. UTC
Use a wrapper to guard `realloc(p, a * b)` type allocations, to detect
multiplication overflows, which result in too few memory being
allocated.

Use a custom implementation if the used C library does not offer one.

Also use temporary variables for realloc(3) results in add_i_to_a() and
fp_to_buffer().

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/src/Makefile           |  6 ++++++
 libsepol/src/kernel_to_common.c |  4 ++--
 libsepol/src/module_to_cil.c    |  9 +++++----
 libsepol/src/optimize.c         |  5 +++--
 libsepol/src/private.h          | 11 +++++++++++
 libsepol/src/services.c         |  6 +++---
 libsepol/src/user_record.c      |  5 +++--
 libsepol/src/users.c            | 12 ++++++------
 libsepol/src/util.c             | 11 +++++++----
 9 files changed, 46 insertions(+), 23 deletions(-)
diff mbox series

Patch

diff --git a/libsepol/src/Makefile b/libsepol/src/Makefile
index dc8b1773..13410c67 100644
--- a/libsepol/src/Makefile
+++ b/libsepol/src/Makefile
@@ -29,6 +29,12 @@  LOBJS += $(sort $(patsubst %.c,%.lo,$(sort $(wildcard $(CILDIR)/src/*.c)) $(CIL_
 override CFLAGS += -I$(CILDIR)/include
 endif
 
+# check for reallocarray(3) availability
+H := \#
+ifeq (yes,$(shell printf '${H}define _GNU_SOURCE\n${H}include <stdlib.h>\nint main(void){void*p=reallocarray(NULL, 1, sizeof(char));return 0;}' | $(CC) -x c -o /dev/null - >/dev/null 2>&1 && echo yes))
+override CFLAGS += -DHAVE_REALLOCARRAY
+endif
+
 LD_SONAME_FLAGS=-soname,$(LIBSO),--version-script=$(LIBMAP),-z,defs
 
 LN=ln
diff --git a/libsepol/src/kernel_to_common.c b/libsepol/src/kernel_to_common.c
index a7453d3c..51df8c25 100644
--- a/libsepol/src/kernel_to_common.c
+++ b/libsepol/src/kernel_to_common.c
@@ -161,7 +161,7 @@  int strs_add(struct strs *strs, char *s)
 		char **new;
 		unsigned i = strs->size;
 		strs->size *= 2;
-		new = realloc(strs->list, sizeof(char *)*strs->size);
+		new = reallocarray(strs->list, strs->size, sizeof(char *));
 		if (!new) {
 			sepol_log_err("Out of memory");
 			return -1;
@@ -220,7 +220,7 @@  int strs_add_at_index(struct strs *strs, char *s, unsigned index)
 		while (index >= strs->size) {
 			strs->size *= 2;
 		}
-		new = realloc(strs->list, sizeof(char *)*strs->size);
+		new = reallocarray(strs->list, strs->size, sizeof(char *));
 		if (!new) {
 			sepol_log_err("Out of memory");
 			return -1;
diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index ad0880bd..84e49c5b 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -453,7 +453,7 @@  static int stack_push(struct stack *stack, void *ptr)
 	void *new_stack;
 
 	if (stack->pos + 1 == stack->size) {
-		new_stack = realloc(stack->stack, sizeof(*stack->stack) * (stack->size * 2));
+		new_stack = reallocarray(stack->stack, stack->size * 2, sizeof(*stack->stack));
 		if (new_stack == NULL) {
 			goto exit;
 		}
@@ -4123,7 +4123,7 @@  exit:
 static int fp_to_buffer(FILE *fp, char **data, size_t *data_len)
 {
 	int rc = -1;
-	char *d = NULL;
+	char *d = NULL, *d_tmp;
 	size_t d_len = 0;
 	size_t read_len = 0;
 	size_t max_len = 1 << 17; // start at 128KB, this is enough to hold about half of all the existing pp files
@@ -4139,12 +4139,13 @@  static int fp_to_buffer(FILE *fp, char **data, size_t *data_len)
 		d_len += read_len;
 		if (d_len == max_len) {
 			max_len *= 2;
-			d = realloc(d, max_len);
-			if (d == NULL) {
+			d_tmp = realloc(d, max_len);
+			if (d_tmp == NULL) {
 				log_err("Out of memory");
 				rc = -1;
 				goto exit;
 			}
+			d = d_tmp;
 		}
 	}
 
diff --git a/libsepol/src/optimize.c b/libsepol/src/optimize.c
index f8298fb7..8a048702 100644
--- a/libsepol/src/optimize.c
+++ b/libsepol/src/optimize.c
@@ -59,8 +59,9 @@  static int type_vec_append(struct type_vec *v, uint32_t type)
 {
 	if (v->capacity == v->count) {
 		unsigned int new_capacity = v->capacity * 2;
-		uint32_t *new_types = realloc(v->types,
-					      new_capacity * sizeof(*v->types));
+		uint32_t *new_types = reallocarray(v->types,
+						   new_capacity,
+						   sizeof(*v->types));
 		if (!new_types)
 			return -1;
 
diff --git a/libsepol/src/private.h b/libsepol/src/private.h
index d3d65a57..a8cc1472 100644
--- a/libsepol/src/private.h
+++ b/libsepol/src/private.h
@@ -92,3 +92,14 @@  static inline void* mallocarray(size_t nmemb, size_t size) {
 
 	return malloc(nmemb * size);
 }
+
+#ifndef HAVE_REALLOCARRAY
+static inline void* reallocarray(void *ptr, size_t nmemb, size_t size) {
+	if (size && nmemb > (size_t)-1 / size) {
+		errno = ENOMEM;
+		return NULL;
+	}
+
+	return realloc(ptr, nmemb * size);
+}
+#endif
diff --git a/libsepol/src/services.c b/libsepol/src/services.c
index edcdde21..0f36ac53 100644
--- a/libsepol/src/services.c
+++ b/libsepol/src/services.c
@@ -94,7 +94,7 @@  static void push(char *expr_ptr)
 		else
 			new_stack_len = stack_len * 2;
 
-		new_stack = realloc(stack, new_stack_len * sizeof(*stack));
+		new_stack = reallocarray(stack, new_stack_len, sizeof(*stack));
 		if (!new_stack) {
 			ERR(NULL, "unable to allocate stack space");
 			return;
@@ -449,8 +449,8 @@  static int constraint_expr_eval_reason(context_struct_t *scontext,
 			else
 				new_expr_list_len = expr_list_len * 2;
 
-			new_expr_list = realloc(expr_list,
-					new_expr_list_len * sizeof(*expr_list));
+			new_expr_list = reallocarray(expr_list,
+					new_expr_list_len, sizeof(*expr_list));
 			if (!new_expr_list) {
 				ERR(NULL, "failed to allocate expr buffer stack");
 				rc = -ENOMEM;
diff --git a/libsepol/src/user_record.c b/libsepol/src/user_record.c
index c1356a6b..404fa3a8 100644
--- a/libsepol/src/user_record.c
+++ b/libsepol/src/user_record.c
@@ -183,8 +183,9 @@  int sepol_user_add_role(sepol_handle_t * handle,
 	if (!role_cp)
 		goto omem;
 
-	roles_realloc = realloc(user->roles,
-				sizeof(char *) * (user->num_roles + 1));
+	roles_realloc = reallocarray(user->roles,
+				     user->num_roles + 1,
+				     sizeof(char *));
 	if (!roles_realloc)
 		goto omem;
 
diff --git a/libsepol/src/users.c b/libsepol/src/users.c
index b895b7f5..a7406214 100644
--- a/libsepol/src/users.c
+++ b/libsepol/src/users.c
@@ -226,17 +226,17 @@  int sepol_user_modify(sepol_handle_t * handle,
 		void *tmp_ptr;
 
 		/* Ensure reverse lookup array has enough space */
-		tmp_ptr = realloc(policydb->user_val_to_struct,
-				  (policydb->p_users.nprim +
-				   1) * sizeof(user_datum_t *));
+		tmp_ptr = reallocarray(policydb->user_val_to_struct,
+				  policydb->p_users.nprim + 1,
+				  sizeof(user_datum_t *));
 		if (!tmp_ptr)
 			goto omem;
 		policydb->user_val_to_struct = tmp_ptr;
 		policydb->user_val_to_struct[policydb->p_users.nprim] = NULL;
 
-		tmp_ptr = realloc(policydb->sym_val_to_name[SYM_USERS],
-				  (policydb->p_users.nprim +
-				   1) * sizeof(char *));
+		tmp_ptr = reallocarray(policydb->sym_val_to_name[SYM_USERS],
+				  policydb->p_users.nprim + 1,
+				  sizeof(char *));
 		if (!tmp_ptr)
 			goto omem;
 		policydb->sym_val_to_name[SYM_USERS] = tmp_ptr;
diff --git a/libsepol/src/util.c b/libsepol/src/util.c
index 902c63c5..b7230564 100644
--- a/libsepol/src/util.c
+++ b/libsepol/src/util.c
@@ -40,6 +40,8 @@  struct val_to_name {
  * 0).  Return 0 on success, -1 on out of memory. */
 int add_i_to_a(uint32_t i, uint32_t * cnt, uint32_t ** a)
 {
+	uint32_t *new;
+
 	if (cnt == NULL || a == NULL)
 		return -1;
 
@@ -48,17 +50,18 @@  int add_i_to_a(uint32_t i, uint32_t * cnt, uint32_t ** a)
 	 * than be smart about it, for now we realloc() the array each
 	 * time a new uint32_t is added! */
 	if (*a != NULL)
-		*a = (uint32_t *) realloc(*a, (*cnt + 1) * sizeof(uint32_t));
+		new = (uint32_t *) reallocarray(*a, *cnt + 1, sizeof(uint32_t));
 	else {			/* empty list */
 
 		*cnt = 0;
-		*a = (uint32_t *) malloc(sizeof(uint32_t));
+		new = (uint32_t *) malloc(sizeof(uint32_t));
 	}
-	if (*a == NULL) {
+	if (new == NULL) {
 		return -1;
 	}
-	(*a)[*cnt] = i;
+	new[*cnt] = i;
 	(*cnt)++;
+	*a = new;
 	return 0;
 }