| Message ID | 20210929202454.136401-1-jwcart2@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | libsepol/cil: Do not skip macros when resolving until later passes | expand |
On Wed, Sep 29, 2021 at 10:25 PM James Carter <jwcart2@gmail.com> wrote: > > Since only tunableifs need to be resolved in a macro before the macro > is copied for each call, macros were being skipped after resolving > tunableifs. Statments not allowed to be in macros would be found during > the pass that resolved tunableifs. Unfortunately, in-statments are > resolved after tunableifs and they can be used to add statements to > macros that are not allowed. > > Instead, do not skip macros until after the pass that resolves in- > statements that are to be resolved after block inheritance. This > allows blocks, blockinherits, blockabstracts, and macros that were > added by an in-statement to be found and an error reported. > > This bug was found by the secilc-fuzzer. > > Signed-off-by: James Carter <jwcart2@gmail.com> I confirm this fixes the issue reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=36466&q=selinux&can=2 which can be reproduced by the following policy: (macro MACRO ()) (in MACRO (blockinherit BLOCK)) (call MACRO) (in MACRO (blockinherit BLOCK)) (call MACRO) (in MACRO (blockinherit BLOCK)) (call MACRO) (in MACRO (blockinherit BLOCK)) (call MACRO) (in MACRO (blockinherit BLOCK)) (call MACRO) (in MACRO (blockinherit BLOCK)) (call MACRO) ... repeated at least 4000 times Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org> Thanks, Nicolas > --- > libsepol/cil/src/cil_resolve_ast.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c > index 2cf94368..e97a9f46 100644 > --- a/libsepol/cil/src/cil_resolve_ast.c > +++ b/libsepol/cil/src/cil_resolve_ast.c > @@ -3946,7 +3946,7 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished > } > > if (node->flavor == CIL_MACRO) { > - if (pass != CIL_PASS_TIF) { > + if (pass > CIL_PASS_IN_AFTER) { > *finished = CIL_TREE_SKIP_HEAD; > rc = SEPOL_OK; > goto exit; > -- > 2.31.1 >
Nicolas Iooss <nicolas.iooss@m4x.org> writes: > On Wed, Sep 29, 2021 at 10:25 PM James Carter <jwcart2@gmail.com> wrote: >> >> Since only tunableifs need to be resolved in a macro before the macro >> is copied for each call, macros were being skipped after resolving >> tunableifs. Statments not allowed to be in macros would be found during >> the pass that resolved tunableifs. Unfortunately, in-statments are >> resolved after tunableifs and they can be used to add statements to >> macros that are not allowed. >> >> Instead, do not skip macros until after the pass that resolves in- >> statements that are to be resolved after block inheritance. This >> allows blocks, blockinherits, blockabstracts, and macros that were >> added by an in-statement to be found and an error reported. >> >> This bug was found by the secilc-fuzzer. >> >> Signed-off-by: James Carter <jwcart2@gmail.com> > > I confirm this fixes the issue reported in > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=36466&q=selinux&can=2 > which can be reproduced by the following policy: > > (macro MACRO ()) > (in MACRO (blockinherit BLOCK)) (call MACRO) > (in MACRO (blockinherit BLOCK)) (call MACRO) > (in MACRO (blockinherit BLOCK)) (call MACRO) > (in MACRO (blockinherit BLOCK)) (call MACRO) > (in MACRO (blockinherit BLOCK)) (call MACRO) > (in MACRO (blockinherit BLOCK)) (call MACRO) > ... > repeated at least 4000 times > > Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org> > Merged. Thanks! >> --- >> libsepol/cil/src/cil_resolve_ast.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c >> index 2cf94368..e97a9f46 100644 >> --- a/libsepol/cil/src/cil_resolve_ast.c >> +++ b/libsepol/cil/src/cil_resolve_ast.c >> @@ -3946,7 +3946,7 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished >> } >> >> if (node->flavor == CIL_MACRO) { >> - if (pass != CIL_PASS_TIF) { >> + if (pass > CIL_PASS_IN_AFTER) { >> *finished = CIL_TREE_SKIP_HEAD; >> rc = SEPOL_OK; >> goto exit; >> -- >> 2.31.1 >>
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c index 2cf94368..e97a9f46 100644 --- a/libsepol/cil/src/cil_resolve_ast.c +++ b/libsepol/cil/src/cil_resolve_ast.c @@ -3946,7 +3946,7 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished } if (node->flavor == CIL_MACRO) { - if (pass != CIL_PASS_TIF) { + if (pass > CIL_PASS_IN_AFTER) { *finished = CIL_TREE_SKIP_HEAD; rc = SEPOL_OK; goto exit;
Since only tunableifs need to be resolved in a macro before the macro is copied for each call, macros were being skipped after resolving tunableifs. Statments not allowed to be in macros would be found during the pass that resolved tunableifs. Unfortunately, in-statments are resolved after tunableifs and they can be used to add statements to macros that are not allowed. Instead, do not skip macros until after the pass that resolves in- statements that are to be resolved after block inheritance. This allows blocks, blockinherits, blockabstracts, and macros that were added by an in-statement to be found and an error reported. This bug was found by the secilc-fuzzer. Signed-off-by: James Carter <jwcart2@gmail.com> --- libsepol/cil/src/cil_resolve_ast.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)