| 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-14-cgoettsche@seltendoof.de>
Date: Fri, 15 Nov 2024 14:35:33 +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>,
linux-kernel@...r.kernel.org
Subject: [RFC PATCH 14/22] selinux: pre-validate conditional expressions
From: Christian Göttsche <cgzones@...glemail.com>
Validate conditional expressions while reading the policy, to avoid
unexpected access decisions on malformed policies.
Signed-off-by: Christian Göttsche <cgzones@...glemail.com>
---
security/selinux/ss/conditional.c | 116 ++++++++++++++++++++----------
security/selinux/ss/policydb.c | 7 ++
security/selinux/ss/policydb.h | 1 +
3 files changed, 88 insertions(+), 36 deletions(-)
diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index 07008ea081ba..d37b4bdf6ba9 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -21,65 +21,119 @@
* or undefined (-1). Undefined occurs when the expression
* exceeds the stack depth of COND_EXPR_MAXDEPTH.
*/
-static int cond_evaluate_expr(struct policydb *p, struct cond_expr *expr)
+static int cond_evaluate_expr(const struct policydb *p, const struct cond_expr *expr)
{
u32 i;
int s[COND_EXPR_MAXDEPTH];
int sp = -1;
- if (expr->len == 0)
- return -1;
+ if (unlikely(expr->len == 0))
+ goto invalid;
for (i = 0; i < expr->len; i++) {
- struct cond_expr_node *node = &expr->nodes[i];
+ const struct cond_expr_node *node = &expr->nodes[i];
switch (node->expr_type) {
case COND_BOOL:
- if (sp == (COND_EXPR_MAXDEPTH - 1))
- return -1;
+ if (unlikely(sp >= (COND_EXPR_MAXDEPTH - 1)))
+ goto invalid;
sp++;
s[sp] = p->bool_val_to_struct[node->boolean - 1]->state;
break;
case COND_NOT:
- if (sp < 0)
- return -1;
+ if (unlikely(sp < 0))
+ goto invalid;
s[sp] = !s[sp];
break;
case COND_OR:
- if (sp < 1)
- return -1;
+ if (unlikely(sp < 1))
+ goto invalid;
sp--;
s[sp] |= s[sp + 1];
break;
case COND_AND:
- if (sp < 1)
- return -1;
+ if (unlikely(sp < 1))
+ goto invalid;
sp--;
s[sp] &= s[sp + 1];
break;
case COND_XOR:
- if (sp < 1)
- return -1;
+ if (unlikely(sp < 1))
+ goto invalid;
sp--;
s[sp] ^= s[sp + 1];
break;
case COND_EQ:
- if (sp < 1)
- return -1;
+ if (unlikely(sp < 1))
+ goto invalid;
sp--;
s[sp] = (s[sp] == s[sp + 1]);
break;
case COND_NEQ:
- if (sp < 1)
- return -1;
+ if (unlikely(sp < 1))
+ goto invalid;
sp--;
s[sp] = (s[sp] != s[sp + 1]);
break;
default:
- return -1;
+ goto invalid;
}
}
+
+ if (unlikely(sp != 0))
+ goto invalid;
+
return s[0];
+
+invalid:
+ /* Should *never* be reached, cause malformed expressions should
+ * have been filtered by cond_validate_expr().
+ */
+ WARN_ONCE(true, "SELinux: invalid conditional expression passed validation\n");
+ return -1;
+}
+
+static int cond_validate_expr(const struct policydb *p, const struct cond_expr *expr)
+{
+ u32 i;
+ int depth = -1;
+
+ if (expr->len == 0)
+ return -EINVAL;
+
+ for (i = 0; i < expr->len; i++) {
+ const struct cond_expr_node *node = &expr->nodes[i];
+
+ switch (node->expr_type) {
+ case COND_BOOL:
+ if (depth >= (COND_EXPR_MAXDEPTH - 1))
+ return -EINVAL;
+ depth++;
+ if (!policydb_boolean_isvalid(p, node->boolean))
+ return -EINVAL;
+ break;
+ case COND_NOT:
+ if (depth < 0)
+ return -EINVAL;
+ break;
+ case COND_OR:
+ case COND_AND:
+ case COND_XOR:
+ case COND_EQ:
+ case COND_NEQ:
+ if (depth < 1)
+ return -EINVAL;
+ depth--;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ if (depth != 0)
+ return -EINVAL;
+
+ return 0;
}
/*
@@ -355,21 +409,6 @@ static int cond_read_av_list(struct policydb *p, struct policy_file *fp,
return 0;
}
-static int expr_node_isvalid(struct policydb *p, struct cond_expr_node *expr)
-{
- if (expr->expr_type <= 0 || expr->expr_type > COND_LAST) {
- pr_err("SELinux: conditional expressions uses unknown operator.\n");
- return 0;
- }
-
- if (expr->expr_type == COND_BOOL &&
- (expr->boolean == 0 || expr->boolean > p->p_bools.nprim)) {
- pr_err("SELinux: conditional expressions uses unknown bool.\n");
- return 0;
- }
- return 1;
-}
-
static int cond_read_node(struct policydb *p, struct cond_node *node, struct policy_file *fp)
{
__le32 buf[2];
@@ -384,6 +423,8 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, struct pol
/* expr */
len = le32_to_cpu(buf[1]);
+ if (len == 0)
+ return -EINVAL;
rc = oom_check(2 * sizeof(u32), len, fp);
if (rc)
@@ -404,9 +445,12 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, struct pol
expr->expr_type = le32_to_cpu(buf[0]);
expr->boolean = le32_to_cpu(buf[1]);
+ }
- if (!expr_node_isvalid(p, expr))
- return -EINVAL;
+ rc = cond_validate_expr(p, &node->expr);
+ if (rc) {
+ pr_err("SELinux: invalid conditional expression\n");
+ return rc;
}
rc = cond_read_av_list(p, fp, &node->true_list, NULL);
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 5d99e1498b55..1768ac4ecc2c 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -940,6 +940,13 @@ int policydb_type_isvalid(struct policydb *p, unsigned int type)
return 1;
}
+int policydb_boolean_isvalid(const struct policydb *p, u32 boolean)
+{
+ if (!boolean || boolean > p->p_bools.nprim)
+ return 0;
+ return 1;
+}
+
/*
* Return 1 if the fields in the security context
* structure `c' are valid. Return 0 otherwise.
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index fee9132b0d42..c94253a1ddbc 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -318,6 +318,7 @@ extern int policydb_context_isvalid(struct policydb *p, struct context *c);
extern int policydb_class_isvalid(struct policydb *p, u16 class);
extern int policydb_type_isvalid(struct policydb *p, unsigned int type);
extern int policydb_role_isvalid(struct policydb *p, unsigned int role);
+extern int policydb_boolean_isvalid(const struct policydb *p, u32 boolean);
struct policy_file {
char *data;
--
2.45.2
Powered by blists - more mailing lists