* [PATCH 1/1] libsepol/cil: unlink blockinherit->block link when destroying a block
@ 2021-02-01 22:17 Nicolas Iooss
2021-02-02 20:36 ` James Carter
0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Iooss @ 2021-02-01 22:17 UTC (permalink / raw)
To: selinux
The following CIL policy triggers a heap use-after-free in secilc
because when the blockinherit node is destroyed, the block node was
already destroyed:
(block b2a)
(blockinherit b2a)
Fix this by setting blockinherit->block to NULL when destroying block.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
libsepol/cil/src/cil_build_ast.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 02481558ad11..c6edcde6bc5d 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -231,13 +231,30 @@ exit:
void cil_destroy_block(struct cil_block *block)
{
+ struct cil_list_item *item;
+ struct cil_tree_node *bi_node;
+ struct cil_blockinherit *inherit;
+
if (block == NULL) {
return;
}
cil_symtab_datum_destroy(&block->datum);
cil_symtab_array_destroy(block->symtab);
- cil_list_destroy(&block->bi_nodes, CIL_FALSE);
+ if (block->bi_nodes != NULL) {
+ /* unlink blockinherit->block */
+ cil_list_for_each(item, block->bi_nodes) {
+ bi_node = item->data;
+ /* the conditions should always be true, but better be sure */
+ if (bi_node->flavor == CIL_BLOCKINHERIT) {
+ inherit = bi_node->data;
+ if (inherit->block == block) {
+ inherit->block = NULL;
+ }
+ }
+ }
+ cil_list_destroy(&block->bi_nodes, CIL_FALSE);
+ }
free(block);
}
--
2.30.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] libsepol/cil: unlink blockinherit->block link when destroying a block
2021-02-01 22:17 [PATCH 1/1] libsepol/cil: unlink blockinherit->block link when destroying a block Nicolas Iooss
@ 2021-02-02 20:36 ` James Carter
2021-02-03 8:35 ` Petr Lautrbach
0 siblings, 1 reply; 3+ messages in thread
From: James Carter @ 2021-02-02 20:36 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: SElinux list
On Mon, Feb 1, 2021 at 5:20 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> The following CIL policy triggers a heap use-after-free in secilc
> because when the blockinherit node is destroyed, the block node was
> already destroyed:
>
> (block b2a)
> (blockinherit b2a)
>
> Fix this by setting blockinherit->block to NULL when destroying block.
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/cil/src/cil_build_ast.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index 02481558ad11..c6edcde6bc5d 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -231,13 +231,30 @@ exit:
>
> void cil_destroy_block(struct cil_block *block)
> {
> + struct cil_list_item *item;
> + struct cil_tree_node *bi_node;
> + struct cil_blockinherit *inherit;
> +
> if (block == NULL) {
> return;
> }
>
> cil_symtab_datum_destroy(&block->datum);
> cil_symtab_array_destroy(block->symtab);
> - cil_list_destroy(&block->bi_nodes, CIL_FALSE);
> + if (block->bi_nodes != NULL) {
> + /* unlink blockinherit->block */
> + cil_list_for_each(item, block->bi_nodes) {
> + bi_node = item->data;
> + /* the conditions should always be true, but better be sure */
> + if (bi_node->flavor == CIL_BLOCKINHERIT) {
> + inherit = bi_node->data;
> + if (inherit->block == block) {
> + inherit->block = NULL;
> + }
> + }
> + }
> + cil_list_destroy(&block->bi_nodes, CIL_FALSE);
> + }
>
> free(block);
> }
> --
> 2.30.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] libsepol/cil: unlink blockinherit->block link when destroying a block
2021-02-02 20:36 ` James Carter
@ 2021-02-03 8:35 ` Petr Lautrbach
0 siblings, 0 replies; 3+ messages in thread
From: Petr Lautrbach @ 2021-02-03 8:35 UTC (permalink / raw)
To: SElinux list; +Cc: James Carter, Nicolas Iooss
James Carter <jwcart2@gmail.com> writes:
> On Mon, Feb 1, 2021 at 5:20 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>>
>> The following CIL policy triggers a heap use-after-free in secilc
>> because when the blockinherit node is destroyed, the block node was
>> already destroyed:
>>
>> (block b2a)
>> (blockinherit b2a)
>>
>> Fix this by setting blockinherit->block to NULL when destroying block.
>>
>> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Acked-by: James Carter <jwcart2@gmail.com>
Merged. Thanks!
>> ---
>> libsepol/cil/src/cil_build_ast.c | 19 ++++++++++++++++++-
>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
>> index 02481558ad11..c6edcde6bc5d 100644
>> --- a/libsepol/cil/src/cil_build_ast.c
>> +++ b/libsepol/cil/src/cil_build_ast.c
>> @@ -231,13 +231,30 @@ exit:
>>
>> void cil_destroy_block(struct cil_block *block)
>> {
>> + struct cil_list_item *item;
>> + struct cil_tree_node *bi_node;
>> + struct cil_blockinherit *inherit;
>> +
>> if (block == NULL) {
>> return;
>> }
>>
>> cil_symtab_datum_destroy(&block->datum);
>> cil_symtab_array_destroy(block->symtab);
>> - cil_list_destroy(&block->bi_nodes, CIL_FALSE);
>> + if (block->bi_nodes != NULL) {
>> + /* unlink blockinherit->block */
>> + cil_list_for_each(item, block->bi_nodes) {
>> + bi_node = item->data;
>> + /* the conditions should always be true, but better be sure */
>> + if (bi_node->flavor == CIL_BLOCKINHERIT) {
>> + inherit = bi_node->data;
>> + if (inherit->block == block) {
>> + inherit->block = NULL;
>> + }
>> + }
>> + }
>> + cil_list_destroy(&block->bi_nodes, CIL_FALSE);
>> + }
>>
>> free(block);
>> }
>> --
>> 2.30.0
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-03 8:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-01 22:17 [PATCH 1/1] libsepol/cil: unlink blockinherit->block link when destroying a block Nicolas Iooss
2021-02-02 20:36 ` James Carter
2021-02-03 8:35 ` Petr Lautrbach
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox