libsepol/cil: use-after-free with optional+class common

libsepol/cil: use-after-free with optional+class common

[Nicolas Iooss]

Hello,

I am continuing to investigate OSS-Fuzz crashes and the following one
is quite complex. Here is a CIL policy which triggers a
heap-use-after-free error in the CIL compiler:

(class CLASS (PERM2))
(classorder (CLASS))
(classpermission CLSPRM)
(optional o
    (mlsvalidatetrans x (domby l1 h1))
    (common CLSCOMMON (PERM1))
    (classcommon CLASS CLSCOMMON)
)
(classpermissionset CLSPRM (CLASS (PERM1)))

The issue is that the mlsvalidatetrans fails to resolve in pass
CIL_PASS_MISC3, which comes after the resolution of classcommon (in
pass CIL_PASS_MISC2). So:

* In pass CIL_PASS_MISC2, the optional block still exists, the
classcommon is resolved and class CLASS is linked with common
CLSCOMMON.
* In pass CIL_PASS_MISC3, the optional block is destroyed, including
the common CLSCOMMON.
* When classpermissionset is resolved, function cil_resolve_classperms
uses "common_symtab = &class->common->perms;", which has been freed.
The use-after-free issue occurs in __cil_resolve_perms (in
libsepol/cil/src/cil_resolve_ast.c):

  // common_symtab was freed
  rc = cil_symtab_get_datum(common_symtab, curr->data, &perm_datum);

I have not found a simple way to fix this issue. For example there is
no way to get all the classes which references a common, from a common
object (which would be helpful to remove these references when
destructing the common). Another idea would be to implement some kind
of reference counter and only really destroy the common when this
refcount reaches zero, but the current implementation does not do such
a thing.

How should this issue be fixed?

(For those who have access, the OSS-Fuzz issue is
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29002)

Nicolas

Re: libsepol/cil: use-after-free with optional+class common

[James Carter]

On Wed, Feb 3, 2021 at 3:49 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> Hello,
>
> I am continuing to investigate OSS-Fuzz crashes and the following one
> is quite complex. Here is a CIL policy which triggers a
> heap-use-after-free error in the CIL compiler:
>
> (class CLASS (PERM2))
> (classorder (CLASS))
> (classpermission CLSPRM)
> (optional o
>     (mlsvalidatetrans x (domby l1 h1))
>     (common CLSCOMMON (PERM1))
>     (classcommon CLASS CLSCOMMON)
> )
> (classpermissionset CLSPRM (CLASS (PERM1)))
>
> The issue is that the mlsvalidatetrans fails to resolve in pass
> CIL_PASS_MISC3, which comes after the resolution of classcommon (in
> pass CIL_PASS_MISC2). So:
>
> * In pass CIL_PASS_MISC2, the optional block still exists, the
> classcommon is resolved and class CLASS is linked with common
> CLSCOMMON.
> * In pass CIL_PASS_MISC3, the optional block is destroyed, including
> the common CLSCOMMON.
> * When classpermissionset is resolved, function cil_resolve_classperms
> uses "common_symtab = &class->common->perms;", which has been freed.
> The use-after-free issue occurs in __cil_resolve_perms (in
> libsepol/cil/src/cil_resolve_ast.c):
>
>   // common_symtab was freed
>   rc = cil_symtab_get_datum(common_symtab, curr->data, &perm_datum);
>
> I have not found a simple way to fix this issue. For example there is
> no way to get all the classes which references a common, from a common
> object (which would be helpful to remove these references when
> destructing the common). Another idea would be to implement some kind
> of reference counter and only really destroy the common when this
> refcount reaches zero, but the current implementation does not do such
> a thing.
>
> How should this issue be fixed?
>

The main problem here is that it really is not safe to destroy the
optional block subtree in the middle of the pass. Everything should
work if the disabled optional blocks are put in a list and destroyed
after the pass is finished and the reset is done. I will send out a
patch.

Thanks for looking into this.

Jim

> (For those who have access, the OSS-Fuzz issue is
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29002)
>
> Nicolas
>