| Message ID | 20210702110705.435223-1-nicolas.iooss@m4x.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | libsepol/cil: do not override previous results of __cil_verify_classperms | expand |
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 >
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 > >
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; }
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(-)