[PATCH] libsepol/cil: Do not skip macros when resolving until later passes

[PATCH] libsepol/cil: Do not skip macros when resolving until later passes

[James Carter]

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(-)

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

Re: [PATCH] libsepol/cil: Do not skip macros when resolving until later passes

[Nicolas Iooss]

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
>

Re: [PATCH] libsepol/cil: Do not skip macros when resolving until later passes

[Petr Lautrbach]

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
>>