| Message ID | 20201230100746.2549568-5-nicolas.iooss@m4x.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/6] libsepol: do not decode out-of-bound rolebounds | expand |
On Wed, Dec 30, 2020 at 5:09 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > OSS-Fuzz found a heap buffer overflow (out-of-bound reads) when the CIL > compiler tries to report a recursive blockinherit with an optional > block: > > $ echo '(block b (optional o (blockinherit b)))' > tmp.cil > $ secilc tmp.cil > Segmentation fault (core dumped) > > This is because cil_print_recursive_blockinherit() assumes that all > nodes are either CIL_BLOCK or CIL_BLOCKINHERIT. Add support for other > block kinds, using cil_node_to_string() to show them. > > Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28462 > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> Acked-by: James Carter <jwcart2@gmail.com> > --- > libsepol/cil/src/cil_resolve_ast.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c > index f6deb1002fbd..ecd05dfa5dab 100644 > --- a/libsepol/cil/src/cil_resolve_ast.c > +++ b/libsepol/cil/src/cil_resolve_ast.c > @@ -2343,11 +2343,13 @@ void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_ > for (curr = bi_node; curr != terminating_node; curr = curr->parent) { > if (curr->flavor == CIL_BLOCK) { > cil_list_prepend(trace, CIL_NODE, curr); > - } else { > + } else if (curr->flavor == CIL_BLOCKINHERIT) { > if (curr != bi_node) { > cil_list_prepend(trace, CIL_NODE, NODE(((struct cil_blockinherit *)curr->data)->block)); > } > cil_list_prepend(trace, CIL_NODE, curr); > + } else { > + cil_list_prepend(trace, CIL_NODE, curr); > } > } > cil_list_prepend(trace, CIL_NODE, terminating_node); > @@ -2356,8 +2358,12 @@ void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_ > curr = item->data; > if (curr->flavor == CIL_BLOCK) { > cil_tree_log(curr, CIL_ERR, "block %s", DATUM(curr->data)->name); > - } else { > + } else if (curr->flavor == CIL_BLOCKINHERIT) { > cil_tree_log(curr, CIL_ERR, "blockinherit %s", ((struct cil_blockinherit *)curr->data)->block_str); > + } else if (curr->flavor == CIL_OPTIONAL) { > + cil_tree_log(curr, CIL_ERR, "optional %s", DATUM(curr->data)->name); > + } else { > + cil_tree_log(curr, CIL_ERR, "%s", cil_node_to_string(curr)); > } > } > > -- > 2.29.2 >
Applied. Thanks, Jim On Mon, Jan 4, 2021 at 1:17 PM James Carter <jwcart2@gmail.com> wrote: > > On Wed, Dec 30, 2020 at 5:09 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > OSS-Fuzz found a heap buffer overflow (out-of-bound reads) when the CIL > > compiler tries to report a recursive blockinherit with an optional > > block: > > > > $ echo '(block b (optional o (blockinherit b)))' > tmp.cil > > $ secilc tmp.cil > > Segmentation fault (core dumped) > > > > This is because cil_print_recursive_blockinherit() assumes that all > > nodes are either CIL_BLOCK or CIL_BLOCKINHERIT. Add support for other > > block kinds, using cil_node_to_string() to show them. > > > > Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28462 > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> > > Acked-by: James Carter <jwcart2@gmail.com> > > > --- > > libsepol/cil/src/cil_resolve_ast.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c > > index f6deb1002fbd..ecd05dfa5dab 100644 > > --- a/libsepol/cil/src/cil_resolve_ast.c > > +++ b/libsepol/cil/src/cil_resolve_ast.c > > @@ -2343,11 +2343,13 @@ void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_ > > for (curr = bi_node; curr != terminating_node; curr = curr->parent) { > > if (curr->flavor == CIL_BLOCK) { > > cil_list_prepend(trace, CIL_NODE, curr); > > - } else { > > + } else if (curr->flavor == CIL_BLOCKINHERIT) { > > if (curr != bi_node) { > > cil_list_prepend(trace, CIL_NODE, NODE(((struct cil_blockinherit *)curr->data)->block)); > > } > > cil_list_prepend(trace, CIL_NODE, curr); > > + } else { > > + cil_list_prepend(trace, CIL_NODE, curr); > > } > > } > > cil_list_prepend(trace, CIL_NODE, terminating_node); > > @@ -2356,8 +2358,12 @@ void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_ > > curr = item->data; > > if (curr->flavor == CIL_BLOCK) { > > cil_tree_log(curr, CIL_ERR, "block %s", DATUM(curr->data)->name); > > - } else { > > + } else if (curr->flavor == CIL_BLOCKINHERIT) { > > cil_tree_log(curr, CIL_ERR, "blockinherit %s", ((struct cil_blockinherit *)curr->data)->block_str); > > + } else if (curr->flavor == CIL_OPTIONAL) { > > + cil_tree_log(curr, CIL_ERR, "optional %s", DATUM(curr->data)->name); > > + } else { > > + cil_tree_log(curr, CIL_ERR, "%s", cil_node_to_string(curr)); > > } > > } > > > > -- > > 2.29.2 > >
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c index f6deb1002fbd..ecd05dfa5dab 100644 --- a/libsepol/cil/src/cil_resolve_ast.c +++ b/libsepol/cil/src/cil_resolve_ast.c @@ -2343,11 +2343,13 @@ void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_ for (curr = bi_node; curr != terminating_node; curr = curr->parent) { if (curr->flavor == CIL_BLOCK) { cil_list_prepend(trace, CIL_NODE, curr); - } else { + } else if (curr->flavor == CIL_BLOCKINHERIT) { if (curr != bi_node) { cil_list_prepend(trace, CIL_NODE, NODE(((struct cil_blockinherit *)curr->data)->block)); } cil_list_prepend(trace, CIL_NODE, curr); + } else { + cil_list_prepend(trace, CIL_NODE, curr); } } cil_list_prepend(trace, CIL_NODE, terminating_node); @@ -2356,8 +2358,12 @@ void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_ curr = item->data; if (curr->flavor == CIL_BLOCK) { cil_tree_log(curr, CIL_ERR, "block %s", DATUM(curr->data)->name); - } else { + } else if (curr->flavor == CIL_BLOCKINHERIT) { cil_tree_log(curr, CIL_ERR, "blockinherit %s", ((struct cil_blockinherit *)curr->data)->block_str); + } else if (curr->flavor == CIL_OPTIONAL) { + cil_tree_log(curr, CIL_ERR, "optional %s", DATUM(curr->data)->name); + } else { + cil_tree_log(curr, CIL_ERR, "%s", cil_node_to_string(curr)); } }
OSS-Fuzz found a heap buffer overflow (out-of-bound reads) when the CIL compiler tries to report a recursive blockinherit with an optional block: $ echo '(block b (optional o (blockinherit b)))' > tmp.cil $ secilc tmp.cil Segmentation fault (core dumped) This is because cil_print_recursive_blockinherit() assumes that all nodes are either CIL_BLOCK or CIL_BLOCKINHERIT. Add support for other block kinds, using cil_node_to_string() to show them. Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28462 Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> --- libsepol/cil/src/cil_resolve_ast.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)