[PATCH 1/3] libsepol: Fix out-of-bounds memory write in discard_tunbables()

[James Carter <jwcart2@gmail.com>]

The function discard_tunables() will walk all the avrule blocks
and do one of the following two options.
1) If preserve_tunables == 0, then it will evalutate tunable
expressions and add the appropriate true or false block to the
avrules list of the current enabled block.
2) If preserve_tunables !- 0, then it will remove the tunable flag
from all tunables making them booleans.

The function was allocating an array of pointers to cond_bool_datum_t
with a length of COND_EXPR_MAXDEPTH. The number of tunables was the
index and each tunable found would be pointed to be the array. This
is a potential buffer overflow because COND_EXPR_MAXDEPTH is the
limit on the depth of sub expressions, not the limit on the number
of items in an expression. Having more than COND_EXPR_MAXDEPTH
number of tunables in an expression that had a maximum sub
expression depth of less than COND_EXPR_MAXDEPTH would cause an
out-of-bounds memory write.

There is no need to wait to update a tunable datum's flag, so
just update the flags as tunables are found when preserve_tunables
is true.

This patch is based on a report and patch from the security firm
Trail of Bits.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/src/expand.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index ed912b57..5b2b7b03 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -3042,22 +3042,21 @@ static void discard_tunables(sepol_handle_t *sh, policydb_t *pol)
 
 		for (cur_node = decl->cond_list; cur_node != NULL;
 		     cur_node = cur_node->next) {
-			int booleans, tunables, i;
+			int booleans = 0, tunables = 0;
 			cond_bool_datum_t *booldatum;
-			cond_bool_datum_t *tmp[COND_EXPR_MAXDEPTH];
-
-			booleans = tunables = 0;
-			memset(tmp, 0, sizeof(cond_bool_datum_t *) * COND_EXPR_MAXDEPTH);
 
 			for (cur_expr = cur_node->expr; cur_expr != NULL;
 			     cur_expr = cur_expr->next) {
 				if (cur_expr->expr_type != COND_BOOL)
 					continue;
 				booldatum = pol->bool_val_to_struct[cur_expr->boolean - 1];
-				if (booldatum->flags & COND_BOOL_FLAGS_TUNABLE)
-					tmp[tunables++] = booldatum;
-				else
+				if (booldatum->flags & COND_BOOL_FLAGS_TUNABLE) {
+					tunables++;
+					if (preserve_tunables)
+						booldatum->flags &= ~COND_BOOL_FLAGS_TUNABLE;
+				} else {
 					booleans++;
+				}
 			}
 
 			/* bool_copy_callback() at link phase has ensured
@@ -3069,10 +3068,6 @@ static void discard_tunables(sepol_handle_t *sh, policydb_t *pol)
 
 			if (booleans || preserve_tunables) {
 				cur_node->flags &= ~COND_NODE_FLAGS_TUNABLE;
-				if (tunables) {
-					for (i = 0; i < tunables; i++)
-						tmp[i]->flags &= ~COND_BOOL_FLAGS_TUNABLE;
-				}
 			} else {
 				cur_node->flags |= COND_NODE_FLAGS_TUNABLE;
 				cur_state = cond_evaluate_expr(pol, cur_node->expr);
-- 
2.53.0