Re: [PATCH 1/1] libsepol/cil: unlink blockinherit->block link when destroying a block

[Petr Lautrbach <plautrba@redhat.com>]

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
>>