[PATCH] libsepol/cil: do not override previous results of __cil_verify_classperms

[PATCH] libsepol/cil: do not override previous results of __cil_verify_classperms

[Nicolas Iooss]

When __cil_verify_map_class() verifies a classpermission, it calls
__verify_map_perm_classperms() on each item. If the first item reports a
failure and the next one succeeds, the failure is overwritten in
map_args->rc. This is a bug which causes a NULL pointer dereference in
the CIL compiler when compiling the following policy:

    (sid SID)
    (sidorder (SID))

    (class CLASS (PERM1))
    (classorder (CLASS))

    (classpermission CLSPERM)
    (classpermissionset CLSPERM (CLASS (PERM1)))
    (classmap files (CLAMAPxx x))
    (classmapping files CLAMAPxx CLSPERM)

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=30286

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/cil/src/cil_verify.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
index 59397f70f2ea..8ad3dc9e114a 100644
--- a/libsepol/cil/src/cil_verify.c
+++ b/libsepol/cil/src/cil_verify.c
@@ -1786,8 +1786,12 @@ static int __verify_map_perm_classperms(__attribute__((unused)) hashtab_key_t k,
 {
 	struct cil_verify_map_args *map_args = args;
 	struct cil_perm *cmp = (struct cil_perm *)d;
+	int rc;
 
-	map_args->rc = __cil_verify_classperms(cmp->classperms, &cmp->datum, &map_args->class->datum, &cmp->datum, CIL_MAP_PERM, 0, 2);
+	rc = __cil_verify_classperms(cmp->classperms, &cmp->datum, &map_args->class->datum, &cmp->datum, CIL_MAP_PERM, 0, 2);
+	if (rc != SEPOL_OK) {
+		map_args->rc = rc;
+	}
 
 	return SEPOL_OK;
 }
-- 
2.32.0

Re: [PATCH] libsepol/cil: do not override previous results of __cil_verify_classperms

[James Carter]

On Fri, Jul 2, 2021 at 7:15 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> When __cil_verify_map_class() verifies a classpermission, it calls
> __verify_map_perm_classperms() on each item. If the first item reports a
> failure and the next one succeeds, the failure is overwritten in
> map_args->rc. This is a bug which causes a NULL pointer dereference in
> the CIL compiler when compiling the following policy:
>
>     (sid SID)
>     (sidorder (SID))
>
>     (class CLASS (PERM1))
>     (classorder (CLASS))
>
>     (classpermission CLSPERM)
>     (classpermissionset CLSPERM (CLASS (PERM1)))
>     (classmap files (CLAMAPxx x))
>     (classmapping files CLAMAPxx CLSPERM)
>
> Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=30286
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  libsepol/cil/src/cil_verify.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
> index 59397f70f2ea..8ad3dc9e114a 100644
> --- a/libsepol/cil/src/cil_verify.c
> +++ b/libsepol/cil/src/cil_verify.c
> @@ -1786,8 +1786,12 @@ static int __verify_map_perm_classperms(__attribute__((unused)) hashtab_key_t k,
>  {
>         struct cil_verify_map_args *map_args = args;
>         struct cil_perm *cmp = (struct cil_perm *)d;
> +       int rc;
>
> -       map_args->rc = __cil_verify_classperms(cmp->classperms, &cmp->datum, &map_args->class->datum, &cmp->datum, CIL_MAP_PERM, 0, 2);
> +       rc = __cil_verify_classperms(cmp->classperms, &cmp->datum, &map_args->class->datum, &cmp->datum, CIL_MAP_PERM, 0, 2);
> +       if (rc != SEPOL_OK) {
> +               map_args->rc = rc;
> +       }
>
>         return SEPOL_OK;
>  }
> --
> 2.32.0
>

Re: [PATCH] libsepol/cil: do not override previous results of __cil_verify_classperms

[James Carter]

On Tue, Jul 6, 2021 at 10:58 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Fri, Jul 2, 2021 at 7:15 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> >
> > When __cil_verify_map_class() verifies a classpermission, it calls
> > __verify_map_perm_classperms() on each item. If the first item reports a
> > failure and the next one succeeds, the failure is overwritten in
> > map_args->rc. This is a bug which causes a NULL pointer dereference in
> > the CIL compiler when compiling the following policy:
> >
> >     (sid SID)
> >     (sidorder (SID))
> >
> >     (class CLASS (PERM1))
> >     (classorder (CLASS))
> >
> >     (classpermission CLSPERM)
> >     (classpermissionset CLSPERM (CLASS (PERM1)))
> >     (classmap files (CLAMAPxx x))
> >     (classmapping files CLAMAPxx CLSPERM)
> >
> > Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=30286
> >
> > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>

Merged.
Thanks,
Jim

> > ---
> >  libsepol/cil/src/cil_verify.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
> > index 59397f70f2ea..8ad3dc9e114a 100644
> > --- a/libsepol/cil/src/cil_verify.c
> > +++ b/libsepol/cil/src/cil_verify.c
> > @@ -1786,8 +1786,12 @@ static int __verify_map_perm_classperms(__attribute__((unused)) hashtab_key_t k,
> >  {
> >         struct cil_verify_map_args *map_args = args;
> >         struct cil_perm *cmp = (struct cil_perm *)d;
> > +       int rc;
> >
> > -       map_args->rc = __cil_verify_classperms(cmp->classperms, &cmp->datum, &map_args->class->datum, &cmp->datum, CIL_MAP_PERM, 0, 2);
> > +       rc = __cil_verify_classperms(cmp->classperms, &cmp->datum, &map_args->class->datum, &cmp->datum, CIL_MAP_PERM, 0, 2);
> > +       if (rc != SEPOL_OK) {
> > +               map_args->rc = rc;
> > +       }
> >
> >         return SEPOL_OK;
> >  }
> > --
> > 2.32.0
> >