| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241115133619.114393-13-cgoettsche@seltendoof.de>
Date: Fri, 15 Nov 2024 14:35:32 +0100
From: Christian Göttsche <cgoettsche@...tendoof.de>
To: selinux@...r.kernel.org
Cc: Christian Göttsche <cgzones@...glemail.com>,
Paul Moore <paul@...l-moore.com>,
Stephen Smalley <stephen.smalley.work@...il.com>,
Ondrej Mosnacek <omosnace@...hat.com>,
Casey Schaufler <casey@...aufler-ca.com>,
Canfeng Guo <guocanfeng@...ontech.com>,
Mimi Zohar <zohar@...ux.ibm.com>,
GUO Zihua <guozihua@...wei.com>,
Thiébaud Weksteen <tweek@...gle.com>,
linux-kernel@...r.kernel.org
Subject: [RFC PATCH 13/22] selinux: validate constraints
From: Christian Göttsche <cgzones@...glemail.com>
Validate constraint expressions during reading the policy.
Avoid the usage of BUG() on constraint evaluation, to mitigate malformed
policies halting the system.
Closes: https://github.com/SELinuxProject/selinux-testsuite/issues/76
Signed-off-by: Christian Göttsche <cgzones@...glemail.com>
---
security/selinux/ss/policydb.c | 61 ++++++++++++++++++++++++++++++++--
security/selinux/ss/services.c | 55 +++++++++++++++---------------
2 files changed, 88 insertions(+), 28 deletions(-)
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 5ba1e8d483fb..5d99e1498b55 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -1251,6 +1251,8 @@ static int read_cons_helper(struct policydb *p, struct constraint_node **nodep,
return rc;
c->permissions = le32_to_cpu(buf[0]);
nexpr = le32_to_cpu(buf[1]);
+ if (nexpr == 0)
+ return -EINVAL;
le = NULL;
depth = -1;
for (j = 0; j < nexpr; j++) {
@@ -1282,15 +1284,70 @@ static int read_cons_helper(struct policydb *p, struct constraint_node **nodep,
depth--;
break;
case CEXPR_ATTR:
- if (depth == (CEXPR_MAXDEPTH - 1))
+ if (depth >= (CEXPR_MAXDEPTH - 1))
return -EINVAL;
depth++;
break;
+
+ switch (e->op) {
+ case CEXPR_EQ:
+ case CEXPR_NEQ:
+ break;
+ case CEXPR_DOM:
+ case CEXPR_DOMBY:
+ case CEXPR_INCOMP:
+ if ((e->attr & CEXPR_USER) || (e->attr & CEXPR_TYPE))
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (e->attr) {
+ case CEXPR_USER:
+ case CEXPR_ROLE:
+ case CEXPR_TYPE:
+ case CEXPR_L1L2:
+ case CEXPR_L1H2:
+ case CEXPR_H1L2:
+ case CEXPR_H1H2:
+ case CEXPR_L1H1:
+ case CEXPR_L2H2:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ break;
case CEXPR_NAMES:
if (!allowxtarget && (e->attr & CEXPR_XTARGET))
return -EINVAL;
- if (depth == (CEXPR_MAXDEPTH - 1))
+ if (depth >= (CEXPR_MAXDEPTH - 1))
+ return -EINVAL;
+
+ switch (e->op) {
+ case CEXPR_EQ:
+ case CEXPR_NEQ:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (e->attr) {
+ case CEXPR_USER:
+ case CEXPR_USER | CEXPR_TARGET:
+ case CEXPR_USER | CEXPR_XTARGET:
+ case CEXPR_ROLE:
+ case CEXPR_ROLE | CEXPR_TARGET:
+ case CEXPR_ROLE | CEXPR_XTARGET:
+ case CEXPR_TYPE:
+ case CEXPR_TYPE | CEXPR_TARGET:
+ case CEXPR_TYPE | CEXPR_XTARGET:
+ break;
+ default:
return -EINVAL;
+ }
+
depth++;
rc = ebitmap_read(&e->names, fp);
if (rc)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d8b5fb98adb9..b1b90f802328 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -278,22 +278,25 @@ static int constraint_expr_eval(struct policydb *policydb,
for (e = cexpr; e; e = e->next) {
switch (e->expr_type) {
case CEXPR_NOT:
- BUG_ON(sp < 0);
+ if (unlikely(sp < 0))
+ goto invalid;
s[sp] = !s[sp];
break;
case CEXPR_AND:
- BUG_ON(sp < 1);
+ if (unlikely(sp < 1))
+ goto invalid;
sp--;
s[sp] &= s[sp + 1];
break;
case CEXPR_OR:
- BUG_ON(sp < 1);
+ if (unlikely(sp < 1))
+ goto invalid;
sp--;
s[sp] |= s[sp + 1];
break;
case CEXPR_ATTR:
- if (sp == (CEXPR_MAXDEPTH - 1))
- return 0;
+ if (unlikely(sp >= (CEXPR_MAXDEPTH - 1)))
+ goto invalid;
switch (e->attr) {
case CEXPR_USER:
val1 = scontext->user;
@@ -369,13 +372,11 @@ static int constraint_expr_eval(struct policydb *policydb,
s[++sp] = mls_level_incomp(l2, l1);
continue;
default:
- BUG();
- return 0;
+ goto invalid;
}
break;
default:
- BUG();
- return 0;
+ goto invalid;
}
switch (e->op) {
@@ -386,22 +387,19 @@ static int constraint_expr_eval(struct policydb *policydb,
s[++sp] = (val1 != val2);
break;
default:
- BUG();
- return 0;
+ goto invalid;
}
break;
case CEXPR_NAMES:
- if (sp == (CEXPR_MAXDEPTH-1))
- return 0;
+ if (unlikely(sp >= (CEXPR_MAXDEPTH-1)))
+ goto invalid;
c = scontext;
if (e->attr & CEXPR_TARGET)
c = tcontext;
else if (e->attr & CEXPR_XTARGET) {
c = xcontext;
- if (!c) {
- BUG();
- return 0;
- }
+ if (unlikely(!c))
+ goto invalid;
}
if (e->attr & CEXPR_USER)
val1 = c->user;
@@ -409,10 +407,8 @@ static int constraint_expr_eval(struct policydb *policydb,
val1 = c->role;
else if (e->attr & CEXPR_TYPE)
val1 = c->type;
- else {
- BUG();
- return 0;
- }
+ else
+ goto invalid;
switch (e->op) {
case CEXPR_EQ:
@@ -422,18 +418,25 @@ static int constraint_expr_eval(struct policydb *policydb,
s[++sp] = !ebitmap_get_bit(&e->names, val1 - 1);
break;
default:
- BUG();
- return 0;
+ goto invalid;
}
break;
default:
- BUG();
- return 0;
+ goto invalid;
}
}
- BUG_ON(sp != 0);
+ if (unlikely(sp != 0))
+ goto invalid;
+
return s[0];
+
+invalid:
+ /* Should *never* be reached, cause malformed constraints should
+ * have been filtered by read_cons_helper().
+ */
+ WARN_ONCE(true, "SELinux: invalid constraint passed validation\n");
+ return 0;
}
/*
--
2.45.2
Powered by blists - more mailing lists