On Fri, Nov 17, 2023 at 10:31 AM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Fri, Oct 13, 2023 at 9:52 AM James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > Macros can use classpermission arguments. These are used in two
> > different ways. Either a named classpermission is passed (which is
> > declared using a classpermisison rule) or an anonymous classpermission
> > is passed (something like "(CLASS (PERM))").
> >
> > Usually this will look like either of the following:
> > Ex1/
> > (classpermission cp1)
> > (classpermisisonset cp1 (CLASS (PERM)))
> > (macro m1 ((classpermisison ARG1))
> > (allow t1 self ARG1)
> > )
> > (call m1 (cp1))
> > or
> > Ex2/
> > (macro m2 ((classpermission ARG2))
> > (allow t2 self ARG2)
> > )
> > (call m2 ((CLASS (PERM))))
> >
> > The following would also be valid:
> > Ex3/
> > (classpermission cp3)
> > (macro m3 ((classpermission ARG3))
> > (classpermissionset ARG3 (CLASS (PERM)))
> > (allow t3 self ARG3)
> > )
> > (call m3 (cp3))
> >
> > The oss-fuzzer did the equivalent of the following:
> >
> > (classpermission cp4)
> > (macro m4 ((classpermission ARG4))
> > (classpermissionset ARG4 (CLASS (PERM1)))
> > (allow t4 self ARG4)
> > )
> > (call m4 (CLASS (PERM2)))
> >
> > It passed an anonymous classpermission into a macro where there
> > was a classpermissionset rule. Suprisingly, everything worked well
> > until it was time to destroy the AST. There is no way to distinguish
> > between the anonymous classpermission being passed in which needs
> > to be destroyed and the classpermission in the classpermissionset
> > rule which is destroyed when the classpermissionset rule is
> > destroyed. This led to CIL trying to destroy the classpermission
> > in the classpermissionset rule twice.
> >
> > To fix this, when resolving the classpermission name in the
> > classpermissionset rule, check if the datum returned is for
> > an anonymous classpermission (it has no name) and return an
> > error if it is.
> >
> > This fixes oss-fuzz issue 60670.
> >
> > Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
>
> This patch could use a review. I would like to get it in the upcoming release.
> Jim
>
This patch has been merged.
Jim
> > ---
> > libsepol/cil/src/cil_resolve_ast.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
> > index 33b9d321..49de8618 100644
> > --- a/libsepol/cil/src/cil_resolve_ast.c
> > +++ b/libsepol/cil/src/cil_resolve_ast.c
> > @@ -254,6 +254,12 @@ int cil_resolve_classpermissionset(struct cil_tree_node *current, struct cil_cla
> > goto exit;
> > }
> >
> > + if (!datum->fqn) {
> > + cil_tree_log(current, CIL_ERR, "Anonymous classpermission used in a classpermissionset");
> > + rc = SEPOL_ERR;
> > + goto exit;
> > + }
> > +
> > rc = cil_resolve_classperms_list(current, cps->classperms, extra_args);
> > if (rc != SEPOL_OK) {
> > goto exit;
> > --
> > 2.41.0
> >
