libucl icon indicating copy to clipboard operation
libucl copied to clipboard

OSS-fuzz

Open AdamKorcz opened this issue 4 years ago • 1 comments

The fuzzer for ucl_parser_add_string runs into a bug very quickly when fuzzing the function:

https://github.com/vstakhov/libucl/blob/91a3fb447386fa72e2ad35f9922b47640a935bca/src/ucl_parser.c#L3049-L3059

This function accepts a length argument, however, the following function:

https://github.com/vstakhov/libucl/blob/91a3fb447386fa72e2ad35f9922b47640a935bca/src/ucl_parser.c#L3033-L3047

may cause a buffer overflow if the len argument is 0 and the data string is not null-terminated.

I suggest we do one of two things:

  • Remove the use of strlen and len == 0 all together and simply remove the lines (3041-3043)
  • Make it explicit that if len == 0 then the string is expected to be null-terminated and otherwise undefined behavior may occur

The OSS-Fuzz build is currently blocking because the bug is found too quickly: https://travis-ci.org/github/google/oss-fuzz/jobs/670351676?utm_medium=notification&utm_source=github_status

AdamKorcz avatar Apr 03 '20 10:04 AdamKorcz

Yes, assuming that len == 0 implies zero terminated string was a mistake. However, I'd prefer the second option as it will not break the existing API and its usage. I would like to go with the first option if compatibility was not important tbh...

vstakhov avatar Apr 03 '20 11:04 vstakhov