* [PATCH] libsepol/cil: Exit with an error if declaration name is a reserved word
@ 2021-03-18 19:08 James Carter
2021-03-18 22:11 ` Nicolas Iooss
0 siblings, 1 reply; 4+ messages in thread
From: James Carter @ 2021-03-18 19:08 UTC (permalink / raw)
To: selinux; +Cc: James Carter
When CIL evaluates sets, conditional expressions, or constraint
expressions, bad things can happen if an identifier has a name that
is the same as an operator. CIL will interpret the name as an
identifier in some places and as an operator in others.
Example:
(classmap CM1 (and pm1 pm2))
(classmapping CM1 and (C1 (P1a)))
(classmapping CM1 pm1 (C1 (P1b)))
(classmapping CM1 pm2 (C1 (P1c)))
(allow TYPE self (CM1 (and pm1 pm2)))
In the classmap and classmapping statements, "and" is the name of an
identifier, but in the allow rule, "and" is an expression operator.
The result is a segfault.
When an identifier is declared and it is being validated, check if
it has the same name as a reserved word for an expression operator
that can be used with the identifer's flavor and exit with an error
if it does.
Also, change the name of the function __cil_verify_name() to
cil_verify_name(), since this function is neither static nor a
helper function.
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_build_ast.c | 28 ++--------------
libsepol/cil/src/cil_verify.c | 56 +++++++++++++++++++++++++++++++-
libsepol/cil/src/cil_verify.h | 2 +-
3 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 4e53f06a..e57de662 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -114,7 +114,7 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
symtab_t *symtab = NULL;
struct cil_symtab_datum *prev;
- rc = __cil_verify_name((const char*)key);
+ rc = cil_verify_name((const char*)key, nflavor);
if (rc != SEPOL_OK) {
goto exit;
}
@@ -1953,12 +1953,6 @@ int cil_gen_roleattribute(struct cil_db *db, struct cil_tree_node *parse_current
goto exit;
}
- if (parse_current->next->data == CIL_KEY_SELF) {
- cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
- rc = SEPOL_ERR;
- goto exit;
- }
-
cil_roleattribute_init(&attr);
key = parse_current->next->data;
@@ -2337,12 +2331,6 @@ int cil_gen_type(struct cil_db *db, struct cil_tree_node *parse_current, struct
goto exit;
}
- if (parse_current->next->data == CIL_KEY_SELF) {
- cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
- rc = SEPOL_ERR;
- goto exit;
- }
-
cil_type_init(&type);
key = parse_current->next->data;
@@ -2391,12 +2379,6 @@ int cil_gen_typeattribute(struct cil_db *db, struct cil_tree_node *parse_current
goto exit;
}
- if (parse_current->next->data == CIL_KEY_SELF) {
- cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
- rc = SEPOL_ERR;
- goto exit;
- }
-
cil_typeattribute_init(&attr);
key = parse_current->next->data;
@@ -3048,12 +3030,6 @@ int cil_gen_alias(struct cil_db *db, struct cil_tree_node *parse_current, struct
goto exit;
}
- if (flavor == CIL_TYPEALIAS && parse_current->next->data == CIL_KEY_SELF) {
- cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
- rc = SEPOL_ERR;
- goto exit;
- }
-
cil_alias_init(&alias);
key = parse_current->next->data;
@@ -5278,7 +5254,7 @@ int cil_gen_macro(struct cil_db *db, struct cil_tree_node *parse_current, struct
param->str = current_item->cl_head->next->data;
- rc = __cil_verify_name(param->str);
+ rc = cil_verify_name(param->str, param->flavor);
if (rc != SEPOL_OK) {
cil_destroy_param(param);
goto exit;
diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
index 09e3daf9..c2efbf1c 100644
--- a/libsepol/cil/src/cil_verify.c
+++ b/libsepol/cil/src/cil_verify.c
@@ -47,7 +47,55 @@
#include "cil_verify.h"
-int __cil_verify_name(const char *name)
+static int __cil_is_reserved_name(const char *name, enum cil_flavor flavor)
+{
+ switch (flavor) {
+ case CIL_BOOL:
+ case CIL_TUNABLE:
+ if ((name == CIL_KEY_EQ) || (name == CIL_KEY_NEQ))
+ return CIL_TRUE;
+ break;
+ case CIL_PERM:
+ case CIL_MAP_PERM:
+ if (name == CIL_KEY_ALL)
+ return CIL_TRUE;
+ break;
+ case CIL_USER:
+ case CIL_USERATTRIBUTE:
+ if ((name == CIL_KEY_ALL) || (name == CIL_KEY_CONS_U1) || (name == CIL_KEY_CONS_U2) || (name == CIL_KEY_CONS_U3))
+ return CIL_TRUE;
+ break;
+ case CIL_ROLE:
+ case CIL_ROLEATTRIBUTE:
+ if ((name == CIL_KEY_ALL) || (name == CIL_KEY_CONS_R1) || (name == CIL_KEY_CONS_R2) || (name == CIL_KEY_CONS_R3))
+ return CIL_TRUE;
+ break;
+ case CIL_TYPE:
+ case CIL_TYPEATTRIBUTE:
+ case CIL_TYPEALIAS:
+ if ((name == CIL_KEY_ALL) || (name == CIL_KEY_SELF) || (name == CIL_KEY_CONS_T1) || (name == CIL_KEY_CONS_T2) || (name == CIL_KEY_CONS_T3))
+ return CIL_TRUE;
+ break;
+ case CIL_CAT:
+ case CIL_CATSET:
+ case CIL_CATALIAS:
+ case CIL_PERMISSIONX:
+ if ((name == CIL_KEY_ALL) || (name == CIL_KEY_RANGE))
+ return CIL_TRUE;
+ break;
+ default:
+ return CIL_FALSE;
+ break;
+ }
+
+ if ((name == CIL_KEY_AND) || (name == CIL_KEY_OR) || (name == CIL_KEY_NOT) || (name == CIL_KEY_XOR)) {
+ return CIL_TRUE;
+ }
+
+ return CIL_FALSE;
+}
+
+int cil_verify_name(const char *name, enum cil_flavor flavor)
{
int rc = SEPOL_ERR;
int len;
@@ -77,6 +125,12 @@ int __cil_verify_name(const char *name)
goto exit;
}
}
+
+ if (__cil_is_reserved_name(name, flavor)) {
+ cil_log(CIL_ERR, "Name %s is a reserved word\n", name);
+ goto exit;
+ }
+
return SEPOL_OK;
exit:
diff --git a/libsepol/cil/src/cil_verify.h b/libsepol/cil/src/cil_verify.h
index 905761b0..1887ae3f 100644
--- a/libsepol/cil/src/cil_verify.h
+++ b/libsepol/cil/src/cil_verify.h
@@ -56,7 +56,7 @@ struct cil_args_verify {
int *pass;
};
-int __cil_verify_name(const char *name);
+int cil_verify_name(const char *name, enum cil_flavor flavor);
int __cil_verify_syntax(struct cil_tree_node *parse_current, enum cil_syntax s[], int len);
int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, enum cil_flavor expr_flavor);
int cil_verify_constraint_leaf_expr_syntax(enum cil_flavor l_flavor, enum cil_flavor r_flavor, enum cil_flavor op, enum cil_flavor expr_flavor);
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] libsepol/cil: Exit with an error if declaration name is a reserved word
2021-03-18 19:08 [PATCH] libsepol/cil: Exit with an error if declaration name is a reserved word James Carter
@ 2021-03-18 22:11 ` Nicolas Iooss
2021-03-19 13:46 ` James Carter
0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Iooss @ 2021-03-18 22:11 UTC (permalink / raw)
To: James Carter; +Cc: SElinux list
On Thu, Mar 18, 2021 at 8:09 PM James Carter <jwcart2@gmail.com> wrote:
>
> When CIL evaluates sets, conditional expressions, or constraint
> expressions, bad things can happen if an identifier has a name that
> is the same as an operator. CIL will interpret the name as an
> identifier in some places and as an operator in others.
>
> Example:
> (classmap CM1 (and pm1 pm2))
> (classmapping CM1 and (C1 (P1a)))
> (classmapping CM1 pm1 (C1 (P1b)))
> (classmapping CM1 pm2 (C1 (P1c)))
> (allow TYPE self (CM1 (and pm1 pm2)))
> In the classmap and classmapping statements, "and" is the name of an
> identifier, but in the allow rule, "and" is an expression operator.
> The result is a segfault.
>
> When an identifier is declared and it is being validated, check if
> it has the same name as a reserved word for an expression operator
> that can be used with the identifer's flavor and exit with an error
> if it does.
>
> Also, change the name of the function __cil_verify_name() to
> cil_verify_name(), since this function is neither static nor a
> helper function.
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
Hello,
I like this patch, but it makes "make test" fail:
make[1]: Entering directory '/selinux/secilc'
./secilc test/policy.cil
Name t1 is a reserved word
Invalid name
Bad type declaration at test/policy.cil:341
Failed to compile cildb: -1
I guess the test policy needs to be updated in order to no longer use
types t1 and t2 (in
https://github.com/SELinuxProject/selinux/blob/d155b410d4bbc90d28f361b966f0429598da8188/secilc/test/policy.cil#L341-L354).
Nicolas
> ---
> libsepol/cil/src/cil_build_ast.c | 28 ++--------------
> libsepol/cil/src/cil_verify.c | 56 +++++++++++++++++++++++++++++++-
> libsepol/cil/src/cil_verify.h | 2 +-
> 3 files changed, 58 insertions(+), 28 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index 4e53f06a..e57de662 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -114,7 +114,7 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> symtab_t *symtab = NULL;
> struct cil_symtab_datum *prev;
>
> - rc = __cil_verify_name((const char*)key);
> + rc = cil_verify_name((const char*)key, nflavor);
> if (rc != SEPOL_OK) {
> goto exit;
> }
> @@ -1953,12 +1953,6 @@ int cil_gen_roleattribute(struct cil_db *db, struct cil_tree_node *parse_current
> goto exit;
> }
>
> - if (parse_current->next->data == CIL_KEY_SELF) {
> - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> - rc = SEPOL_ERR;
> - goto exit;
> - }
> -
> cil_roleattribute_init(&attr);
>
> key = parse_current->next->data;
> @@ -2337,12 +2331,6 @@ int cil_gen_type(struct cil_db *db, struct cil_tree_node *parse_current, struct
> goto exit;
> }
>
> - if (parse_current->next->data == CIL_KEY_SELF) {
> - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> - rc = SEPOL_ERR;
> - goto exit;
> - }
> -
> cil_type_init(&type);
>
> key = parse_current->next->data;
> @@ -2391,12 +2379,6 @@ int cil_gen_typeattribute(struct cil_db *db, struct cil_tree_node *parse_current
> goto exit;
> }
>
> - if (parse_current->next->data == CIL_KEY_SELF) {
> - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> - rc = SEPOL_ERR;
> - goto exit;
> - }
> -
> cil_typeattribute_init(&attr);
>
> key = parse_current->next->data;
> @@ -3048,12 +3030,6 @@ int cil_gen_alias(struct cil_db *db, struct cil_tree_node *parse_current, struct
> goto exit;
> }
>
> - if (flavor == CIL_TYPEALIAS && parse_current->next->data == CIL_KEY_SELF) {
> - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> - rc = SEPOL_ERR;
> - goto exit;
> - }
> -
> cil_alias_init(&alias);
>
> key = parse_current->next->data;
> @@ -5278,7 +5254,7 @@ int cil_gen_macro(struct cil_db *db, struct cil_tree_node *parse_current, struct
>
> param->str = current_item->cl_head->next->data;
>
> - rc = __cil_verify_name(param->str);
> + rc = cil_verify_name(param->str, param->flavor);
> if (rc != SEPOL_OK) {
> cil_destroy_param(param);
> goto exit;
> diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
> index 09e3daf9..c2efbf1c 100644
> --- a/libsepol/cil/src/cil_verify.c
> +++ b/libsepol/cil/src/cil_verify.c
> @@ -47,7 +47,55 @@
>
> #include "cil_verify.h"
>
> -int __cil_verify_name(const char *name)
> +static int __cil_is_reserved_name(const char *name, enum cil_flavor flavor)
> +{
> + switch (flavor) {
> + case CIL_BOOL:
> + case CIL_TUNABLE:
> + if ((name == CIL_KEY_EQ) || (name == CIL_KEY_NEQ))
> + return CIL_TRUE;
> + break;
> + case CIL_PERM:
> + case CIL_MAP_PERM:
> + if (name == CIL_KEY_ALL)
> + return CIL_TRUE;
> + break;
> + case CIL_USER:
> + case CIL_USERATTRIBUTE:
> + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_CONS_U1) || (name == CIL_KEY_CONS_U2) || (name == CIL_KEY_CONS_U3))
> + return CIL_TRUE;
> + break;
> + case CIL_ROLE:
> + case CIL_ROLEATTRIBUTE:
> + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_CONS_R1) || (name == CIL_KEY_CONS_R2) || (name == CIL_KEY_CONS_R3))
> + return CIL_TRUE;
> + break;
> + case CIL_TYPE:
> + case CIL_TYPEATTRIBUTE:
> + case CIL_TYPEALIAS:
> + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_SELF) || (name == CIL_KEY_CONS_T1) || (name == CIL_KEY_CONS_T2) || (name == CIL_KEY_CONS_T3))
> + return CIL_TRUE;
> + break;
> + case CIL_CAT:
> + case CIL_CATSET:
> + case CIL_CATALIAS:
> + case CIL_PERMISSIONX:
> + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_RANGE))
> + return CIL_TRUE;
> + break;
> + default:
> + return CIL_FALSE;
> + break;
> + }
> +
> + if ((name == CIL_KEY_AND) || (name == CIL_KEY_OR) || (name == CIL_KEY_NOT) || (name == CIL_KEY_XOR)) {
> + return CIL_TRUE;
> + }
> +
> + return CIL_FALSE;
> +}
> +
> +int cil_verify_name(const char *name, enum cil_flavor flavor)
> {
> int rc = SEPOL_ERR;
> int len;
> @@ -77,6 +125,12 @@ int __cil_verify_name(const char *name)
> goto exit;
> }
> }
> +
> + if (__cil_is_reserved_name(name, flavor)) {
> + cil_log(CIL_ERR, "Name %s is a reserved word\n", name);
> + goto exit;
> + }
> +
> return SEPOL_OK;
>
> exit:
> diff --git a/libsepol/cil/src/cil_verify.h b/libsepol/cil/src/cil_verify.h
> index 905761b0..1887ae3f 100644
> --- a/libsepol/cil/src/cil_verify.h
> +++ b/libsepol/cil/src/cil_verify.h
> @@ -56,7 +56,7 @@ struct cil_args_verify {
> int *pass;
> };
>
> -int __cil_verify_name(const char *name);
> +int cil_verify_name(const char *name, enum cil_flavor flavor);
> int __cil_verify_syntax(struct cil_tree_node *parse_current, enum cil_syntax s[], int len);
> int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, enum cil_flavor expr_flavor);
> int cil_verify_constraint_leaf_expr_syntax(enum cil_flavor l_flavor, enum cil_flavor r_flavor, enum cil_flavor op, enum cil_flavor expr_flavor);
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libsepol/cil: Exit with an error if declaration name is a reserved word
2021-03-18 22:11 ` Nicolas Iooss
@ 2021-03-19 13:46 ` James Carter
2021-03-19 15:16 ` James Carter
0 siblings, 1 reply; 4+ messages in thread
From: James Carter @ 2021-03-19 13:46 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: SElinux list
On Thu, Mar 18, 2021 at 6:11 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Thu, Mar 18, 2021 at 8:09 PM James Carter <jwcart2@gmail.com> wrote:
> >
> > When CIL evaluates sets, conditional expressions, or constraint
> > expressions, bad things can happen if an identifier has a name that
> > is the same as an operator. CIL will interpret the name as an
> > identifier in some places and as an operator in others.
> >
> > Example:
> > (classmap CM1 (and pm1 pm2))
> > (classmapping CM1 and (C1 (P1a)))
> > (classmapping CM1 pm1 (C1 (P1b)))
> > (classmapping CM1 pm2 (C1 (P1c)))
> > (allow TYPE self (CM1 (and pm1 pm2)))
> > In the classmap and classmapping statements, "and" is the name of an
> > identifier, but in the allow rule, "and" is an expression operator.
> > The result is a segfault.
> >
> > When an identifier is declared and it is being validated, check if
> > it has the same name as a reserved word for an expression operator
> > that can be used with the identifer's flavor and exit with an error
> > if it does.
> >
> > Also, change the name of the function __cil_verify_name() to
> > cil_verify_name(), since this function is neither static nor a
> > helper function.
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
>
> Hello,
> I like this patch, but it makes "make test" fail:
>
> make[1]: Entering directory '/selinux/secilc'
> ./secilc test/policy.cil
> Name t1 is a reserved word
> Invalid name
> Bad type declaration at test/policy.cil:341
> Failed to compile cildb: -1
>
> I guess the test policy needs to be updated in order to no longer use
> types t1 and t2 (in
> https://github.com/SELinuxProject/selinux/blob/d155b410d4bbc90d28f361b966f0429598da8188/secilc/test/policy.cil#L341-L354).
>
I will need to update the test policy.
I really wish I could allow t1 and the others, especially in macros.
Maybe I can delay checking the right side until the resolve phase and
then check first to see if the right side has been declared and only
if it hasn't would it be treated as the reserved word.
Jim
> Nicolas
>
> > ---
> > libsepol/cil/src/cil_build_ast.c | 28 ++--------------
> > libsepol/cil/src/cil_verify.c | 56 +++++++++++++++++++++++++++++++-
> > libsepol/cil/src/cil_verify.h | 2 +-
> > 3 files changed, 58 insertions(+), 28 deletions(-)
> >
> > diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> > index 4e53f06a..e57de662 100644
> > --- a/libsepol/cil/src/cil_build_ast.c
> > +++ b/libsepol/cil/src/cil_build_ast.c
> > @@ -114,7 +114,7 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> > symtab_t *symtab = NULL;
> > struct cil_symtab_datum *prev;
> >
> > - rc = __cil_verify_name((const char*)key);
> > + rc = cil_verify_name((const char*)key, nflavor);
> > if (rc != SEPOL_OK) {
> > goto exit;
> > }
> > @@ -1953,12 +1953,6 @@ int cil_gen_roleattribute(struct cil_db *db, struct cil_tree_node *parse_current
> > goto exit;
> > }
> >
> > - if (parse_current->next->data == CIL_KEY_SELF) {
> > - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> > - rc = SEPOL_ERR;
> > - goto exit;
> > - }
> > -
> > cil_roleattribute_init(&attr);
> >
> > key = parse_current->next->data;
> > @@ -2337,12 +2331,6 @@ int cil_gen_type(struct cil_db *db, struct cil_tree_node *parse_current, struct
> > goto exit;
> > }
> >
> > - if (parse_current->next->data == CIL_KEY_SELF) {
> > - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> > - rc = SEPOL_ERR;
> > - goto exit;
> > - }
> > -
> > cil_type_init(&type);
> >
> > key = parse_current->next->data;
> > @@ -2391,12 +2379,6 @@ int cil_gen_typeattribute(struct cil_db *db, struct cil_tree_node *parse_current
> > goto exit;
> > }
> >
> > - if (parse_current->next->data == CIL_KEY_SELF) {
> > - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> > - rc = SEPOL_ERR;
> > - goto exit;
> > - }
> > -
> > cil_typeattribute_init(&attr);
> >
> > key = parse_current->next->data;
> > @@ -3048,12 +3030,6 @@ int cil_gen_alias(struct cil_db *db, struct cil_tree_node *parse_current, struct
> > goto exit;
> > }
> >
> > - if (flavor == CIL_TYPEALIAS && parse_current->next->data == CIL_KEY_SELF) {
> > - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> > - rc = SEPOL_ERR;
> > - goto exit;
> > - }
> > -
> > cil_alias_init(&alias);
> >
> > key = parse_current->next->data;
> > @@ -5278,7 +5254,7 @@ int cil_gen_macro(struct cil_db *db, struct cil_tree_node *parse_current, struct
> >
> > param->str = current_item->cl_head->next->data;
> >
> > - rc = __cil_verify_name(param->str);
> > + rc = cil_verify_name(param->str, param->flavor);
> > if (rc != SEPOL_OK) {
> > cil_destroy_param(param);
> > goto exit;
> > diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
> > index 09e3daf9..c2efbf1c 100644
> > --- a/libsepol/cil/src/cil_verify.c
> > +++ b/libsepol/cil/src/cil_verify.c
> > @@ -47,7 +47,55 @@
> >
> > #include "cil_verify.h"
> >
> > -int __cil_verify_name(const char *name)
> > +static int __cil_is_reserved_name(const char *name, enum cil_flavor flavor)
> > +{
> > + switch (flavor) {
> > + case CIL_BOOL:
> > + case CIL_TUNABLE:
> > + if ((name == CIL_KEY_EQ) || (name == CIL_KEY_NEQ))
> > + return CIL_TRUE;
> > + break;
> > + case CIL_PERM:
> > + case CIL_MAP_PERM:
> > + if (name == CIL_KEY_ALL)
> > + return CIL_TRUE;
> > + break;
> > + case CIL_USER:
> > + case CIL_USERATTRIBUTE:
> > + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_CONS_U1) || (name == CIL_KEY_CONS_U2) || (name == CIL_KEY_CONS_U3))
> > + return CIL_TRUE;
> > + break;
> > + case CIL_ROLE:
> > + case CIL_ROLEATTRIBUTE:
> > + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_CONS_R1) || (name == CIL_KEY_CONS_R2) || (name == CIL_KEY_CONS_R3))
> > + return CIL_TRUE;
> > + break;
> > + case CIL_TYPE:
> > + case CIL_TYPEATTRIBUTE:
> > + case CIL_TYPEALIAS:
> > + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_SELF) || (name == CIL_KEY_CONS_T1) || (name == CIL_KEY_CONS_T2) || (name == CIL_KEY_CONS_T3))
> > + return CIL_TRUE;
> > + break;
> > + case CIL_CAT:
> > + case CIL_CATSET:
> > + case CIL_CATALIAS:
> > + case CIL_PERMISSIONX:
> > + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_RANGE))
> > + return CIL_TRUE;
> > + break;
> > + default:
> > + return CIL_FALSE;
> > + break;
> > + }
> > +
> > + if ((name == CIL_KEY_AND) || (name == CIL_KEY_OR) || (name == CIL_KEY_NOT) || (name == CIL_KEY_XOR)) {
> > + return CIL_TRUE;
> > + }
> > +
> > + return CIL_FALSE;
> > +}
> > +
> > +int cil_verify_name(const char *name, enum cil_flavor flavor)
> > {
> > int rc = SEPOL_ERR;
> > int len;
> > @@ -77,6 +125,12 @@ int __cil_verify_name(const char *name)
> > goto exit;
> > }
> > }
> > +
> > + if (__cil_is_reserved_name(name, flavor)) {
> > + cil_log(CIL_ERR, "Name %s is a reserved word\n", name);
> > + goto exit;
> > + }
> > +
> > return SEPOL_OK;
> >
> > exit:
> > diff --git a/libsepol/cil/src/cil_verify.h b/libsepol/cil/src/cil_verify.h
> > index 905761b0..1887ae3f 100644
> > --- a/libsepol/cil/src/cil_verify.h
> > +++ b/libsepol/cil/src/cil_verify.h
> > @@ -56,7 +56,7 @@ struct cil_args_verify {
> > int *pass;
> > };
> >
> > -int __cil_verify_name(const char *name);
> > +int cil_verify_name(const char *name, enum cil_flavor flavor);
> > int __cil_verify_syntax(struct cil_tree_node *parse_current, enum cil_syntax s[], int len);
> > int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, enum cil_flavor expr_flavor);
> > int cil_verify_constraint_leaf_expr_syntax(enum cil_flavor l_flavor, enum cil_flavor r_flavor, enum cil_flavor op, enum cil_flavor expr_flavor);
> > --
> > 2.26.2
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libsepol/cil: Exit with an error if declaration name is a reserved word
2021-03-19 13:46 ` James Carter
@ 2021-03-19 15:16 ` James Carter
0 siblings, 0 replies; 4+ messages in thread
From: James Carter @ 2021-03-19 15:16 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: SElinux list
On Fri, Mar 19, 2021 at 9:46 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Thu, Mar 18, 2021 at 6:11 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> >
> > On Thu, Mar 18, 2021 at 8:09 PM James Carter <jwcart2@gmail.com> wrote:
> > >
> > > When CIL evaluates sets, conditional expressions, or constraint
> > > expressions, bad things can happen if an identifier has a name that
> > > is the same as an operator. CIL will interpret the name as an
> > > identifier in some places and as an operator in others.
> > >
> > > Example:
> > > (classmap CM1 (and pm1 pm2))
> > > (classmapping CM1 and (C1 (P1a)))
> > > (classmapping CM1 pm1 (C1 (P1b)))
> > > (classmapping CM1 pm2 (C1 (P1c)))
> > > (allow TYPE self (CM1 (and pm1 pm2)))
> > > In the classmap and classmapping statements, "and" is the name of an
> > > identifier, but in the allow rule, "and" is an expression operator.
> > > The result is a segfault.
> > >
> > > When an identifier is declared and it is being validated, check if
> > > it has the same name as a reserved word for an expression operator
> > > that can be used with the identifer's flavor and exit with an error
> > > if it does.
> > >
> > > Also, change the name of the function __cil_verify_name() to
> > > cil_verify_name(), since this function is neither static nor a
> > > helper function.
> > >
> > > Signed-off-by: James Carter <jwcart2@gmail.com>
> >
> > Hello,
> > I like this patch, but it makes "make test" fail:
> >
> > make[1]: Entering directory '/selinux/secilc'
> > ./secilc test/policy.cil
> > Name t1 is a reserved word
> > Invalid name
> > Bad type declaration at test/policy.cil:341
> > Failed to compile cildb: -1
> >
> > I guess the test policy needs to be updated in order to no longer use
> > types t1 and t2 (in
> > https://github.com/SELinuxProject/selinux/blob/d155b410d4bbc90d28f361b966f0429598da8188/secilc/test/policy.cil#L341-L354).
> >
>
> I will need to update the test policy.
>
> I really wish I could allow t1 and the others, especially in macros.
> Maybe I can delay checking the right side until the resolve phase and
> then check first to see if the right side has been declared and only
> if it hasn't would it be treated as the reserved word.
>
Thinking about this some more. I think that I will redo this patch and
remove the checks for t1, t2, t3, r1, r2, r3, u1, u2, and u3. These
will just be assumed to be the reserved words if encountered in a
constraint expression. With the recent patch that allows lists of
identifiers in a constraint expression, then one could write "(eq t1
(t1))" if you really want to refer to a type named t1 in a constraint
expression.
Jim
> Jim
>
>
> > Nicolas
> >
> > > ---
> > > libsepol/cil/src/cil_build_ast.c | 28 ++--------------
> > > libsepol/cil/src/cil_verify.c | 56 +++++++++++++++++++++++++++++++-
> > > libsepol/cil/src/cil_verify.h | 2 +-
> > > 3 files changed, 58 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> > > index 4e53f06a..e57de662 100644
> > > --- a/libsepol/cil/src/cil_build_ast.c
> > > +++ b/libsepol/cil/src/cil_build_ast.c
> > > @@ -114,7 +114,7 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> > > symtab_t *symtab = NULL;
> > > struct cil_symtab_datum *prev;
> > >
> > > - rc = __cil_verify_name((const char*)key);
> > > + rc = cil_verify_name((const char*)key, nflavor);
> > > if (rc != SEPOL_OK) {
> > > goto exit;
> > > }
> > > @@ -1953,12 +1953,6 @@ int cil_gen_roleattribute(struct cil_db *db, struct cil_tree_node *parse_current
> > > goto exit;
> > > }
> > >
> > > - if (parse_current->next->data == CIL_KEY_SELF) {
> > > - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> > > - rc = SEPOL_ERR;
> > > - goto exit;
> > > - }
> > > -
> > > cil_roleattribute_init(&attr);
> > >
> > > key = parse_current->next->data;
> > > @@ -2337,12 +2331,6 @@ int cil_gen_type(struct cil_db *db, struct cil_tree_node *parse_current, struct
> > > goto exit;
> > > }
> > >
> > > - if (parse_current->next->data == CIL_KEY_SELF) {
> > > - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> > > - rc = SEPOL_ERR;
> > > - goto exit;
> > > - }
> > > -
> > > cil_type_init(&type);
> > >
> > > key = parse_current->next->data;
> > > @@ -2391,12 +2379,6 @@ int cil_gen_typeattribute(struct cil_db *db, struct cil_tree_node *parse_current
> > > goto exit;
> > > }
> > >
> > > - if (parse_current->next->data == CIL_KEY_SELF) {
> > > - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> > > - rc = SEPOL_ERR;
> > > - goto exit;
> > > - }
> > > -
> > > cil_typeattribute_init(&attr);
> > >
> > > key = parse_current->next->data;
> > > @@ -3048,12 +3030,6 @@ int cil_gen_alias(struct cil_db *db, struct cil_tree_node *parse_current, struct
> > > goto exit;
> > > }
> > >
> > > - if (flavor == CIL_TYPEALIAS && parse_current->next->data == CIL_KEY_SELF) {
> > > - cil_log(CIL_ERR, "The keyword '%s' is reserved\n", CIL_KEY_SELF);
> > > - rc = SEPOL_ERR;
> > > - goto exit;
> > > - }
> > > -
> > > cil_alias_init(&alias);
> > >
> > > key = parse_current->next->data;
> > > @@ -5278,7 +5254,7 @@ int cil_gen_macro(struct cil_db *db, struct cil_tree_node *parse_current, struct
> > >
> > > param->str = current_item->cl_head->next->data;
> > >
> > > - rc = __cil_verify_name(param->str);
> > > + rc = cil_verify_name(param->str, param->flavor);
> > > if (rc != SEPOL_OK) {
> > > cil_destroy_param(param);
> > > goto exit;
> > > diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
> > > index 09e3daf9..c2efbf1c 100644
> > > --- a/libsepol/cil/src/cil_verify.c
> > > +++ b/libsepol/cil/src/cil_verify.c
> > > @@ -47,7 +47,55 @@
> > >
> > > #include "cil_verify.h"
> > >
> > > -int __cil_verify_name(const char *name)
> > > +static int __cil_is_reserved_name(const char *name, enum cil_flavor flavor)
> > > +{
> > > + switch (flavor) {
> > > + case CIL_BOOL:
> > > + case CIL_TUNABLE:
> > > + if ((name == CIL_KEY_EQ) || (name == CIL_KEY_NEQ))
> > > + return CIL_TRUE;
> > > + break;
> > > + case CIL_PERM:
> > > + case CIL_MAP_PERM:
> > > + if (name == CIL_KEY_ALL)
> > > + return CIL_TRUE;
> > > + break;
> > > + case CIL_USER:
> > > + case CIL_USERATTRIBUTE:
> > > + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_CONS_U1) || (name == CIL_KEY_CONS_U2) || (name == CIL_KEY_CONS_U3))
> > > + return CIL_TRUE;
> > > + break;
> > > + case CIL_ROLE:
> > > + case CIL_ROLEATTRIBUTE:
> > > + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_CONS_R1) || (name == CIL_KEY_CONS_R2) || (name == CIL_KEY_CONS_R3))
> > > + return CIL_TRUE;
> > > + break;
> > > + case CIL_TYPE:
> > > + case CIL_TYPEATTRIBUTE:
> > > + case CIL_TYPEALIAS:
> > > + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_SELF) || (name == CIL_KEY_CONS_T1) || (name == CIL_KEY_CONS_T2) || (name == CIL_KEY_CONS_T3))
> > > + return CIL_TRUE;
> > > + break;
> > > + case CIL_CAT:
> > > + case CIL_CATSET:
> > > + case CIL_CATALIAS:
> > > + case CIL_PERMISSIONX:
> > > + if ((name == CIL_KEY_ALL) || (name == CIL_KEY_RANGE))
> > > + return CIL_TRUE;
> > > + break;
> > > + default:
> > > + return CIL_FALSE;
> > > + break;
> > > + }
> > > +
> > > + if ((name == CIL_KEY_AND) || (name == CIL_KEY_OR) || (name == CIL_KEY_NOT) || (name == CIL_KEY_XOR)) {
> > > + return CIL_TRUE;
> > > + }
> > > +
> > > + return CIL_FALSE;
> > > +}
> > > +
> > > +int cil_verify_name(const char *name, enum cil_flavor flavor)
> > > {
> > > int rc = SEPOL_ERR;
> > > int len;
> > > @@ -77,6 +125,12 @@ int __cil_verify_name(const char *name)
> > > goto exit;
> > > }
> > > }
> > > +
> > > + if (__cil_is_reserved_name(name, flavor)) {
> > > + cil_log(CIL_ERR, "Name %s is a reserved word\n", name);
> > > + goto exit;
> > > + }
> > > +
> > > return SEPOL_OK;
> > >
> > > exit:
> > > diff --git a/libsepol/cil/src/cil_verify.h b/libsepol/cil/src/cil_verify.h
> > > index 905761b0..1887ae3f 100644
> > > --- a/libsepol/cil/src/cil_verify.h
> > > +++ b/libsepol/cil/src/cil_verify.h
> > > @@ -56,7 +56,7 @@ struct cil_args_verify {
> > > int *pass;
> > > };
> > >
> > > -int __cil_verify_name(const char *name);
> > > +int cil_verify_name(const char *name, enum cil_flavor flavor);
> > > int __cil_verify_syntax(struct cil_tree_node *parse_current, enum cil_syntax s[], int len);
> > > int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, enum cil_flavor expr_flavor);
> > > int cil_verify_constraint_leaf_expr_syntax(enum cil_flavor l_flavor, enum cil_flavor r_flavor, enum cil_flavor op, enum cil_flavor expr_flavor);
> > > --
> > > 2.26.2
> > >
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-19 15:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 19:08 [PATCH] libsepol/cil: Exit with an error if declaration name is a reserved word James Carter
2021-03-18 22:11 ` Nicolas Iooss
2021-03-19 13:46 ` James Carter
2021-03-19 15:16 ` James Carter
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.