116 lines
4.5 KiB
Diff
116 lines
4.5 KiB
Diff
From d668f8e3a0a0361c03881ae3f00509196eaee064 Mon Sep 17 00:00:00 2001
|
|
From: James Carter <jwcart2@gmail.com>
|
|
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 <nicolas.iooss@m4x.org>
|
|
Signed-off-by: James Carter <jwcart2@gmail.com>
|
|
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
|
|
---
|
|
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
|
|
|