A crash found in png_warning when fuzzing

[hopper-vul]

Hi, I found a crash in png_warning by using fuzzing. The png_warning will crash when the length of second argument less than 15 and the first byte is 0x23, if the program is compiled with AddressSanitizer.

The root cause i found is that, png_warning check the first byte and increase an offset until to 15. If the input warning_message has less than 15 bytes, a out of bound read is happened. Then the after call png_default_warning will print the not owned memory, which maybe unexpected and insecure.

As png_warning could be a widely-used API, maybe this porblem should be fixed, thanks.

void PNGAPI
png_warning(png_const_structrp png_ptr, png_const_charp warning_message)
{
   int offset = 0;
   if (png_ptr != NULL)
   {
#ifdef PNG_ERROR_NUMBERS_SUPPORTED
   if ((png_ptr->flags &
       (PNG_FLAG_STRIP_ERROR_NUMBERS|PNG_FLAG_STRIP_ERROR_TEXT)) != 0)
#endif
      {
         if (*warning_message == PNG_LITERAL_SHARP) // check the first byte
         {
            for (offset = 1; offset < 15; offset++)
               if (warning_message[offset] == ' ') // out of bound read.
                  break;
         }
      }
      ...
      png_default_warning(png_ptr, warning_message + offset); // still out of bound use
}

[nayuki]

My description of the function's behavior: When calling png_warning(png_ptr, "#foo bar"), if the warning message starts with '#', then the function will scan up to 14 bytes after '#' to find the first occurrence of space, and then call png_default_warning with an offsetted message, like png_default_warning(png_ptr, " bar"). If the string ends before a space is found (e.g. "#hello"), then there may be out-of-bounds reads.

[jbowler]

If a warning or error message starts with a '#' then it must have the approved format; '#nnnn '.

PNG_ERROR_NUMBERS_SUPPORTED is off by default and three of the four pieces of code which handle the error numbers completely remove the handling so the message is unmodified if it starts with PNG_LITERAL_SHARP. png_warning, however, has a completely different block of code which looks, to be, to be wrong. The simple approach is to make png_warning have the same form as png_error; then the code simply won't be compiled in!

There seem to be problems in the code quite apart from not handling mal-formed messages. In the png_error case with the default error handler png_error strips the text down but then png_default_error repeats the excercise.

Consider a message "#42 bad argument" and what happens in png_error depending on the png_ptr flags:

with PNG_FLAG_STRIP_ERROR_TEXT: " bad argument" (note: leading space) else if PNG_FLAG_STRIP_ERROR_NUMBERS: "4" (note: truncated number)

Then if png_default_error is called there is only a leading '#' in the second (else) case with a malformed message starting with two '#', i.e. "##".

Clearly the code is totally broken. The code in png_default_* should not be there at all, the code in png_error needs to be fixed and png_warning needs the same code.

If anyone did do full i18n for the messages this would all get rewritten anyway.