On Tue, Feb 2, 2021 at 1:37 AM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
>
> Hello,
>
> OSS-Fuzz found an integer overflow when compiling the following
> (empty) CIL policy:
>
> ;;*lms 2147483647 a
> ; (empty line)
>
> ";;*lms" is a line mark which can be produced by the HLL compiler (if
> I understand correctly the meaning of CIL_KEY_HLL_LMS in
> libsepol/cil/src/cil_parser.c). The line number is parsed as an "int"
> variable:
>
> *hll_lineno = strtol(tok.value, &end, 10);
> if (errno == ERANGE || *end != '\0') {
> cil_log(CIL_ERR, "Problem parsing line number for line mark\n");
> goto exit;
> }
>
> This code has another issue which is that it silently truncates values
> to 32-bit signed integers on systems where sizeof(long) is 8, because
> hll_lineno is of type "int *", not "long *".
>
> But the issue found by OSS-Fuzz is that when 2147483647 is used (which
> is INT_MAX, 0x7fffffff in hexadecimal), "hll_lineno++;" overflows the
> capacity of signed integers, in cil_parser(), and this is an undefined
> behavior. This could be fixed by limiting the number of lines in a
> source file to some sane value. Another approach consists in emitting
> a warning and resetting the line counter every time it reaches
> INT_MAX. Thoughts?
I would lean towards using the proper type, so we get the full range depending
on architecture and warn on lineno wrap, but let it happen. IIUC,
Lineno is a helper.
I don't see it really affecting anything, so why make it a fatal error.
>
> For reference (and for the people who have access to it), the related
> OSS-Fuzz issue is
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28751.
>
> Cheers,
> Nicolas
>
