| Message ID | 20201230201141.3455302-4-nicolas.iooss@m4x.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/4] libsepol/cil: remove useless print statement | expand |
On Wed, Dec 30, 2020 at 2:13 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying > to compile the following policy: > > (optional o (validatetrans x (eq t3 (a ())))) > > With some logs, secilc reports: > > Invalid syntax > Destroying Parse Tree > Resolving AST > Failed to resolve validatetrans statement at fuzz:1 > Disabling optional 'o' at tmp.cil:1 > > So there is an "Invalid syntax" error, but the compilation continues. > Fix this issue by stopping the compilation when cil_fill_list() reports > an error: > > Invalid syntax > Bad expression tree for constraint > Bad validatetrans declaration at tmp.cil:1 > > Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29061 > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> > --- > libsepol/cil/src/cil_build_ast.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c > index 4caff3cb3c98..0ea90cf92186 100644 > --- a/libsepol/cil/src/cil_build_ast.c > +++ b/libsepol/cil/src/cil_build_ast.c > @@ -2713,7 +2713,11 @@ static int __cil_fill_constraint_leaf_expr(struct cil_tree_node *current, enum c > cil_list_append(*leaf_expr, CIL_STRING, current->next->next->data); > } else if (r_flavor == CIL_LIST) { > struct cil_list *sub_list; > - cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); > + rc = cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); > + if (rc != SEPOL_OK) { > + cil_list_destroy(leaf_expr, CIL_TRUE); > + goto exit; > + } > cil_list_append(*leaf_expr, CIL_LIST, sub_list); > } else { > cil_list_append(*leaf_expr, CIL_CONS_OPERAND, (void *)r_flavor); > -- > 2.29.2 > ack for the series.
On Thu, Dec 31, 2020 at 10:29 AM William Roberts <bill.c.roberts@gmail.com> wrote: > > On Wed, Dec 30, 2020 at 2:13 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying > > to compile the following policy: > > > > (optional o (validatetrans x (eq t3 (a ())))) > > > > With some logs, secilc reports: > > > > Invalid syntax > > Destroying Parse Tree > > Resolving AST > > Failed to resolve validatetrans statement at fuzz:1 > > Disabling optional 'o' at tmp.cil:1 > > > > So there is an "Invalid syntax" error, but the compilation continues. > > Fix this issue by stopping the compilation when cil_fill_list() reports > > an error: > > > > Invalid syntax > > Bad expression tree for constraint > > Bad validatetrans declaration at tmp.cil:1 > > > > Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29061 > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> > > --- > > libsepol/cil/src/cil_build_ast.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c > > index 4caff3cb3c98..0ea90cf92186 100644 > > --- a/libsepol/cil/src/cil_build_ast.c > > +++ b/libsepol/cil/src/cil_build_ast.c > > @@ -2713,7 +2713,11 @@ static int __cil_fill_constraint_leaf_expr(struct cil_tree_node *current, enum c > > cil_list_append(*leaf_expr, CIL_STRING, current->next->next->data); > > } else if (r_flavor == CIL_LIST) { > > struct cil_list *sub_list; > > - cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); > > + rc = cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); > > + if (rc != SEPOL_OK) { > > + cil_list_destroy(leaf_expr, CIL_TRUE); > > + goto exit; > > + } > > cil_list_append(*leaf_expr, CIL_LIST, sub_list); > > } else { > > cil_list_append(*leaf_expr, CIL_CONS_OPERAND, (void *)r_flavor); > > -- > > 2.29.2 > > > > ack for the series. I've applied this series. Thanks, Jim
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c index 4caff3cb3c98..0ea90cf92186 100644 --- a/libsepol/cil/src/cil_build_ast.c +++ b/libsepol/cil/src/cil_build_ast.c @@ -2713,7 +2713,11 @@ static int __cil_fill_constraint_leaf_expr(struct cil_tree_node *current, enum c cil_list_append(*leaf_expr, CIL_STRING, current->next->next->data); } else if (r_flavor == CIL_LIST) { struct cil_list *sub_list; - cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); + rc = cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); + if (rc != SEPOL_OK) { + cil_list_destroy(leaf_expr, CIL_TRUE); + goto exit; + } cil_list_append(*leaf_expr, CIL_LIST, sub_list); } else { cil_list_append(*leaf_expr, CIL_CONS_OPERAND, (void *)r_flavor);
OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying to compile the following policy: (optional o (validatetrans x (eq t3 (a ())))) With some logs, secilc reports: Invalid syntax Destroying Parse Tree Resolving AST Failed to resolve validatetrans statement at fuzz:1 Disabling optional 'o' at tmp.cil:1 So there is an "Invalid syntax" error, but the compilation continues. Fix this issue by stopping the compilation when cil_fill_list() reports an error: Invalid syntax Bad expression tree for constraint Bad validatetrans declaration at tmp.cil:1 Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29061 Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> --- libsepol/cil/src/cil_build_ast.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)