On Fri, Feb 5, 2021 at 5:02 PM James Carter <jwcart2@xxxxxxxxx> wrote: > > 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 > Eventually, I would like to reject these in the parser, but this patch is fine. Acked-by: James Carter <jwcart2@xxxxxxxxx> Applied. Thanks, 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 > >
