* [PATCH V2] libsepol/cil: Fix heap-use-after-free when using optional blockinherit
@ 2021-02-02 20:54 James Carter
2021-02-03 9:01 ` Nicolas Iooss
0 siblings, 1 reply; 3+ messages in thread
From: James Carter @ 2021-02-02 20:54 UTC (permalink / raw)
To: selinux; +Cc: James Carter, Nicolas Iooss
This is based on a patch by Nicolas Iooss. He writes:
When secilc compiles the following policy:
(block b1
(optional o1
(blockinherit b1)
(blockinherit x)
)
)
it disables the optional block at pass 3 (CIL_PASS_BLKIN_LINK)
because the block "x" does not exist.
__cil_resolve_ast_last_child_helper() calls
cil_tree_children_destroy() on the optional block, which destroys
the two blockinherit statements. But the (blockinherit b1) node
was referenced inside (block b1) node, in its block->bi_nodes list.
Therefore, when this list is used at pass 4 (CIL_PASS_BLKIN_COPY),
it contains a node which was freed: this triggers a use-after-free
issue
Fix this issue by removing blockinherit nodes from their lists of
nodes block->bi_nodes when they are being destroyed. As
cil_destroy_blockinherit() does not have a reference to the node
containing the blockinherit data, implement this new logic in
cil_tree_node_destroy().
This issue was found while investigating a testcase from an OSS-Fuzz
issue which seems unrelated (a Null-dereference READ in
cil_symtab_get_datum,
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29861).
Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_build_ast.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 02481558..ab0efd53 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -283,6 +283,19 @@ void cil_destroy_blockinherit(struct cil_blockinherit *inherit)
return;
}
+ if (inherit->block != NULL && inherit->block->bi_nodes != NULL) {
+ struct cil_tree_node *node;
+ struct cil_list_item *item;
+
+ cil_list_for_each(item, inherit->block->bi_nodes) {
+ node = item->data;
+ if (node->data == inherit) {
+ cil_list_remove(inherit->block->bi_nodes, CIL_NODE, node, CIL_FALSE);
+ break;
+ }
+ }
+ }
+
free(inherit);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V2] libsepol/cil: Fix heap-use-after-free when using optional blockinherit
2021-02-02 20:54 [PATCH V2] libsepol/cil: Fix heap-use-after-free when using optional blockinherit James Carter
@ 2021-02-03 9:01 ` Nicolas Iooss
2021-02-03 12:07 ` Petr Lautrbach
0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Iooss @ 2021-02-03 9:01 UTC (permalink / raw)
To: James Carter; +Cc: SElinux list
On Tue, Feb 2, 2021 at 9:54 PM James Carter <jwcart2@gmail.com> wrote:
>
> This is based on a patch by Nicolas Iooss. He writes:
> When secilc compiles the following policy:
>
> (block b1
> (optional o1
> (blockinherit b1)
> (blockinherit x)
> )
> )
>
> it disables the optional block at pass 3 (CIL_PASS_BLKIN_LINK)
> because the block "x" does not exist.
> __cil_resolve_ast_last_child_helper() calls
> cil_tree_children_destroy() on the optional block, which destroys
> the two blockinherit statements. But the (blockinherit b1) node
> was referenced inside (block b1) node, in its block->bi_nodes list.
> Therefore, when this list is used at pass 4 (CIL_PASS_BLKIN_COPY),
> it contains a node which was freed: this triggers a use-after-free
> issue
>
> Fix this issue by removing blockinherit nodes from their lists of
> nodes block->bi_nodes when they are being destroyed. As
> cil_destroy_blockinherit() does not have a reference to the node
> containing the blockinherit data, implement this new logic in
> cil_tree_node_destroy().
>
> This issue was found while investigating a testcase from an OSS-Fuzz
> issue which seems unrelated (a Null-dereference READ in
> cil_symtab_get_datum,
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29861).
>
> Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> Signed-off-by: James Carter <jwcart2@gmail.com>
I tested the patch and confirm it fixes the issue.
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Thanks!
Nicolas
> ---
> libsepol/cil/src/cil_build_ast.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index 02481558..ab0efd53 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -283,6 +283,19 @@ void cil_destroy_blockinherit(struct cil_blockinherit *inherit)
> return;
> }
>
> + if (inherit->block != NULL && inherit->block->bi_nodes != NULL) {
> + struct cil_tree_node *node;
> + struct cil_list_item *item;
> +
> + cil_list_for_each(item, inherit->block->bi_nodes) {
> + node = item->data;
> + if (node->data == inherit) {
> + cil_list_remove(inherit->block->bi_nodes, CIL_NODE, node, CIL_FALSE);
> + break;
> + }
> + }
> + }
> +
> free(inherit);
> }
>
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V2] libsepol/cil: Fix heap-use-after-free when using optional blockinherit
2021-02-03 9:01 ` Nicolas Iooss
@ 2021-02-03 12:07 ` Petr Lautrbach
0 siblings, 0 replies; 3+ messages in thread
From: Petr Lautrbach @ 2021-02-03 12:07 UTC (permalink / raw)
To: SElinux list; +Cc: Nicolas Iooss, James Carter
Nicolas Iooss <nicolas.iooss@m4x.org> writes:
> On Tue, Feb 2, 2021 at 9:54 PM James Carter <jwcart2@gmail.com> wrote:
>>
>> This is based on a patch by Nicolas Iooss. He writes:
>> When secilc compiles the following policy:
>>
>> (block b1
>> (optional o1
>> (blockinherit b1)
>> (blockinherit x)
>> )
>> )
>>
>> it disables the optional block at pass 3 (CIL_PASS_BLKIN_LINK)
>> because the block "x" does not exist.
>> __cil_resolve_ast_last_child_helper() calls
>> cil_tree_children_destroy() on the optional block, which destroys
>> the two blockinherit statements. But the (blockinherit b1) node
>> was referenced inside (block b1) node, in its block->bi_nodes list.
>> Therefore, when this list is used at pass 4 (CIL_PASS_BLKIN_COPY),
>> it contains a node which was freed: this triggers a use-after-free
>> issue
>>
>> Fix this issue by removing blockinherit nodes from their lists of
>> nodes block->bi_nodes when they are being destroyed. As
>> cil_destroy_blockinherit() does not have a reference to the node
>> containing the blockinherit data, implement this new logic in
>> cil_tree_node_destroy().
>>
>> This issue was found while investigating a testcase from an OSS-Fuzz
>> issue which seems unrelated (a Null-dereference READ in
>> cil_symtab_get_datum,
>> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29861).
>>
>> Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>> Signed-off-by: James Carter <jwcart2@gmail.com>
>
> I tested the patch and confirm it fixes the issue.
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
Merged and released in 3.2-rc2
Thanks!
>> ---
>> libsepol/cil/src/cil_build_ast.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
>> index 02481558..ab0efd53 100644
>> --- a/libsepol/cil/src/cil_build_ast.c
>> +++ b/libsepol/cil/src/cil_build_ast.c
>> @@ -283,6 +283,19 @@ void cil_destroy_blockinherit(struct cil_blockinherit *inherit)
>> return;
>> }
>>
>> + if (inherit->block != NULL && inherit->block->bi_nodes != NULL) {
>> + struct cil_tree_node *node;
>> + struct cil_list_item *item;
>> +
>> + cil_list_for_each(item, inherit->block->bi_nodes) {
>> + node = item->data;
>> + if (node->data == inherit) {
>> + cil_list_remove(inherit->block->bi_nodes, CIL_NODE, node, CIL_FALSE);
>> + break;
>> + }
>> + }
>> + }
>> +
>> free(inherit);
>> }
>>
>> --
>> 2.26.2
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-03 12:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 20:54 [PATCH V2] libsepol/cil: Fix heap-use-after-free when using optional blockinherit James Carter
2021-02-03 9:01 ` Nicolas Iooss
2021-02-03 12:07 ` Petr Lautrbach
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.