Re: [PATCH] libsepol: Use calloc when initializing bool_val_to_struct array

[Christian Göttsche <cgzones@xxxxxxxxxxxxxx>]

On Mon, 14 Mar 2022 at 19:24, James Carter <jwcart2@xxxxxxxxx> wrote:
>
> Use calloc() instead of mallocarray() so that everything is
> initialized to zero to prevent the use of unitialized memory when
> validating malformed binary policies.
>
> Found by oss-fuzz (#45493)
>
> Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
> ---
>  libsepol/src/conditional.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
> index f78b38a2..a620451d 100644
> --- a/libsepol/src/conditional.c
> +++ b/libsepol/src/conditional.c
> @@ -522,7 +522,7 @@ int cond_init_bool_indexes(policydb_t * p)
>         if (p->bool_val_to_struct)
>                 free(p->bool_val_to_struct);
>         p->bool_val_to_struct = (cond_bool_datum_t **)
> -           mallocarray(p->p_bools.nprim, sizeof(cond_bool_datum_t *));
> +           calloc(p->p_bools.nprim, sizeof(cond_bool_datum_t *));
>         if (!p->bool_val_to_struct)
>                 return -1;
>         return 0;
> --
> 2.34.1
>

Can this be merged? I think it might hurt the fuzzer, e.g. cause the
flakiness in issue #45327.

On a technical note:
In src/policydb.c::policydb_index_others() the return value of
cond_init_bool_indexes() is not checked.

diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index fc71463e..e29cbd51 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -1252,7 +1252,8 @@ int policydb_index_others(sepol_handle_t * handle,
       if (!p->type_val_to_struct)
               return -1;

-       cond_init_bool_indexes(p);
+       if (cond_init_bool_indexes(p) == -1)
+               return -1;

       for (i = SYM_ROLES; i < SYM_NUM; i++) {
               free(p->sym_val_to_name[i]);