Re: [PATCH 2/3] libsepol/cil: be more robust when encountering <src_info>

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 5, 2021 at 4:54 AM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
>
> OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying
> to compile the following policy:
>
>     (<src_info>)
>
> In cil_gen_src_info(), parse_current->next is NULL even though the code
> expects that both parse_current->next and parse_current->next->next
> exists.
>
> Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28457
> Signed-off-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>

This is an interesting case. All of the <SOMETHING> are generated
internally. I never even thought about what would happen if they
actually appeared in the policy. I'll have to think about what the
best way to handle this is. Your patch works fine, but it might be
better to check for and reject these in the parser.

Jim

> ---
>  libsepol/cil/src/cil_build_ast.c | 5 +++++
>  libsepol/cil/src/cil_tree.c      | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index 5094d62e2238..726f46cd478d 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -6070,6 +6070,11 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
>         /* No need to check syntax, because this is auto generated */
>         struct cil_src_info *info = NULL;
>
> +       if (parse_current->next == NULL || parse_current->next->next == NULL) {
> +               cil_tree_log(parse_current, CIL_ERR, "Bad <src_info>");
> +               return SEPOL_ERR;
> +       }
> +
>         cil_src_info_init(&info);
>
>         info->is_cil = (parse_current->next->data == CIL_KEY_SRC_CIL) ? CIL_TRUE : CIL_FALSE;
> diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
> index 886412d1b974..3da972e9cf4e 100644
> --- a/libsepol/cil/src/cil_tree.c
> +++ b/libsepol/cil/src/cil_tree.c
> @@ -69,7 +69,7 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
>
>         while (node) {
>                 if (node->flavor == CIL_NODE && node->data == NULL) {
> -                       if (node->cl_head->data == CIL_KEY_SRC_INFO) {
> +                       if (node->cl_head->data == CIL_KEY_SRC_INFO && node->cl_head->next != NULL && node->cl_head->next->next != NULL) {
>                                 /* Parse Tree */
>                                 *path = node->cl_head->next->next->data;
>                                 *is_cil = (node->cl_head->next->data == CIL_KEY_SRC_CIL);
> --
> 2.30.0
>



[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux