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

[James Carter <jwcart2@gmail.com>]

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
>