mbox series

[0/2] libsepol: Validate policydb values when reading binary

Message ID 20210205140800.275993-1-jwcart2@gmail.com (mailing list archive)
Headers show
Series libsepol: Validate policydb values when reading binary | expand

Message

James Carter Feb. 5, 2021, 2:07 p.m. UTC
Nicolas Iooss reports that fuzzing /usr/libexec/hll/pp with the
American Fuzzy Lop revealed that inconsistent policy modules could be
created that caused NULL dereferences and other problems.

This patch validates the policydb when reading in the binary policy. See
the description of the second patch for more details.

The validation requires a negligible amount of time to complete.

James Carter (2):
  libsepol: Create function ebitmap_highest_set_bit()
  libsepol: Validate policydb values when reading binary policy

 libsepol/include/sepol/policydb/ebitmap.h |   1 +
 libsepol/src/ebitmap.c                    |  20 +
 libsepol/src/policydb.c                   |  35 +-
 libsepol/src/policydb_validate.c          | 764 ++++++++++++++++++++++
 libsepol/src/policydb_validate.h          |   7 +
 5 files changed, 815 insertions(+), 12 deletions(-)
 create mode 100644 libsepol/src/policydb_validate.c
 create mode 100644 libsepol/src/policydb_validate.h

Comments

Nicolas Iooss Feb. 18, 2021, 7:31 a.m. UTC | #1
On Fri, Feb 5, 2021 at 3:08 PM James Carter <jwcart2@gmail.com> wrote:
>
> Nicolas Iooss reports that fuzzing /usr/libexec/hll/pp with the
> American Fuzzy Lop revealed that inconsistent policy modules could be
> created that caused NULL dereferences and other problems.
>
> This patch validates the policydb when reading in the binary policy. See
> the description of the second patch for more details.
>
> The validation requires a negligible amount of time to complete.
>
> James Carter (2):
>   libsepol: Create function ebitmap_highest_set_bit()
>   libsepol: Validate policydb values when reading binary policy
>
>  libsepol/include/sepol/policydb/ebitmap.h |   1 +
>  libsepol/src/ebitmap.c                    |  20 +
>  libsepol/src/policydb.c                   |  35 +-
>  libsepol/src/policydb_validate.c          | 764 ++++++++++++++++++++++
>  libsepol/src/policydb_validate.h          |   7 +
>  5 files changed, 815 insertions(+), 12 deletions(-)
>  create mode 100644 libsepol/src/policydb_validate.c
>  create mode 100644 libsepol/src/policydb_validate.h
>
> --
> 2.26.2
>

Hello,
Thanks for these patches! I tested them and the fuzzer I am using
(which consists in running AFL on "pp") no longer crashed :) So I
confirm they fixed the issues I was experiencing, and the code looks
good.

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Nicolas
Petr Lautrbach Feb. 19, 2021, 3:48 p.m. UTC | #2
Nicolas Iooss <nicolas.iooss@m4x.org> writes:

> On Fri, Feb 5, 2021 at 3:08 PM James Carter <jwcart2@gmail.com> wrote:
>>
>> Nicolas Iooss reports that fuzzing /usr/libexec/hll/pp with the
>> American Fuzzy Lop revealed that inconsistent policy modules could be
>> created that caused NULL dereferences and other problems.
>>
>> This patch validates the policydb when reading in the binary policy. See
>> the description of the second patch for more details.
>>
>> The validation requires a negligible amount of time to complete.
>>
>> James Carter (2):
>>   libsepol: Create function ebitmap_highest_set_bit()
>>   libsepol: Validate policydb values when reading binary policy
>>
>>  libsepol/include/sepol/policydb/ebitmap.h |   1 +
>>  libsepol/src/ebitmap.c                    |  20 +
>>  libsepol/src/policydb.c                   |  35 +-
>>  libsepol/src/policydb_validate.c          | 764 ++++++++++++++++++++++
>>  libsepol/src/policydb_validate.h          |   7 +
>>  5 files changed, 815 insertions(+), 12 deletions(-)
>>  create mode 100644 libsepol/src/policydb_validate.c
>>  create mode 100644 libsepol/src/policydb_validate.h
>>
>> --
>> 2.26.2
>>
>
> Hello,
> Thanks for these patches! I tested them and the fuzzer I am using
> (which consists in running AFL on "pp") no longer crashed :) So I
> confirm they fixed the issues I was experiencing, and the code looks
> good.
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Nicolas

Merged using --whitespace=fix
Thanks!

^&^ git am --whitespace=fix 2-2-libsepol-Validate-policydb-values-when-reading-binary-policy.patch                                                      
Applying: libsepol: Validate policydb values when reading binary policy
.git/rebase-apply/patch:331: trailing whitespace.
        return -1;
.git/rebase-apply/patch:590: trailing whitespace.
        return 0;
.git/rebase-apply/patch:747: trailing whitespace.
        return -1;
.git/rebase-apply/patch:763: trailing whitespace.
        return -1;
.git/rebase-apply/patch:886: trailing whitespace.
        return -1;