diff --git a/SOURCES/0001-libsepol-cil-Fix-out-of-bound-read-of-file-context-p.patch b/SOURCES/0001-libsepol-cil-Fix-out-of-bound-read-of-file-context-p.patch new file mode 100644 index 0000000..dc2da32 --- /dev/null +++ b/SOURCES/0001-libsepol-cil-Fix-out-of-bound-read-of-file-context-p.patch @@ -0,0 +1,50 @@ +From 2b2f42f9311ede75c3fe61d356094999e8e161b9 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Thu, 8 Apr 2021 13:24:29 -0400 +Subject: [PATCH] libsepol/cil: Fix out-of-bound read of file context pattern + ending with "\" + +Based on patch by Nicolas Iooss, who writes: + OSS-Fuzz found a Heap-buffer-overflow in the CIL compiler when trying + to compile the following policy: + + (sid SID) + (sidorder(SID)) + (filecon "\" any ()) + (filecon "" any ()) + + When cil_post_fc_fill_data() processes "\", it goes beyond the NUL + terminator of the string. Fix this by returning when '\0' is read + after a backslash. + +To be consistent with the function compute_diffdata() in +refpolicy/support/fc_sort.py, also increment str_len in this case. + +Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28484 +Reported-by: Nicolas Iooss +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_post.c | 7 +++++++ + 1 file changed, 7 insertions(+) + +diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c +index 0b09cecc..bdeaa7c6 100644 +--- a/libsepol/cil/src/cil_post.c ++++ b/libsepol/cil/src/cil_post.c +@@ -179,6 +179,13 @@ void cil_post_fc_fill_data(struct fc_data *fc, char *path) + break; + case '\\': + c++; ++ if (path[c] == '\0') { ++ if (!fc->meta) { ++ fc->stem_len++; ++ } ++ fc->str_len++; ++ return; ++ } + /* FALLTHRU */ + default: + if (!fc->meta) { +-- +2.30.2 + diff --git a/SOURCES/0002-libsepol-cil-Destroy-classperms-list-when-resetting-.patch b/SOURCES/0002-libsepol-cil-Destroy-classperms-list-when-resetting-.patch new file mode 100644 index 0000000..185e1ac --- /dev/null +++ b/SOURCES/0002-libsepol-cil-Destroy-classperms-list-when-resetting-.patch @@ -0,0 +1,100 @@ +From 5012fee580f5e4744166462855767949311f9154 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Thu, 8 Apr 2021 13:32:01 -0400 +Subject: [PATCH] libsepol/cil: Destroy classperms list when resetting + classpermission + +Nicolas Iooss reports: + A few months ago, OSS-Fuzz found a crash in the CIL compiler, which + got reported as + https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28648 (the title + is misleading, or is caused by another issue that conflicts with the + one I report in this message). Here is a minimized CIL policy which + reproduces the issue: + + (class CLASS (PERM)) + (classorder (CLASS)) + (sid SID) + (sidorder (SID)) + (user USER) + (role ROLE) + (type TYPE) + (category CAT) + (categoryorder (CAT)) + (sensitivity SENS) + (sensitivityorder (SENS)) + (sensitivitycategory SENS (CAT)) + (allow TYPE self (CLASS (PERM))) + (roletype ROLE TYPE) + (userrole USER ROLE) + (userlevel USER (SENS)) + (userrange USER ((SENS)(SENS (CAT)))) + (sidcontext SID (USER ROLE TYPE ((SENS)(SENS)))) + + (classpermission CLAPERM) + + (optional OPT + (roletype nonexistingrole nonexistingtype) + (classpermissionset CLAPERM (CLASS (PERM))) + ) + + The CIL policy fuzzer (which mimics secilc built with clang Address + Sanitizer) reports: + + ==36541==ERROR: AddressSanitizer: heap-use-after-free on address + 0x603000004f98 at pc 0x56445134c842 bp 0x7ffe2a256590 sp + 0x7ffe2a256588 + READ of size 8 at 0x603000004f98 thread T0 + #0 0x56445134c841 in __cil_verify_classperms + /selinux/libsepol/src/../cil/src/cil_verify.c:1620:8 + #1 0x56445134a43e in __cil_verify_classpermission + /selinux/libsepol/src/../cil/src/cil_verify.c:1650:9 + #2 0x56445134a43e in __cil_pre_verify_helper + /selinux/libsepol/src/../cil/src/cil_verify.c:1715:8 + #3 0x5644513225ac in cil_tree_walk_core + /selinux/libsepol/src/../cil/src/cil_tree.c:272:9 + #4 0x564451322ab1 in cil_tree_walk + /selinux/libsepol/src/../cil/src/cil_tree.c:316:7 + #5 0x5644513226af in cil_tree_walk_core + /selinux/libsepol/src/../cil/src/cil_tree.c:284:9 + #6 0x564451322ab1 in cil_tree_walk + /selinux/libsepol/src/../cil/src/cil_tree.c:316:7 + #7 0x5644512b88fd in cil_pre_verify + /selinux/libsepol/src/../cil/src/cil_post.c:2510:7 + #8 0x5644512b88fd in cil_post_process + /selinux/libsepol/src/../cil/src/cil_post.c:2524:7 + #9 0x5644511856ff in cil_compile + /selinux/libsepol/src/../cil/src/cil.c:564:7 + +The classperms list of a classpermission rule is created and filled +in when classpermissionset rules are processed, so it doesn't own any +part of the list and shouldn't retain any of it when it is reset. + +Destroy the classperms list (without destroying the data in it) when +resetting a classpermission rule. + +Reported-by: Nicolas Iooss +Signed-off-by: James Carter + +(cherry-picked from SElinuxProject + commit: f34d3d30c8325e4847a6b696fe7a3936a8a361f3) +--- + libsepol/cil/src/cil_reset_ast.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_reset_ast.c b/libsepol/cil/src/cil_reset_ast.c +index 43e6b88e..73e1fcf0 100644 +--- a/libsepol/cil/src/cil_reset_ast.c ++++ b/libsepol/cil/src/cil_reset_ast.c +@@ -52,7 +52,7 @@ static void cil_reset_classpermission(struct cil_classpermission *cp) + return; + } + +- cil_reset_classperms_list(cp->classperms); ++ cil_list_destroy(&cp->classperms, CIL_FALSE); + } + + static void cil_reset_classperms_set(struct cil_classperms_set *cp_set) +-- +2.30.2 + diff --git a/SOURCES/0003-libsepol-cil-Destroy-classperm-list-when-resetting-m.patch b/SOURCES/0003-libsepol-cil-Destroy-classperm-list-when-resetting-m.patch new file mode 100644 index 0000000..27d91ae --- /dev/null +++ b/SOURCES/0003-libsepol-cil-Destroy-classperm-list-when-resetting-m.patch @@ -0,0 +1,39 @@ +From 8c8a21d4ca75e4b767d3dfaa181a83e4c0e1f3a1 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Thu, 8 Apr 2021 13:32:04 -0400 +Subject: [PATCH] libsepol/cil: Destroy classperm list when resetting map perms + +Map perms share the same struct as regular perms, but only the +map perms use the classperms field. This field is a pointer to a +list of classperms that is created and added to when resolving +classmapping rules, so the map permission doesn't own any of the +data in the list and this list should be destroyed when the AST is +reset. + +When resetting a perm, destroy the classperms list without destroying +the data in the list. + +Signed-off-by: James Carter + +(cherry-picked from SElinuxProject + commit: 2d35fcc7e9e976a2346b1de20e54f8663e8a6cba) +--- + libsepol/cil/src/cil_reset_ast.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_reset_ast.c b/libsepol/cil/src/cil_reset_ast.c +index 73e1fcf0..f321b396 100644 +--- a/libsepol/cil/src/cil_reset_ast.c ++++ b/libsepol/cil/src/cil_reset_ast.c +@@ -34,7 +34,7 @@ static void cil_reset_class(struct cil_class *class) + + static void cil_reset_perm(struct cil_perm *perm) + { +- cil_reset_classperms_list(perm->classperms); ++ cil_list_destroy(&perm->classperms, CIL_FALSE); + } + + static inline void cil_reset_classperms(struct cil_classperms *cp) +-- +2.30.2 + diff --git a/SOURCES/0004-libsepol-cil-cil_reset_classperms_set-should-not-res.patch b/SOURCES/0004-libsepol-cil-cil_reset_classperms_set-should-not-res.patch new file mode 100644 index 0000000..bc37ffb --- /dev/null +++ b/SOURCES/0004-libsepol-cil-cil_reset_classperms_set-should-not-res.patch @@ -0,0 +1,42 @@ +From 52bf0fe9ce922229e8bb3b99faa7c7dce2c3531f Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Thu, 8 Apr 2021 13:32:06 -0400 +Subject: [PATCH] libsepol/cil: cil_reset_classperms_set() should not reset + classpermission + +In struct cil_classperms_set, the set field is a pointer to a +struct cil_classpermission which is looked up in the symbol table. +Since the cil_classperms_set does not create the cil_classpermission, +it should not reset it. + +Set the set field to NULL instead of resetting the classpermission +that it points to. + +Signed-off-by: James Carter + +(cherry-picked from SElinuxProject + commit: c49a8ea09501ad66e799ea41b8154b6770fec2c8) +--- + libsepol/cil/src/cil_reset_ast.c | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_reset_ast.c b/libsepol/cil/src/cil_reset_ast.c +index f321b396..7bf0391b 100644 +--- a/libsepol/cil/src/cil_reset_ast.c ++++ b/libsepol/cil/src/cil_reset_ast.c +@@ -57,7 +57,11 @@ static void cil_reset_classpermission(struct cil_classpermission *cp) + + static void cil_reset_classperms_set(struct cil_classperms_set *cp_set) + { +- cil_reset_classpermission(cp_set->set); ++ if (cp_set == NULL) { ++ return; ++ } ++ ++ cp_set->set = NULL; + } + + static inline void cil_reset_classperms_list(struct cil_list *cp_list) +-- +2.30.2 + diff --git a/SOURCES/0005-libsepol-cil-Set-class-field-to-NULL-when-resetting-.patch b/SOURCES/0005-libsepol-cil-Set-class-field-to-NULL-when-resetting-.patch new file mode 100644 index 0000000..ec9349b --- /dev/null +++ b/SOURCES/0005-libsepol-cil-Set-class-field-to-NULL-when-resetting-.patch @@ -0,0 +1,32 @@ +From 6beea9f422cb452c01a24619247b559b67a4aeec Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Thu, 8 Apr 2021 13:32:08 -0400 +Subject: [PATCH] libsepol/cil: Set class field to NULL when resetting struct + cil_classperms + +The class field of a struct cil_classperms points to the class looked +up in the symbol table, so that field should be set to NULL when +the cil_classperms is reset. + +Set the class field to NULL when resetting the struct cil_classperms. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_reset_ast.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/libsepol/cil/src/cil_reset_ast.c b/libsepol/cil/src/cil_reset_ast.c +index 7bf0391b..e86ee3b8 100644 +--- a/libsepol/cil/src/cil_reset_ast.c ++++ b/libsepol/cil/src/cil_reset_ast.c +@@ -43,6 +43,7 @@ static inline void cil_reset_classperms(struct cil_classperms *cp) + return; + } + ++ cp->class = NULL; + cil_list_destroy(&cp->perms, CIL_FALSE); + } + +-- +2.30.2 + diff --git a/SOURCES/0006-libsepol-cil-More-strict-verification-of-constraint-.patch b/SOURCES/0006-libsepol-cil-More-strict-verification-of-constraint-.patch new file mode 100644 index 0000000..3719767 --- /dev/null +++ b/SOURCES/0006-libsepol-cil-More-strict-verification-of-constraint-.patch @@ -0,0 +1,42 @@ +From e42e31d865be8dbb5ea1b99ffab434fcfec14df2 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Thu, 8 Apr 2021 13:32:11 -0400 +Subject: [PATCH] libsepol/cil: More strict verification of constraint leaf + expressions + +In constraint expressions u1, u3, r1, r3, t1, and t3 are never +allowed on the right side of an expression, but there were no checks +to verify that they were not used on the right side. The result was +that the expression "(eq t1 t1)" would be silently turned into +"(eq t1 t2)" when the binary policy was created. + +Verify that u1, u3, r1, r3, t1, and t3 are not used on the right +side of a constraint expression. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_verify.c | 8 +++++++- + 1 file changed, 7 insertions(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c +index 1036d73c..3972b1e9 100644 +--- a/libsepol/cil/src/cil_verify.c ++++ b/libsepol/cil/src/cil_verify.c +@@ -227,7 +227,13 @@ int cil_verify_constraint_leaf_expr_syntax(enum cil_flavor l_flavor, enum cil_fl + } + } + } else { +- if (r_flavor == CIL_CONS_U2) { ++ if (r_flavor == CIL_CONS_U1 || r_flavor == CIL_CONS_R1 || r_flavor == CIL_CONS_T1) { ++ cil_log(CIL_ERR, "u1, r1, and t1 are not allowed on the right side\n"); ++ goto exit; ++ } else if (r_flavor == CIL_CONS_U3 || r_flavor == CIL_CONS_R3 || r_flavor == CIL_CONS_T3) { ++ cil_log(CIL_ERR, "u3, r3, and t3 are not allowed on the right side\n"); ++ goto exit; ++ } else if (r_flavor == CIL_CONS_U2) { + if (op != CIL_EQ && op != CIL_NEQ) { + cil_log(CIL_ERR, "u2 on the right side must be used with eq or neq as the operator\n"); + goto exit; +-- +2.30.2 + diff --git a/SOURCES/0007-libsepol-cil-Exit-with-an-error-if-declaration-name-.patch b/SOURCES/0007-libsepol-cil-Exit-with-an-error-if-declaration-name-.patch new file mode 100644 index 0000000..c6ab282 --- /dev/null +++ b/SOURCES/0007-libsepol-cil-Exit-with-an-error-if-declaration-name-.patch @@ -0,0 +1,194 @@ +From 5edd2126ad3dc30f75f0ec9f73cd609bbe432c29 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Thu, 8 Apr 2021 13:32:12 -0400 +Subject: [PATCH] libsepol/cil: Exit with an error if declaration name is a + reserved word + +When CIL parses sets or conditional expressions, any identifier that +matches an operator name will always be taken as an operator. If a +declaration has the same name as an operator, then there is the +possibility of causing either confusion or a syntax error if it is +used in an expression. The potential for problems is much greater +than any possible advantage in allowing a declaration to share the +name of a reserved word. + +Create a new function, __cil_is_reserved_name() that is called when +an identifier is declared and its name is being validated. In this +function, check if the declaration 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, move the check for types, type aliases, and type attributes +matching the reserved word "self" to this new function. + +Finally, 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 +--- + libsepol/cil/src/cil_build_ast.c | 28 ++--------------- + libsepol/cil/src/cil_verify.c | 52 +++++++++++++++++++++++++++++++- + libsepol/cil/src/cil_verify.h | 2 +- + 3 files changed, 54 insertions(+), 28 deletions(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index b90b0f60..fe7b7777 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -110,7 +110,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; + } +@@ -1919,12 +1919,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; +@@ -2303,12 +2297,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; +@@ -2357,12 +2345,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; +@@ -3064,12 +3046,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; +@@ -5294,7 +5270,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 3972b1e9..ea95c2cb 100644 +--- a/libsepol/cil/src/cil_verify.c ++++ b/libsepol/cil/src/cil_verify.c +@@ -47,7 +47,51 @@ + + #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: ++ case CIL_USER: ++ case CIL_USERATTRIBUTE: ++ case CIL_ROLE: ++ case CIL_ROLEATTRIBUTE: ++ if (name == CIL_KEY_ALL) ++ return CIL_TRUE; ++ break; ++ case CIL_TYPE: ++ case CIL_TYPEATTRIBUTE: ++ case CIL_TYPEALIAS: ++ if ((name == CIL_KEY_ALL) || (name == CIL_KEY_SELF)) ++ 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: ++ /* All of these are not used in expressions */ ++ return CIL_FALSE; ++ break; ++ } ++ ++ /* Everything not under the default case is also checked for these */ ++ 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 +121,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 bda1565f..e4b98919 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.30.2 + diff --git a/SOURCES/0008-libsepol-cil-Allow-permission-expressions-when-using.patch b/SOURCES/0008-libsepol-cil-Allow-permission-expressions-when-using.patch new file mode 100644 index 0000000..094448b --- /dev/null +++ b/SOURCES/0008-libsepol-cil-Allow-permission-expressions-when-using.patch @@ -0,0 +1,75 @@ +From d6863cc6e4f472444a7944c9ea95333e587efd73 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Thu, 8 Apr 2021 13:32:14 -0400 +Subject: [PATCH] libsepol/cil: Allow permission expressions when using map + classes + +The following policy will cause a segfault: + (class CLASS (PERM)) + (class C (P1 P2 P3)) + (classorder (CLASS C)) + (sid SID) + (sidorder (SID)) + (user USER) + (role ROLE) + (type TYPE) + (category CAT) + (categoryorder (CAT)) + (sensitivity SENS) + (sensitivityorder (SENS)) + (sensitivitycategory SENS (CAT)) + (allow TYPE self (CLASS (PERM))) + (roletype ROLE TYPE) + (userrole USER ROLE) + (userlevel USER (SENS)) + (userrange USER ((SENS)(SENS (CAT)))) + (sidcontext SID (USER ROLE TYPE ((SENS)(SENS)))) + + (classmap CM (PM1 PM2 PM3)) + (classmapping CM PM1 (C (P1))) + (classmapping CM PM2 (C (P2))) + (classmapping CM PM3 (C (P3))) + (allow TYPE self (CM (and (all) (not PM2)))) + +The problem is that, while permission expressions are allowed for +normal classes, map classes are expected to only have permission +lists and no check is done to verify that only a permission list +is being used. + +When the above policy is parsed, the "and" and "all" are seen as +expression operators, but when the map permissions are converted to +normal class and permissions, the permission expression is assumed +to be a list of datums and since the operators are not datums a +segfault is the result. + +There is no reason to limit map classes to only using a list of +permissions and, in fact, it would be better to be able to use them +in the same way normal classes are used. + +Allow permissions expressions to be used for map classes by first +evaluating the permission expression and then converting the +resulting list to normal classes and permissions. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_post.c | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c +index bdeaa7c6..a820d5ba 100644 +--- a/libsepol/cil/src/cil_post.c ++++ b/libsepol/cil/src/cil_post.c +@@ -2138,6 +2138,10 @@ static int __evaluate_classperms_list(struct cil_list *classperms, struct cil_db + } + } else { /* MAP */ + struct cil_list_item *i = NULL; ++ rc = __evaluate_classperms(cp, db); ++ if (rc != SEPOL_OK) { ++ goto exit; ++ } + cil_list_for_each(i, cp->perms) { + struct cil_perm *cmp = i->data; + rc = __evaluate_classperms_list(cmp->classperms, db); +-- +2.30.2 + diff --git a/SOURCES/0009-libsepol-cil-Reorder-checks-for-invalid-rules-when-b.patch b/SOURCES/0009-libsepol-cil-Reorder-checks-for-invalid-rules-when-b.patch new file mode 100644 index 0000000..afeaf6b --- /dev/null +++ b/SOURCES/0009-libsepol-cil-Reorder-checks-for-invalid-rules-when-b.patch @@ -0,0 +1,219 @@ +From 6b6a787188804cad4f7f853e95eb0a58dea7ad62 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Tue, 30 Mar 2021 13:39:12 -0400 +Subject: [PATCH] libsepol/cil: Reorder checks for invalid rules when building + AST + +Reorder checks for invalid rules in the blocks of tunableifs, +in-statements, macros, and booleanifs when building the AST for +consistency. + +Order the checks in the same order the blocks will be resolved in, +so tuanbleif, in-statement, macro, booleanif, and then non-block +rules. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_build_ast.c | 100 +++++++++++++++---------------- + 1 file changed, 50 insertions(+), 50 deletions(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index fe7b7777..6d5a57fa 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -49,10 +49,10 @@ + struct cil_args_build { + struct cil_tree_node *ast; + struct cil_db *db; +- struct cil_tree_node *macro; +- struct cil_tree_node *boolif; + struct cil_tree_node *tunif; + struct cil_tree_node *in; ++ struct cil_tree_node *macro; ++ struct cil_tree_node *boolif; + }; + + int cil_fill_list(struct cil_tree_node *current, enum cil_flavor flavor, struct cil_list **list) +@@ -6075,10 +6075,10 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + struct cil_tree_node *ast_current = NULL; + struct cil_db *db = NULL; + struct cil_tree_node *ast_node = NULL; +- struct cil_tree_node *macro = NULL; +- struct cil_tree_node *boolif = NULL; + struct cil_tree_node *tunif = NULL; + struct cil_tree_node *in = NULL; ++ struct cil_tree_node *macro = NULL; ++ struct cil_tree_node *boolif = NULL; + int rc = SEPOL_ERR; + + if (parse_current == NULL || finished == NULL || extra_args == NULL) { +@@ -6088,10 +6088,10 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + args = extra_args; + ast_current = args->ast; + db = args->db; +- macro = args->macro; +- boolif = args->boolif; + tunif = args->tunif; + in = args->in; ++ macro = args->macro; ++ boolif = args->boolif; + + if (parse_current->parent->cl_head != parse_current) { + /* ignore anything that isn't following a parenthesis */ +@@ -6108,13 +6108,31 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + goto exit; + } + ++ if (tunif != NULL) { ++ if (parse_current->data == CIL_KEY_TUNABLE) { ++ rc = SEPOL_ERR; ++ cil_tree_log(parse_current, CIL_ERR, "Found tunable"); ++ cil_log(CIL_ERR, "Tunables cannot be defined within tunableif statement\n"); ++ goto exit; ++ } ++ } ++ ++ if (in != NULL) { ++ if (parse_current->data == CIL_KEY_IN) { ++ rc = SEPOL_ERR; ++ cil_tree_log(parse_current, CIL_ERR, "Found in-statement"); ++ cil_log(CIL_ERR, "in-statements cannot be defined within in-statements\n"); ++ goto exit; ++ } ++ } ++ + if (macro != NULL) { +- if (parse_current->data == CIL_KEY_MACRO || +- parse_current->data == CIL_KEY_TUNABLE || ++ if (parse_current->data == CIL_KEY_TUNABLE || + parse_current->data == CIL_KEY_IN || + parse_current->data == CIL_KEY_BLOCK || + parse_current->data == CIL_KEY_BLOCKINHERIT || +- parse_current->data == CIL_KEY_BLOCKABSTRACT) { ++ parse_current->data == CIL_KEY_BLOCKABSTRACT || ++ parse_current->data == CIL_KEY_MACRO) { + rc = SEPOL_ERR; + cil_tree_log(parse_current, CIL_ERR, "%s is not allowed in macros", (char *)parse_current->data); + goto exit; +@@ -6122,15 +6140,15 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + } + + if (boolif != NULL) { +- if (parse_current->data != CIL_KEY_CONDTRUE && ++ if (parse_current->data != CIL_KEY_TUNABLEIF && ++ parse_current->data != CIL_KEY_CALL && ++ parse_current->data != CIL_KEY_CONDTRUE && + parse_current->data != CIL_KEY_CONDFALSE && +- parse_current->data != CIL_KEY_AUDITALLOW && +- parse_current->data != CIL_KEY_TUNABLEIF && + parse_current->data != CIL_KEY_ALLOW && + parse_current->data != CIL_KEY_DONTAUDIT && ++ parse_current->data != CIL_KEY_AUDITALLOW && + parse_current->data != CIL_KEY_TYPETRANSITION && +- parse_current->data != CIL_KEY_TYPECHANGE && +- parse_current->data != CIL_KEY_CALL) { ++ parse_current->data != CIL_KEY_TYPECHANGE) { + rc = SEPOL_ERR; + cil_tree_log(parse_current, CIL_ERR, "Found %s", (char*)parse_current->data); + if (((struct cil_booleanif*)boolif->data)->preserved_tunable) { +@@ -6144,24 +6162,6 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + } + } + +- if (tunif != NULL) { +- if (parse_current->data == CIL_KEY_TUNABLE) { +- rc = SEPOL_ERR; +- cil_tree_log(parse_current, CIL_ERR, "Found tunable"); +- cil_log(CIL_ERR, "Tunables cannot be defined within tunableif statement\n"); +- goto exit; +- } +- } +- +- if (in != NULL) { +- if (parse_current->data == CIL_KEY_IN) { +- rc = SEPOL_ERR; +- cil_tree_log(parse_current, CIL_ERR, "Found in-statement"); +- cil_log(CIL_ERR, "in-statements cannot be defined within in-statements\n"); +- goto exit; +- } +- } +- + cil_tree_node_init(&ast_node); + + ast_node->parent = ast_current; +@@ -6447,14 +6447,6 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + + if (rc == SEPOL_OK) { + if (ast_current->cl_head == NULL) { +- if (ast_current->flavor == CIL_MACRO) { +- args->macro = ast_current; +- } +- +- if (ast_current->flavor == CIL_BOOLEANIF) { +- args->boolif = ast_current; +- } +- + if (ast_current->flavor == CIL_TUNABLEIF) { + args->tunif = ast_current; + } +@@ -6463,6 +6455,14 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + args->in = ast_current; + } + ++ if (ast_current->flavor == CIL_MACRO) { ++ args->macro = ast_current; ++ } ++ ++ if (ast_current->flavor == CIL_BOOLEANIF) { ++ args->boolif = ast_current; ++ } ++ + ast_current->cl_head = ast_node; + } else { + ast_current->cl_tail->next = ast_node; +@@ -6498,14 +6498,6 @@ int __cil_build_ast_last_child_helper(struct cil_tree_node *parse_current, void + + args->ast = ast->parent; + +- if (ast->flavor == CIL_MACRO) { +- args->macro = NULL; +- } +- +- if (ast->flavor == CIL_BOOLEANIF) { +- args->boolif = NULL; +- } +- + if (ast->flavor == CIL_TUNABLEIF) { + args->tunif = NULL; + } +@@ -6514,6 +6506,14 @@ int __cil_build_ast_last_child_helper(struct cil_tree_node *parse_current, void + args->in = NULL; + } + ++ if (ast->flavor == CIL_MACRO) { ++ args->macro = NULL; ++ } ++ ++ if (ast->flavor == CIL_BOOLEANIF) { ++ args->boolif = NULL; ++ } ++ + // At this point we no longer have any need for parse_current or any of its + // siblings; they have all been converted to the appropriate AST node. The + // full parse tree will get deleted elsewhere, but in an attempt to +@@ -6538,10 +6538,10 @@ int cil_build_ast(struct cil_db *db, struct cil_tree_node *parse_tree, struct ci + + extra_args.ast = ast; + extra_args.db = db; +- extra_args.macro = NULL; +- extra_args.boolif = NULL; + extra_args.tunif = NULL; + extra_args.in = NULL; ++ extra_args.macro = NULL; ++ extra_args.boolif = NULL; + + rc = cil_tree_walk(parse_tree, __cil_build_ast_node_helper, NULL, __cil_build_ast_last_child_helper, &extra_args); + if (rc != SEPOL_OK) { +-- +2.30.2 + diff --git a/SOURCES/0010-libsepol-cil-Cleanup-build-AST-helper-functions.patch b/SOURCES/0010-libsepol-cil-Cleanup-build-AST-helper-functions.patch new file mode 100644 index 0000000..3f8a754 --- /dev/null +++ b/SOURCES/0010-libsepol-cil-Cleanup-build-AST-helper-functions.patch @@ -0,0 +1,91 @@ +From 34f3ecbcffaa0ede0252d015d381cef847432fa0 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Tue, 30 Mar 2021 13:39:13 -0400 +Subject: [PATCH] libsepol/cil: Cleanup build AST helper functions + +Since parse_current, finished, and extra_args can never be NULL, +remove the useless check and directly assign local variables from +extra_args. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_build_ast.c | 44 ++++++++------------------------ + 1 file changed, 10 insertions(+), 34 deletions(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index 6d5a57fa..b7245dbc 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -6071,28 +6071,16 @@ void cil_destroy_src_info(struct cil_src_info *info) + + int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *finished, void *extra_args) + { +- struct cil_args_build *args = NULL; +- struct cil_tree_node *ast_current = NULL; +- struct cil_db *db = NULL; ++ struct cil_args_build *args = extra_args; ++ struct cil_db *db = args->db; ++ struct cil_tree_node *ast_current = args->ast; ++ struct cil_tree_node *tunif = args->tunif; ++ struct cil_tree_node *in = args->in; ++ struct cil_tree_node *macro = args->macro; ++ struct cil_tree_node *boolif = args->boolif; + struct cil_tree_node *ast_node = NULL; +- struct cil_tree_node *tunif = NULL; +- struct cil_tree_node *in = NULL; +- struct cil_tree_node *macro = NULL; +- struct cil_tree_node *boolif = NULL; + int rc = SEPOL_ERR; + +- if (parse_current == NULL || finished == NULL || extra_args == NULL) { +- goto exit; +- } +- +- args = extra_args; +- ast_current = args->ast; +- db = args->db; +- tunif = args->tunif; +- in = args->in; +- macro = args->macro; +- boolif = args->boolif; +- + if (parse_current->parent->cl_head != parse_current) { + /* ignore anything that isn't following a parenthesis */ + rc = SEPOL_OK; +@@ -6480,20 +6468,11 @@ exit: + + int __cil_build_ast_last_child_helper(struct cil_tree_node *parse_current, void *extra_args) + { +- int rc = SEPOL_ERR; +- struct cil_tree_node *ast = NULL; +- struct cil_args_build *args = NULL; +- +- if (extra_args == NULL) { +- goto exit; +- } +- +- args = extra_args; +- ast = args->ast; ++ struct cil_args_build *args = extra_args; ++ struct cil_tree_node *ast = args->ast; + + if (ast->flavor == CIL_ROOT) { +- rc = SEPOL_OK; +- goto exit; ++ return SEPOL_OK; + } + + args->ast = ast->parent; +@@ -6522,9 +6501,6 @@ int __cil_build_ast_last_child_helper(struct cil_tree_node *parse_current, void + cil_tree_children_destroy(parse_current->parent); + + return SEPOL_OK; +- +-exit: +- return rc; + } + + int cil_build_ast(struct cil_db *db, struct cil_tree_node *parse_tree, struct cil_tree_node *ast) +-- +2.30.2 + diff --git a/SOURCES/0011-libsepol-cil-Create-new-first-child-helper-function-.patch b/SOURCES/0011-libsepol-cil-Create-new-first-child-helper-function-.patch new file mode 100644 index 0000000..3df60b7 --- /dev/null +++ b/SOURCES/0011-libsepol-cil-Create-new-first-child-helper-function-.patch @@ -0,0 +1,98 @@ +From 3e82b1e527fab1fb1dbcad8c70bdb59810a98783 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Tue, 30 Mar 2021 13:39:14 -0400 +Subject: [PATCH] libsepol/cil: Create new first child helper function for + building AST + +In order to find statements not allowed in tunableifs, in-statements, +macros, and booleanifs, there are tree node pointers that point to +each of these kinds of statements when its block is being parsed. +If the pointer is non-NULL, then the rule being parsed is in the block +of that kind of statement. + +The tree node pointers were being updated at the wrong point which +prevented an invalid statement from being found if it was the first +statement in the block of a tunableif, in-statement, macro, or +booleanif. + +Create a first child helper function for walking the parse tree and +in that function set the appropriate tree node pointer if the +current AST node is a tunableif, in-statement, macro, or booleanif. +This also makes the code symmetrical with the last child helper +where the tree node pointers are set to NULL. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_build_ast.c | 42 +++++++++++++++++++------------- + 1 file changed, 25 insertions(+), 17 deletions(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index b7245dbc..ceb55324 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -6435,22 +6435,6 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + + if (rc == SEPOL_OK) { + if (ast_current->cl_head == NULL) { +- if (ast_current->flavor == CIL_TUNABLEIF) { +- args->tunif = ast_current; +- } +- +- if (ast_current->flavor == CIL_IN) { +- args->in = ast_current; +- } +- +- if (ast_current->flavor == CIL_MACRO) { +- args->macro = ast_current; +- } +- +- if (ast_current->flavor == CIL_BOOLEANIF) { +- args->boolif = ast_current; +- } +- + ast_current->cl_head = ast_node; + } else { + ast_current->cl_tail->next = ast_node; +@@ -6466,6 +6450,30 @@ exit: + return rc; + } + ++int __cil_build_ast_first_child_helper(__attribute__((unused)) struct cil_tree_node *parse_current, void *extra_args) ++{ ++ struct cil_args_build *args = extra_args; ++ struct cil_tree_node *ast = args->ast; ++ ++ if (ast->flavor == CIL_TUNABLEIF) { ++ args->tunif = ast; ++ } ++ ++ if (ast->flavor == CIL_IN) { ++ args->in = ast; ++ } ++ ++ if (ast->flavor == CIL_MACRO) { ++ args->macro = ast; ++ } ++ ++ if (ast->flavor == CIL_BOOLEANIF) { ++ args->boolif = ast; ++ } ++ ++ return SEPOL_OK; ++} ++ + int __cil_build_ast_last_child_helper(struct cil_tree_node *parse_current, void *extra_args) + { + struct cil_args_build *args = extra_args; +@@ -6519,7 +6527,7 @@ int cil_build_ast(struct cil_db *db, struct cil_tree_node *parse_tree, struct ci + extra_args.macro = NULL; + extra_args.boolif = NULL; + +- rc = cil_tree_walk(parse_tree, __cil_build_ast_node_helper, NULL, __cil_build_ast_last_child_helper, &extra_args); ++ rc = cil_tree_walk(parse_tree, __cil_build_ast_node_helper, __cil_build_ast_first_child_helper, __cil_build_ast_last_child_helper, &extra_args); + if (rc != SEPOL_OK) { + goto exit; + } +-- +2.30.2 + diff --git a/SOURCES/0012-libsepol-cil-Remove-unused-field-from-struct-cil_arg.patch b/SOURCES/0012-libsepol-cil-Remove-unused-field-from-struct-cil_arg.patch new file mode 100644 index 0000000..efda5e2 --- /dev/null +++ b/SOURCES/0012-libsepol-cil-Remove-unused-field-from-struct-cil_arg.patch @@ -0,0 +1,47 @@ +From 628f0f60995c2ed6d2de72bda34e6a62668be74b Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Mon, 16 Nov 2020 17:06:59 -0500 +Subject: [PATCH] libsepol/cil: Remove unused field from struct + cil_args_resolve + +When resolving names, the struct cil_args_resolve is passed to the +various resolve functions. The field last_resolved_name is not used. + +Remove the last_resolved_name field from struct cil_args_resolve. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_resolve_ast.c | 4 ---- + 1 file changed, 4 deletions(-) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index ea08087d..ed876260 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -51,7 +51,6 @@ struct cil_args_resolve { + struct cil_db *db; + enum cil_pass pass; + uint32_t *changed; +- char *last_resolved_name; + struct cil_tree_node *optstack; + struct cil_tree_node *boolif; + struct cil_tree_node *macro; +@@ -3907,7 +3906,6 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + extra_args.db = db; + extra_args.pass = pass; + extra_args.changed = &changed; +- extra_args.last_resolved_name = NULL; + extra_args.optstack = NULL; + extra_args.boolif= NULL; + extra_args.macro = NULL; +@@ -4236,7 +4234,5 @@ exit: + *datum = NULL; + } + +- args->last_resolved_name = name; +- + return rc; + } +-- +2.30.2 + diff --git a/SOURCES/0013-libsepol-cil-Destroy-disabled-optional-blocks-after-.patch b/SOURCES/0013-libsepol-cil-Destroy-disabled-optional-blocks-after-.patch new file mode 100644 index 0000000..80f8dfb --- /dev/null +++ b/SOURCES/0013-libsepol-cil-Destroy-disabled-optional-blocks-after-.patch @@ -0,0 +1,115 @@ +From d668f8e3a0a0361c03881ae3f00509196eaee064 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Mon, 8 Feb 2021 11:23:42 -0500 +Subject: [PATCH] libsepol/cil: Destroy disabled optional blocks after pass is + complete + +Nicolas Iooss reports: + I am continuing to investigate OSS-Fuzz crashes and the following one + is quite complex. Here is a CIL policy which triggers a + heap-use-after-free error in the CIL compiler: + + (class CLASS (PERM2)) + (classorder (CLASS)) + (classpermission CLSPRM) + (optional o + (mlsvalidatetrans x (domby l1 h1)) + (common CLSCOMMON (PERM1)) + (classcommon CLASS CLSCOMMON) + ) + (classpermissionset CLSPRM (CLASS (PERM1))) + + The issue is that the mlsvalidatetrans fails to resolve in pass + CIL_PASS_MISC3, which comes after the resolution of classcommon (in + pass CIL_PASS_MISC2). So: + + * In pass CIL_PASS_MISC2, the optional block still exists, the + classcommon is resolved and class CLASS is linked with common + CLSCOMMON. + * In pass CIL_PASS_MISC3, the optional block is destroyed, including + the common CLSCOMMON. + * When classpermissionset is resolved, function cil_resolve_classperms + uses "common_symtab = &class->common->perms;", which has been freed. + The use-after-free issue occurs in __cil_resolve_perms (in + libsepol/cil/src/cil_resolve_ast.c): + + // common_symtab was freed + rc = cil_symtab_get_datum(common_symtab, curr->data, &perm_datum); + +The fundamental problem here is that when the optional block is +disabled it is immediately destroyed in the middle of the pass, so +the class has not been reset and still refers to the now destroyed +common when the classpermissionset is resolved later in the same pass. + +Added a list, disabled_optionals, to struct cil_args_resolve which is +passed when resolving the tree. When optionals are disabled, they are +now added to this list and then are destroyed after the tree has been +reset between passes. + +Reported-by: Nicolas Iooss +Signed-off-by: James Carter +Acked-by: Nicolas Iooss +--- + libsepol/cil/src/cil_resolve_ast.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index ed876260..979fa17d 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -51,6 +51,7 @@ struct cil_args_resolve { + struct cil_db *db; + enum cil_pass pass; + uint32_t *changed; ++ struct cil_list *disabled_optionals; + struct cil_tree_node *optstack; + struct cil_tree_node *boolif; + struct cil_tree_node *macro; +@@ -3854,7 +3855,7 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext + + if (((struct cil_optional *)parent->data)->enabled == CIL_FALSE) { + *(args->changed) = CIL_TRUE; +- cil_tree_children_destroy(parent); ++ cil_list_append(args->disabled_optionals, CIL_NODE, parent); + } + + /* pop off the stack */ +@@ -3917,6 +3918,7 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + extra_args.in_list = NULL; + extra_args.blockstack = NULL; + ++ cil_list_init(&extra_args.disabled_optionals, CIL_NODE); + cil_list_init(&extra_args.sidorder_lists, CIL_LIST_ITEM); + cil_list_init(&extra_args.classorder_lists, CIL_LIST_ITEM); + cil_list_init(&extra_args.unordered_classorder_lists, CIL_LIST_ITEM); +@@ -3984,6 +3986,7 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + } + + if (changed && (pass > CIL_PASS_CALL1)) { ++ struct cil_list_item *item; + /* Need to re-resolve because an optional was disabled that contained + * one or more declarations. We only need to reset to the call1 pass + * because things done in the preceeding passes aren't allowed in +@@ -4012,6 +4015,11 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + cil_log(CIL_ERR, "Failed to reset declarations\n"); + goto exit; + } ++ cil_list_for_each(item, extra_args.disabled_optionals) { ++ cil_tree_children_destroy(item->data); ++ } ++ cil_list_destroy(&extra_args.disabled_optionals, CIL_FALSE); ++ cil_list_init(&extra_args.disabled_optionals, CIL_NODE); + } + + /* reset the arguments */ +@@ -4040,6 +4048,7 @@ exit: + __cil_ordered_lists_destroy(&extra_args.catorder_lists); + __cil_ordered_lists_destroy(&extra_args.sensitivityorder_lists); + __cil_ordered_lists_destroy(&extra_args.unordered_classorder_lists); ++ cil_list_destroy(&extra_args.disabled_optionals, CIL_FALSE); + cil_list_destroy(&extra_args.in_list, CIL_FALSE); + + return rc; +-- +2.30.2 + diff --git a/SOURCES/0014-libsepol-cil-Check-if-name-is-a-macro-parameter-firs.patch b/SOURCES/0014-libsepol-cil-Check-if-name-is-a-macro-parameter-firs.patch new file mode 100644 index 0000000..876dc94 --- /dev/null +++ b/SOURCES/0014-libsepol-cil-Check-if-name-is-a-macro-parameter-firs.patch @@ -0,0 +1,63 @@ +From 8d7ed6eb2c396d64b1a8f6d516cb9f6f86ba2ece Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Wed, 4 Mar 2020 16:28:11 -0500 +Subject: [PATCH] libsepol/cil: Check if name is a macro parameter first + +Type transition file names are stored in a symbol table. Before the +name is added, the symbol table is searched to see if the name had +already been inserted. If it has, then the already existing datum is +returned. If it has not, then the name is added if either the +typetransition rule does not occur in a macro or the name is not one +of the macro parameters. + +Checking for a previous insertion before checking if the name is a +macro parameter can cause a macro parameter to be treated as the +actual name if a previous type transition file name is the same as +the parameter. + +Now check the name to see if it a macro paramter before checking for +its existence in the symbol table. + +Signed-off-by: James Carter +Acked-by: Ondrej Mosnacek +--- + libsepol/cil/src/cil_resolve_ast.c | 16 ++++++++-------- + 1 file changed, 8 insertions(+), 8 deletions(-) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index 979fa17d..ae334620 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -76,14 +76,6 @@ static struct cil_name * __cil_insert_name(struct cil_db *db, hashtab_key_t key, + enum cil_sym_index sym_index; + struct cil_symtab_datum *datum = NULL; + +- cil_flavor_to_symtab_index(CIL_NAME, &sym_index); +- symtab = &((struct cil_root *)db->ast->root->data)->symtab[sym_index]; +- +- cil_symtab_get_datum(symtab, key, &datum); +- if (datum != NULL) { +- return (struct cil_name *)datum; +- } +- + if (parent->flavor == CIL_CALL) { + struct cil_call *call = parent->data; + macro = call->macro; +@@ -99,6 +91,14 @@ static struct cil_name * __cil_insert_name(struct cil_db *db, hashtab_key_t key, + } + } + ++ cil_flavor_to_symtab_index(CIL_NAME, &sym_index); ++ symtab = &((struct cil_root *)db->ast->root->data)->symtab[sym_index]; ++ ++ cil_symtab_get_datum(symtab, key, &datum); ++ if (datum != NULL) { ++ return (struct cil_name *)datum; ++ } ++ + cil_name_init(&name); + cil_symtab_insert(symtab, key, (struct cil_symtab_datum *)name, ast_node); + cil_list_append(db->names, CIL_NAME, name); +-- +2.30.2 + diff --git a/SOURCES/0015-libsepol-cil-fix-NULL-pointer-dereference-in-__cil_i.patch b/SOURCES/0015-libsepol-cil-fix-NULL-pointer-dereference-in-__cil_i.patch new file mode 100644 index 0000000..78f264a --- /dev/null +++ b/SOURCES/0015-libsepol-cil-fix-NULL-pointer-dereference-in-__cil_i.patch @@ -0,0 +1,42 @@ +From 7cb30b316eda0b2aa8adeaba28a8afe15fc58c28 Mon Sep 17 00:00:00 2001 +From: Nicolas Iooss +Date: Sun, 14 Mar 2021 19:25:58 +0100 +Subject: [PATCH] libsepol/cil: fix NULL pointer dereference in + __cil_insert_name + +OSS-Fuzz found a Null-dereference in __cil_insert_name when trying to +compile the following policy: + + (macro MACRO () + (classmap CLASS (PERM)) + (type TYPE) + (typetransition TYPE TYPE CLASS "name" TYPE) + ) + (call MACRO) + +When using a macro with no argument, macro->params is NULL and +cil_list_for_each(item, macro->params) dereferenced a NULL pointer. +Fix this by checking that macro->params is not NULL before using it. + +Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28565 +Signed-off-by: Nicolas Iooss +--- + libsepol/cil/src/cil_resolve_ast.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index ae334620..91e08633 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -82,7 +82,7 @@ static struct cil_name * __cil_insert_name(struct cil_db *db, hashtab_key_t key, + } else if (parent->flavor == CIL_MACRO) { + macro = parent->data; + } +- if (macro != NULL) { ++ if (macro != NULL && macro->params != NULL) { + struct cil_list_item *item; + cil_list_for_each(item, macro->params) { + if (((struct cil_param*)item->data)->str == key) { +-- +2.30.2 + diff --git a/SOURCES/0016-libsepol-cil-Report-disabling-an-optional-block-only.patch b/SOURCES/0016-libsepol-cil-Report-disabling-an-optional-block-only.patch new file mode 100644 index 0000000..b6ba205 --- /dev/null +++ b/SOURCES/0016-libsepol-cil-Report-disabling-an-optional-block-only.patch @@ -0,0 +1,43 @@ +From 2f9ce13779d3b92198e60cdbd3d19e7c08b5457f Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Fri, 1 Nov 2019 09:50:53 -0400 +Subject: [PATCH] libsepol/cil: Report disabling an optional block only at high + verbose levels + +Since failing to resolve a statement in an optional block is normal, +only display messages about the statement failing to resolve and the +optional block being disabled at the highest verbosity level. + +These messages are now only at log level CIL_INFO instead of CIL_WARN. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_resolve_ast.c | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index 91e08633..dab8b276 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -3765,14 +3765,16 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished + enum cil_log_level lvl = CIL_ERR; + + if (optstack != NULL) { +- lvl = CIL_WARN; ++ lvl = CIL_INFO; + + struct cil_optional *opt = (struct cil_optional *)optstack->data; + struct cil_tree_node *opt_node = opt->datum.nodes->head->data; +- cil_tree_log(opt_node, lvl, "Disabling optional '%s'", opt->datum.name); + /* disable an optional if something failed to resolve */ + opt->enabled = CIL_FALSE; ++ cil_tree_log(node, lvl, "Failed to resolve %s statement", cil_node_to_string(node)); ++ cil_tree_log(opt_node, lvl, "Disabling optional '%s'", opt->datum.name); + rc = SEPOL_OK; ++ goto exit; + } + + cil_tree_log(node, lvl, "Failed to resolve %s statement", cil_node_to_string(node)); +-- +2.30.2 + diff --git a/SOURCES/0017-libsepol-cil-Use-AST-to-track-blocks-and-optionals-w.patch b/SOURCES/0017-libsepol-cil-Use-AST-to-track-blocks-and-optionals-w.patch new file mode 100644 index 0000000..e33729b --- /dev/null +++ b/SOURCES/0017-libsepol-cil-Use-AST-to-track-blocks-and-optionals-w.patch @@ -0,0 +1,249 @@ +From 599c1422479ae9dd9501c43680bf4a1667e7c951 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Tue, 30 Mar 2021 13:39:15 -0400 +Subject: [PATCH] libsepol/cil: Use AST to track blocks and optionals when + resolving + +When resolving the AST, block and optional stacks are used to +determine if the current rule being resolved is in a block or +an optional. There is no need to do this since the parent node +pointers can be used when exiting a block or an optional to +determine if resolution is still within a block or an optional. + +When entering either a block or an optional, update the appropriate +tree node pointer. When finished with the last child of a block or +optional, set the appropriate pointer to NULL. If a parent of the +same kind is found when the parent node pointers are followed back +to the root node, then set the pointer to that tree node. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_resolve_ast.c | 107 +++++++++-------------------- + 1 file changed, 32 insertions(+), 75 deletions(-) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index dab8b276..e0379782 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -52,10 +52,10 @@ struct cil_args_resolve { + enum cil_pass pass; + uint32_t *changed; + struct cil_list *disabled_optionals; +- struct cil_tree_node *optstack; ++ struct cil_tree_node *optional; + struct cil_tree_node *boolif; + struct cil_tree_node *macro; +- struct cil_tree_node *blockstack; ++ struct cil_tree_node *block; + struct cil_list *sidorder_lists; + struct cil_list *classorder_lists; + struct cil_list *unordered_classorder_lists; +@@ -3692,16 +3692,16 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished + int rc = SEPOL_ERR; + struct cil_args_resolve *args = extra_args; + enum cil_pass pass = args->pass; +- struct cil_tree_node *optstack = args->optstack; ++ struct cil_tree_node *optional = args->optional; + struct cil_tree_node *boolif = args->boolif; +- struct cil_tree_node *blockstack = args->blockstack; ++ struct cil_tree_node *block = args->block; + struct cil_tree_node *macro = args->macro; + + if (node == NULL) { + goto exit; + } + +- if (optstack != NULL) { ++ if (optional != NULL) { + if (node->flavor == CIL_TUNABLE || node->flavor == CIL_MACRO) { + /* tuanbles and macros are not allowed in optionals*/ + cil_tree_log(node, CIL_ERR, "%s statement is not allowed in optionals", cil_node_to_string(node)); +@@ -3710,7 +3710,7 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished + } + } + +- if (blockstack != NULL) { ++ if (block != NULL) { + if (node->flavor == CIL_CAT || node->flavor == CIL_SENS) { + cil_tree_log(node, CIL_ERR, "%s statement is not allowed in blocks", cil_node_to_string(node)); + rc = SEPOL_ERR; +@@ -3764,11 +3764,11 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished + if (rc == SEPOL_ENOENT) { + enum cil_log_level lvl = CIL_ERR; + +- if (optstack != NULL) { ++ if (optional != NULL) { + lvl = CIL_INFO; + +- struct cil_optional *opt = (struct cil_optional *)optstack->data; +- struct cil_tree_node *opt_node = opt->datum.nodes->head->data; ++ struct cil_optional *opt = (struct cil_optional *)optional->data; ++ struct cil_tree_node *opt_node = NODE(opt);; + /* disable an optional if something failed to resolve */ + opt->enabled = CIL_FALSE; + cil_tree_log(node, lvl, "Failed to resolve %s statement", cil_node_to_string(node)); +@@ -3791,39 +3791,18 @@ int __cil_resolve_ast_first_child_helper(struct cil_tree_node *current, void *ex + { + int rc = SEPOL_ERR; + struct cil_args_resolve *args = extra_args; +- struct cil_tree_node *optstack = NULL; + struct cil_tree_node *parent = NULL; +- struct cil_tree_node *blockstack = NULL; +- struct cil_tree_node *new = NULL; + + if (current == NULL || extra_args == NULL) { + goto exit; + } + +- optstack = args->optstack; + parent = current->parent; +- blockstack = args->blockstack; + +- if (parent->flavor == CIL_OPTIONAL || parent->flavor == CIL_BLOCK) { +- /* push this node onto a stack */ +- cil_tree_node_init(&new); +- +- new->data = parent->data; +- new->flavor = parent->flavor; +- +- if (parent->flavor == CIL_OPTIONAL) { +- if (optstack != NULL) { +- optstack->parent = new; +- new->cl_head = optstack; +- } +- args->optstack = new; +- } else if (parent->flavor == CIL_BLOCK) { +- if (blockstack != NULL) { +- blockstack->parent = new; +- new->cl_head = blockstack; +- } +- args->blockstack = new; +- } ++ if (parent->flavor == CIL_BLOCK) { ++ args->block = parent; ++ } else if (parent->flavor == CIL_OPTIONAL) { ++ args->optional = parent; + } else if (parent->flavor == CIL_BOOLEANIF) { + args->boolif = parent; + } else if (parent->flavor == CIL_MACRO) { +@@ -3842,7 +3821,6 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext + int rc = SEPOL_ERR; + struct cil_args_resolve *args = extra_args; + struct cil_tree_node *parent = NULL; +- struct cil_tree_node *blockstack = NULL; + + if (current == NULL || extra_args == NULL) { + goto exit; +@@ -3853,30 +3831,31 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext + if (parent->flavor == CIL_MACRO) { + args->macro = NULL; + } else if (parent->flavor == CIL_OPTIONAL) { +- struct cil_tree_node *optstack; +- ++ struct cil_tree_node *n = parent->parent; + if (((struct cil_optional *)parent->data)->enabled == CIL_FALSE) { + *(args->changed) = CIL_TRUE; + cil_list_append(args->disabled_optionals, CIL_NODE, parent); + } +- +- /* pop off the stack */ +- optstack = args->optstack; +- args->optstack = optstack->cl_head; +- if (optstack->cl_head) { +- optstack->cl_head->parent = NULL; ++ args->optional = NULL; ++ while (n && n->flavor != CIL_ROOT) { ++ if (n->flavor == CIL_OPTIONAL) { ++ args->optional = n; ++ break; ++ } ++ n = n->parent; + } +- free(optstack); + } else if (parent->flavor == CIL_BOOLEANIF) { + args->boolif = NULL; + } else if (parent->flavor == CIL_BLOCK) { +- /* pop off the stack */ +- blockstack = args->blockstack; +- args->blockstack = blockstack->cl_head; +- if (blockstack->cl_head) { +- blockstack->cl_head->parent = NULL; ++ struct cil_tree_node *n = parent->parent; ++ args->block = NULL; ++ while (n && n->flavor != CIL_ROOT) { ++ if (n->flavor == CIL_BLOCK) { ++ args->block = n; ++ break; ++ } ++ n = n->parent; + } +- free(blockstack); + } + + return SEPOL_OK; +@@ -3885,16 +3864,6 @@ exit: + return rc; + } + +-static void cil_destroy_tree_node_stack(struct cil_tree_node *curr) +-{ +- struct cil_tree_node *next; +- while (curr != NULL) { +- next = curr->cl_head; +- free(curr); +- curr = next; +- } +-} +- + int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + { + int rc = SEPOL_ERR; +@@ -3909,7 +3878,8 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + extra_args.db = db; + extra_args.pass = pass; + extra_args.changed = &changed; +- extra_args.optstack = NULL; ++ extra_args.block = NULL; ++ extra_args.optional = NULL; + extra_args.boolif= NULL; + extra_args.macro = NULL; + extra_args.sidorder_lists = NULL; +@@ -3918,7 +3888,6 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + extra_args.catorder_lists = NULL; + extra_args.sensitivityorder_lists = NULL; + extra_args.in_list = NULL; +- extra_args.blockstack = NULL; + + cil_list_init(&extra_args.disabled_optionals, CIL_NODE); + cil_list_init(&extra_args.sidorder_lists, CIL_LIST_ITEM); +@@ -4022,17 +3991,7 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + } + cil_list_destroy(&extra_args.disabled_optionals, CIL_FALSE); + cil_list_init(&extra_args.disabled_optionals, CIL_NODE); +- } +- +- /* reset the arguments */ +- changed = 0; +- while (extra_args.optstack != NULL) { +- cil_destroy_tree_node_stack(extra_args.optstack); +- extra_args.optstack = NULL; +- } +- while (extra_args.blockstack!= NULL) { +- cil_destroy_tree_node_stack(extra_args.blockstack); +- extra_args.blockstack = NULL; ++ changed = 0; + } + } + +@@ -4043,8 +4002,6 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + + rc = SEPOL_OK; + exit: +- cil_destroy_tree_node_stack(extra_args.optstack); +- cil_destroy_tree_node_stack(extra_args.blockstack); + __cil_ordered_lists_destroy(&extra_args.sidorder_lists); + __cil_ordered_lists_destroy(&extra_args.classorder_lists); + __cil_ordered_lists_destroy(&extra_args.catorder_lists); +-- +2.30.2 + diff --git a/SOURCES/0018-libsepol-cil-Reorder-checks-for-invalid-rules-when-r.patch b/SOURCES/0018-libsepol-cil-Reorder-checks-for-invalid-rules-when-r.patch new file mode 100644 index 0000000..cb5a832 --- /dev/null +++ b/SOURCES/0018-libsepol-cil-Reorder-checks-for-invalid-rules-when-r.patch @@ -0,0 +1,175 @@ +From 88f4d1c0b93d6a359d7fc7b2116de0da32c74ca5 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Tue, 30 Mar 2021 13:39:16 -0400 +Subject: [PATCH] libsepol/cil: Reorder checks for invalid rules when resolving + AST + +Reorder checks for invalid rules in the blocks of tunableifs, +in-statements, macros, and booleanifs when resolving the AST for +consistency. + +Order the checks in the same order the blocks will be resolved in, +so tuanbleif, in-statement, macro, booleanif, and then non-block +rules. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_resolve_ast.c | 76 +++++++++++++++--------------- + 1 file changed, 39 insertions(+), 37 deletions(-) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index e0379782..c520c44a 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -52,10 +52,10 @@ struct cil_args_resolve { + enum cil_pass pass; + uint32_t *changed; + struct cil_list *disabled_optionals; ++ struct cil_tree_node *block; ++ struct cil_tree_node *macro; + struct cil_tree_node *optional; + struct cil_tree_node *boolif; +- struct cil_tree_node *macro; +- struct cil_tree_node *block; + struct cil_list *sidorder_lists; + struct cil_list *classorder_lists; + struct cil_list *unordered_classorder_lists; +@@ -3692,50 +3692,52 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished + int rc = SEPOL_ERR; + struct cil_args_resolve *args = extra_args; + enum cil_pass pass = args->pass; +- struct cil_tree_node *optional = args->optional; +- struct cil_tree_node *boolif = args->boolif; + struct cil_tree_node *block = args->block; + struct cil_tree_node *macro = args->macro; ++ struct cil_tree_node *optional = args->optional; ++ struct cil_tree_node *boolif = args->boolif; + + if (node == NULL) { + goto exit; + } + +- if (optional != NULL) { +- if (node->flavor == CIL_TUNABLE || node->flavor == CIL_MACRO) { +- /* tuanbles and macros are not allowed in optionals*/ +- cil_tree_log(node, CIL_ERR, "%s statement is not allowed in optionals", cil_node_to_string(node)); ++ if (block != NULL) { ++ if (node->flavor == CIL_CAT || ++ node->flavor == CIL_SENS) { ++ cil_tree_log(node, CIL_ERR, "%s statement is not allowed in blocks", cil_node_to_string(node)); + rc = SEPOL_ERR; + goto exit; + } + } + +- if (block != NULL) { +- if (node->flavor == CIL_CAT || node->flavor == CIL_SENS) { +- cil_tree_log(node, CIL_ERR, "%s statement is not allowed in blocks", cil_node_to_string(node)); ++ if (macro != NULL) { ++ if (node->flavor == CIL_BLOCK || ++ node->flavor == CIL_BLOCKINHERIT || ++ node->flavor == CIL_BLOCKABSTRACT || ++ node->flavor == CIL_MACRO) { ++ cil_tree_log(node, CIL_ERR, "%s statement is not allowed in macros", cil_node_to_string(node)); + rc = SEPOL_ERR; + goto exit; + } + } + +- if (macro != NULL) { +- if (node->flavor == CIL_BLOCKINHERIT || +- node->flavor == CIL_BLOCK || +- node->flavor == CIL_BLOCKABSTRACT || +- node->flavor == CIL_MACRO) { +- cil_tree_log(node, CIL_ERR, "%s statement is not allowed in macros", cil_node_to_string(node)); ++ if (optional != NULL) { ++ if (node->flavor == CIL_TUNABLE || ++ node->flavor == CIL_MACRO) { ++ /* tuanbles and macros are not allowed in optionals*/ ++ cil_tree_log(node, CIL_ERR, "%s statement is not allowed in optionals", cil_node_to_string(node)); + rc = SEPOL_ERR; + goto exit; + } + } + + if (boolif != NULL) { +- if (!(node->flavor == CIL_CONDBLOCK || +- node->flavor == CIL_AVRULE || +- node->flavor == CIL_TYPE_RULE || +- node->flavor == CIL_CALL || +- node->flavor == CIL_TUNABLEIF || +- node->flavor == CIL_NAMETYPETRANSITION)) { ++ if (!(node->flavor == CIL_TUNABLEIF || ++ node->flavor == CIL_CALL || ++ node->flavor == CIL_CONDBLOCK || ++ node->flavor == CIL_AVRULE || ++ node->flavor == CIL_TYPE_RULE || ++ node->flavor == CIL_NAMETYPETRANSITION)) { + if (((struct cil_booleanif*)boolif->data)->preserved_tunable) { + cil_tree_log(node, CIL_ERR, "%s statement is not allowed in booleanifs (tunableif treated as a booleanif)", cil_node_to_string(node)); + } else { +@@ -3801,12 +3803,12 @@ int __cil_resolve_ast_first_child_helper(struct cil_tree_node *current, void *ex + + if (parent->flavor == CIL_BLOCK) { + args->block = parent; ++ } else if (parent->flavor == CIL_MACRO) { ++ args->macro = parent; + } else if (parent->flavor == CIL_OPTIONAL) { + args->optional = parent; + } else if (parent->flavor == CIL_BOOLEANIF) { + args->boolif = parent; +- } else if (parent->flavor == CIL_MACRO) { +- args->macro = parent; + } + + return SEPOL_OK; +@@ -3828,7 +3830,17 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext + + parent = current->parent; + +- if (parent->flavor == CIL_MACRO) { ++ if (parent->flavor == CIL_BLOCK) { ++ struct cil_tree_node *n = parent->parent; ++ args->block = NULL; ++ while (n && n->flavor != CIL_ROOT) { ++ if (n->flavor == CIL_BLOCK) { ++ args->block = n; ++ break; ++ } ++ n = n->parent; ++ } ++ } else if (parent->flavor == CIL_MACRO) { + args->macro = NULL; + } else if (parent->flavor == CIL_OPTIONAL) { + struct cil_tree_node *n = parent->parent; +@@ -3846,16 +3858,6 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext + } + } else if (parent->flavor == CIL_BOOLEANIF) { + args->boolif = NULL; +- } else if (parent->flavor == CIL_BLOCK) { +- struct cil_tree_node *n = parent->parent; +- args->block = NULL; +- while (n && n->flavor != CIL_ROOT) { +- if (n->flavor == CIL_BLOCK) { +- args->block = n; +- break; +- } +- n = n->parent; +- } + } + + return SEPOL_OK; +@@ -3879,9 +3881,9 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current) + extra_args.pass = pass; + extra_args.changed = &changed; + extra_args.block = NULL; ++ extra_args.macro = NULL; + extra_args.optional = NULL; + extra_args.boolif= NULL; +- extra_args.macro = NULL; + extra_args.sidorder_lists = NULL; + extra_args.classorder_lists = NULL; + extra_args.unordered_classorder_lists = NULL; +-- +2.30.2 + diff --git a/SOURCES/0019-libsepol-cil-Sync-checks-for-invalid-rules-in-boolea.patch b/SOURCES/0019-libsepol-cil-Sync-checks-for-invalid-rules-in-boolea.patch new file mode 100644 index 0000000..421a2c7 --- /dev/null +++ b/SOURCES/0019-libsepol-cil-Sync-checks-for-invalid-rules-in-boolea.patch @@ -0,0 +1,86 @@ +From dadf1e9ad66318fdd814cf06af2b83741467a3d8 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Tue, 30 Mar 2021 13:39:17 -0400 +Subject: [PATCH] libsepol/cil: Sync checks for invalid rules in booleanifs + +When building the AST, typemember rules in a booleanif block will +be incorrectly called invalid. They are allowed in the kernel +policy and should be allowed in CIL. + +When resolving the AST, if a neverallow rule is copied into a +booleanif block, it will not be considered an invalid rule, even +though this is not allowed in the kernel policy. + +Update the booleanif checks to allow typemember rules and to not +allow neverallow rules in booleanifs. Also use the same form of +conditional for the checks when building and resolving the AST. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_build_ast.c | 3 ++- + libsepol/cil/src/cil_resolve_ast.c | 23 +++++++++++++++-------- + 2 files changed, 17 insertions(+), 9 deletions(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index ceb55324..3a91be03 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -6136,7 +6136,8 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + parse_current->data != CIL_KEY_DONTAUDIT && + parse_current->data != CIL_KEY_AUDITALLOW && + parse_current->data != CIL_KEY_TYPETRANSITION && +- parse_current->data != CIL_KEY_TYPECHANGE) { ++ parse_current->data != CIL_KEY_TYPECHANGE && ++ parse_current->data != CIL_KEY_TYPEMEMBER) { + rc = SEPOL_ERR; + cil_tree_log(parse_current, CIL_ERR, "Found %s", (char*)parse_current->data); + if (((struct cil_booleanif*)boolif->data)->preserved_tunable) { +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index c520c44a..06b6ab48 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -3689,7 +3689,7 @@ exit: + + int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished, void *extra_args) + { +- int rc = SEPOL_ERR; ++ int rc = SEPOL_OK; + struct cil_args_resolve *args = extra_args; + enum cil_pass pass = args->pass; + struct cil_tree_node *block = args->block; +@@ -3732,18 +3732,25 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished + } + + if (boolif != NULL) { +- if (!(node->flavor == CIL_TUNABLEIF || +- node->flavor == CIL_CALL || +- node->flavor == CIL_CONDBLOCK || +- node->flavor == CIL_AVRULE || +- node->flavor == CIL_TYPE_RULE || +- node->flavor == CIL_NAMETYPETRANSITION)) { ++ if (node->flavor != CIL_TUNABLEIF && ++ node->flavor != CIL_CALL && ++ node->flavor != CIL_CONDBLOCK && ++ node->flavor != CIL_AVRULE && ++ node->flavor != CIL_TYPE_RULE && ++ node->flavor != CIL_NAMETYPETRANSITION) { ++ rc = SEPOL_ERR; ++ } else if (node->flavor == CIL_AVRULE) { ++ struct cil_avrule *rule = node->data; ++ if (rule->rule_kind == CIL_AVRULE_NEVERALLOW) { ++ rc = SEPOL_ERR; ++ } ++ } ++ if (rc == SEPOL_ERR) { + if (((struct cil_booleanif*)boolif->data)->preserved_tunable) { + cil_tree_log(node, CIL_ERR, "%s statement is not allowed in booleanifs (tunableif treated as a booleanif)", cil_node_to_string(node)); + } else { + cil_tree_log(node, CIL_ERR, "%s statement is not allowed in booleanifs", cil_node_to_string(node)); + } +- rc = SEPOL_ERR; + goto exit; + } + } +-- +2.30.2 + diff --git a/SOURCES/0020-libsepol-cil-Check-for-statements-not-allowed-in-opt.patch b/SOURCES/0020-libsepol-cil-Check-for-statements-not-allowed-in-opt.patch new file mode 100644 index 0000000..02a4356 --- /dev/null +++ b/SOURCES/0020-libsepol-cil-Check-for-statements-not-allowed-in-opt.patch @@ -0,0 +1,151 @@ +From 0e420cc6f6debc2050229ea537b592c963b81a7c Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Tue, 30 Mar 2021 13:39:18 -0400 +Subject: [PATCH] libsepol/cil: Check for statements not allowed in optional + blocks + +While there are some checks for invalid statements in an optional +block when resolving the AST, there are no checks when building the +AST. + +OSS-Fuzz found the following policy which caused a null dereference +in cil_tree_get_next_path(). + (blockinherit b3) + (sid SID) + (sidorder(SID)) + (optional o + (ibpkeycon :(1 0)s) + (block b3 + (filecon""block()) + (filecon""block()))) + +The problem is that the blockinherit copies block b3 before +the optional block is disabled. When the optional is disabled, +block b3 is deleted along with everything else in the optional. +Later, when filecon statements with the same path are found an +error message is produced and in trying to find out where the block +was copied from, the reference to the deleted block is used. The +error handling code assumes (rightly) that if something was copied +from a block then that block should still exist. + +It is clear that in-statements, blocks, and macros cannot be in an +optional, because that allows nodes to be copied from the optional +block to somewhere outside even though the optional could be disabled +later. When optionals are disabled the AST is reset and the +resolution is restarted at the point of resolving macro calls, so +anything resolved before macro calls will never be re-resolved. +This includes tunableifs, in-statements, blockinherits, +blockabstracts, and macro definitions. Tunable declarations also +cannot be in an optional block because they are needed to resolve +tunableifs. It should be fine to allow blockinherit statements in +an optional, because that is copying nodes from outside the optional +to the optional and if the optional is later disabled, everything +will be deleted anyway. + +Check and quit with an error if a tunable declaration, in-statement, +block, blockabstract, or macro definition is found within an +optional when either building or resolving the AST. + +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_build_ast.c | 32 ++++++++++++++++++++++++++++++ + libsepol/cil/src/cil_resolve_ast.c | 4 +++- + 2 files changed, 35 insertions(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index 3a91be03..4f72884c 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -52,6 +52,7 @@ struct cil_args_build { + struct cil_tree_node *tunif; + struct cil_tree_node *in; + struct cil_tree_node *macro; ++ struct cil_tree_node *optional; + struct cil_tree_node *boolif; + }; + +@@ -6077,6 +6078,7 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + struct cil_tree_node *tunif = args->tunif; + struct cil_tree_node *in = args->in; + struct cil_tree_node *macro = args->macro; ++ struct cil_tree_node *optional = args->optional; + struct cil_tree_node *boolif = args->boolif; + struct cil_tree_node *ast_node = NULL; + int rc = SEPOL_ERR; +@@ -6127,6 +6129,18 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f + } + } + ++ if (optional != NULL) { ++ if (parse_current->data == CIL_KEY_TUNABLE || ++ parse_current->data == CIL_KEY_IN || ++ parse_current->data == CIL_KEY_BLOCK || ++ parse_current->data == CIL_KEY_BLOCKABSTRACT || ++ parse_current->data == CIL_KEY_MACRO) { ++ rc = SEPOL_ERR; ++ cil_tree_log(parse_current, CIL_ERR, "%s is not allowed in optionals", (char *)parse_current->data); ++ goto exit; ++ } ++ } ++ + if (boolif != NULL) { + if (parse_current->data != CIL_KEY_TUNABLEIF && + parse_current->data != CIL_KEY_CALL && +@@ -6468,6 +6482,10 @@ int __cil_build_ast_first_child_helper(__attribute__((unused)) struct cil_tree_n + args->macro = ast; + } + ++ if (ast->flavor == CIL_OPTIONAL) { ++ args->optional = ast; ++ } ++ + if (ast->flavor == CIL_BOOLEANIF) { + args->boolif = ast; + } +@@ -6498,6 +6516,19 @@ int __cil_build_ast_last_child_helper(struct cil_tree_node *parse_current, void + args->macro = NULL; + } + ++ if (ast->flavor == CIL_OPTIONAL) { ++ struct cil_tree_node *n = ast->parent; ++ args->optional = NULL; ++ /* Optionals can be nested */ ++ while (n && n->flavor != CIL_ROOT) { ++ if (n->flavor == CIL_OPTIONAL) { ++ args->optional = n; ++ break; ++ } ++ n = n->parent; ++ } ++ } ++ + if (ast->flavor == CIL_BOOLEANIF) { + args->boolif = NULL; + } +@@ -6526,6 +6557,7 @@ int cil_build_ast(struct cil_db *db, struct cil_tree_node *parse_tree, struct ci + extra_args.tunif = NULL; + extra_args.in = NULL; + extra_args.macro = NULL; ++ extra_args.optional = NULL; + extra_args.boolif = NULL; + + rc = cil_tree_walk(parse_tree, __cil_build_ast_node_helper, __cil_build_ast_first_child_helper, __cil_build_ast_last_child_helper, &extra_args); +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index 06b6ab48..8ffab438 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -3723,8 +3723,10 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished + + if (optional != NULL) { + if (node->flavor == CIL_TUNABLE || ++ node->flavor == CIL_IN || ++ node->flavor == CIL_BLOCK || ++ node->flavor == CIL_BLOCKABSTRACT || + node->flavor == CIL_MACRO) { +- /* tuanbles and macros are not allowed in optionals*/ + cil_tree_log(node, CIL_ERR, "%s statement is not allowed in optionals", cil_node_to_string(node)); + rc = SEPOL_ERR; + goto exit; +-- +2.30.2 + diff --git a/SPECS/libsepol.spec b/SPECS/libsepol.spec index b016214..eac7fe1 100644 --- a/SPECS/libsepol.spec +++ b/SPECS/libsepol.spec @@ -1,9 +1,29 @@ Summary: SELinux binary policy manipulation library Name: libsepol Version: 2.9 -Release: 2%{?dist} +Release: 3%{?dist} License: LGPLv2+ Source0: https://github.com/SELinuxProject/selinux/releases/download/20190315/libsepol-2.9.tar.gz +Patch0001: 0001-libsepol-cil-Fix-out-of-bound-read-of-file-context-p.patch +Patch0002: 0002-libsepol-cil-Destroy-classperms-list-when-resetting-.patch +Patch0003: 0003-libsepol-cil-Destroy-classperm-list-when-resetting-m.patch +Patch0004: 0004-libsepol-cil-cil_reset_classperms_set-should-not-res.patch +Patch0005: 0005-libsepol-cil-Set-class-field-to-NULL-when-resetting-.patch +Patch0006: 0006-libsepol-cil-More-strict-verification-of-constraint-.patch +Patch0007: 0007-libsepol-cil-Exit-with-an-error-if-declaration-name-.patch +Patch0008: 0008-libsepol-cil-Allow-permission-expressions-when-using.patch +Patch0009: 0009-libsepol-cil-Reorder-checks-for-invalid-rules-when-b.patch +Patch0010: 0010-libsepol-cil-Cleanup-build-AST-helper-functions.patch +Patch0011: 0011-libsepol-cil-Create-new-first-child-helper-function-.patch +Patch0012: 0012-libsepol-cil-Remove-unused-field-from-struct-cil_arg.patch +Patch0013: 0013-libsepol-cil-Destroy-disabled-optional-blocks-after-.patch +Patch0014: 0014-libsepol-cil-Check-if-name-is-a-macro-parameter-firs.patch +Patch0015: 0015-libsepol-cil-fix-NULL-pointer-dereference-in-__cil_i.patch +Patch0016: 0016-libsepol-cil-Report-disabling-an-optional-block-only.patch +Patch0017: 0017-libsepol-cil-Use-AST-to-track-blocks-and-optionals-w.patch +Patch0018: 0018-libsepol-cil-Reorder-checks-for-invalid-rules-when-r.patch +Patch0019: 0019-libsepol-cil-Sync-checks-for-invalid-rules-in-boolea.patch +Patch0020: 0020-libsepol-cil-Check-for-statements-not-allowed-in-opt.patch URL: https://github.com/SELinuxProject/selinux/wiki BuildRequires: gcc BuildRequires: flex @@ -41,7 +61,7 @@ The libsepol-static package contains the static libraries and header files needed for developing applications that manipulate binary policies. %prep -%autosetup -p 1 -n libsepol-%{version} +%autosetup -p 2 -n libsepol-%{version} # sparc64 is an -fPIC arch, so we need to fix it here %ifarch sparc64 @@ -92,6 +112,28 @@ exit 0 %{_libdir}/libsepol.so.1 %changelog +* Wed Aug 18 2021 Vit Mojzis - 2.9-3 +- cil: Fix out-of-bound read of file context pattern ending with "\" +- cil: Destroy classperms list when resetting classpermission (#1983517) +- cil: Destroy classperm list when resetting map perms (#1983521) +- cil: cil_reset_classperms_set() should not reset classpermission (#1983525) +- cil: Set class field to NULL when resetting struct cil_classperms +- cil: More strict verification of constraint leaf expressions +- cil: Exit with an error if declaration name is a reserved word +- cil: Allow permission expressions when using map classes +- cil: Reorder checks for invalid rules when building AST +- cil: Cleanup build AST helper functions +- cil: Create new first child helper function for building AST +- cil: Remove unused field from struct cil_args_resolve +- cil: Destroy disabled optional blocks after pass is complete +- cil: Check if name is a macro parameter first +- cil: fix NULL pointer dereference in __cil_insert_name +- cil: Report disabling an optional block only at high verbose levels +- cil: Use AST to track blocks and optionals when resolving +- cil: Reorder checks for invalid rules when resolving AST +- cil: Sync checks for invalid rules in booleanifs +- cil: Check for statements not allowed in optional blocks (#1983530) + * Wed Jan 06 2021 Vit Mojzis - 2.9-2 - Drop unnecessary telinit (#1838257)