On Mon, Feb 1, 2021 at 5:20 PM Nicolas Iooss <nicolas.iooss@xxxxxxx> 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@xxxxxxx>
Acked-by: James Carter <jwcart2@xxxxxxxxx>
> ---
> 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
>
