All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian Göttsche" <cgzones@googlemail.com>
To: selinux@vger.kernel.org
Subject: [PATCH 1/2] libsepol: reject xperm av rules in conditional statements
Date: Tue,  8 Mar 2022 19:58:09 +0100	[thread overview]
Message-ID: <20220308185811.72407-1-cgzones@googlemail.com> (raw)

Extended permission and neverallow rules are not permitted in
conditional statements.

This causes issues on policy optimization where avtab_search() might
return a non extended permission rule when searching for one.

Found by oss-fuzz (#45327)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/src/policydb_validate.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
index 735c7a33..72063351 100644
--- a/libsepol/src/policydb_validate.c
+++ b/libsepol/src/policydb_validate.c
@@ -658,7 +658,7 @@ bad:
  * Functions to validate a kernel policydb
  */
 
-static int validate_avtab_key(avtab_key_t *key, validate_t flavors[])
+static int validate_avtab_key(avtab_key_t *key, int conditional, validate_t flavors[])
 {
 	if (validate_value(key->source_type, &flavors[SYM_TYPES]))
 		goto bad;
@@ -670,13 +670,16 @@ static int validate_avtab_key(avtab_key_t *key, validate_t flavors[])
 	case AVTAB_ALLOWED:
 	case AVTAB_AUDITALLOW:
 	case AVTAB_AUDITDENY:
-	case AVTAB_XPERMS_ALLOWED:
-	case AVTAB_XPERMS_AUDITALLOW:
-	case AVTAB_XPERMS_DONTAUDIT:
 	case AVTAB_TRANSITION:
 	case AVTAB_MEMBER:
 	case AVTAB_CHANGE:
 		break;
+	case AVTAB_XPERMS_ALLOWED:
+	case AVTAB_XPERMS_AUDITALLOW:
+	case AVTAB_XPERMS_DONTAUDIT:
+		if (conditional)
+			goto bad;
+		break;
 	default:
 		goto bad;
 	}
@@ -691,7 +694,7 @@ static int validate_avtab_key_and_datum(avtab_key_t *k, avtab_datum_t *d, void *
 {
 	validate_t *flavors = (validate_t *)args;
 
-	if (validate_avtab_key(k, flavors))
+	if (validate_avtab_key(k, 0, flavors))
 		return -1;
 
 	if ((k->specified & AVTAB_TYPE) && validate_value(d->data, &flavors[SYM_TYPES]))
@@ -716,7 +719,7 @@ static int validate_cond_av_list(sepol_handle_t *handle, cond_av_list_t *cond_av
 
 	for (; cond_av; cond_av = cond_av->next) {
 		for (avtab_ptr = cond_av->node; avtab_ptr; avtab_ptr = avtab_ptr->next) {
-			if (validate_avtab_key(&avtab_ptr->key, flavors)) {
+			if (validate_avtab_key(&avtab_ptr->key, 1, flavors)) {
 				ERR(handle, "Invalid cond av list");
 				return -1;
 			}
@@ -726,7 +729,7 @@ static int validate_cond_av_list(sepol_handle_t *handle, cond_av_list_t *cond_av
 	return 0;
 }
 
-static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, validate_t flavors[])
+static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, int conditional, validate_t flavors[])
 {
 	class_perm_node_t *class;
 
@@ -746,14 +749,17 @@ static int validate_avrules(sepol_handle_t *handle, avrule_t *avrule, validate_t
 		case AVRULE_AUDITALLOW:
 		case AVRULE_AUDITDENY:
 		case AVRULE_DONTAUDIT:
-		case AVRULE_NEVERALLOW:
 		case AVRULE_TRANSITION:
 		case AVRULE_MEMBER:
 		case AVRULE_CHANGE:
+			break;
+		case AVRULE_NEVERALLOW:
 		case AVRULE_XPERMS_ALLOWED:
 		case AVRULE_XPERMS_AUDITALLOW:
 		case AVRULE_XPERMS_DONTAUDIT:
 		case AVRULE_XPERMS_NEVERALLOW:
+			if (conditional)
+				goto bad;
 			break;
 		default:
 			goto bad;
@@ -814,9 +820,9 @@ static int validate_cond_list(sepol_handle_t *handle, cond_list_t *cond, validat
 			goto bad;
 		if (validate_cond_av_list(handle, cond->false_list, flavors))
 			goto bad;
-		if (validate_avrules(handle, cond->avtrue_list, flavors))
+		if (validate_avrules(handle, cond->avtrue_list, 1, flavors))
 			goto bad;
-		if (validate_avrules(handle, cond->avfalse_list, flavors))
+		if (validate_avrules(handle, cond->avfalse_list, 1, flavors))
 			goto bad;
 		if (validate_bool_id_array(handle, cond->bool_ids, cond->nbools, &flavors[SYM_BOOLS]))
 			goto bad;
@@ -1098,7 +1104,7 @@ static int validate_avrule_blocks(sepol_handle_t *handle, avrule_block_t *avrule
 		for (decl = avrule_block->branch_list; decl != NULL; decl = decl->next) {
 			if (validate_cond_list(handle, decl->cond_list, flavors))
 				goto bad;
-			if (validate_avrules(handle, decl->avrules, flavors))
+			if (validate_avrules(handle, decl->avrules, 0, flavors))
 				goto bad;
 			if (validate_role_trans_rules(handle, decl->role_tr_rules, flavors))
 				goto bad;
-- 
2.35.1


             reply	other threads:[~2022-03-08 18:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-08 18:58 Christian Göttsche [this message]
2022-03-08 18:58 ` [PATCH 2/2] libsepol: validate boolean datum arrays Christian Göttsche
2022-03-11 14:57 ` [PATCH 1/2] libsepol: reject xperm av rules in conditional statements James Carter
2022-03-11 16:04   ` James Carter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220308185811.72407-1-cgzones@googlemail.com \
    --to=cgzones@googlemail.com \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.