| 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: <20241216164055.96267-13-cgoettsche@seltendoof.de>
Date: Mon, 16 Dec 2024 17:40:11 +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>,
Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Bill Wendling <morbo@...gle.com>,
Justin Stitt <justinstitt@...gle.com>,
Thiébaud Weksteen <tweek@...gle.com>,
Bram Bonné <brambonne@...gle.com>,
Masahiro Yamada <masahiroy@...nel.org>,
linux-kernel@...r.kernel.org,
llvm@...ts.linux.dev,
Casey Schaufler <casey@...aufler-ca.com>,
GUO Zihua <guozihua@...wei.com>,
Canfeng Guo <guocanfeng@...ontech.com>
Subject: [RFC PATCH v2 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 4bc1e225f2fe..2c2bd0df8645 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -1256,6 +1256,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++) {
@@ -1287,15 +1289,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 d5725c768d59..797b9a0692fd 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