Use of uninitialized memory of trailer list

[fish98]

Description

The use of uninitialized memory of the trailer array is found in function cups_fill of cups/file.c. Detailed code can be found below:

unsigned char   trailer[8]; // Trailer bytes
uLong       tcrc;       // Trailer CRC
ssize_t     tbytes = 0; // Number of bytes

if (fp->stream.avail_in > 0)
{
  if (fp->stream.avail_in > sizeof(trailer))
    tbytes = (ssize_t)sizeof(trailer);
  else
    tbytes = (ssize_t)fp->stream.avail_in;

  memcpy(trailer, fp->stream.next_in, (size_t)tbytes);
  fp->stream.next_in  += tbytes;
  fp->stream.avail_in -= (size_t)tbytes;
}

      if (tbytes < (ssize_t)sizeof(trailer))
{
  if (read(fp->fd, trailer + tbytes, sizeof(trailer) - (size_t)tbytes) < ((ssize_t)sizeof(trailer) - tbytes))
  {
...
  }
}

tcrc = ((((((uLong)trailer[3] << 8) | (uLong)trailer[2]) << 8) | (uLong)trailer[1]) << 8) | (uLong)trailer[0];

if (tcrc != fp->crc)
{ 
...

when vail_in is less than sizeof(trailer), the operation memcpy(trailer, fp->stream.next_in, (size_t)tbytes); will end up with uninitialized value in trailer array. The subsequent function if (read(fp->fd, trailer + tbytes, sizeof(trailer) - (size_t)tbytes) < ((ssize_t)sizeof(trailer) - tbytes)) may also inroduce unitialized value issue when read() function returns EOF or error.

Suggested Fix

1. Initialize trailer with zero e.g., unsigned char trailer[8] = {0};
2. Handle read() error

Postscript

The issue is identified by OSS-Fuzz harness fuzzipp with MSAN. Here is the linked issue.

[michaelrsweet]

OK, the code handles read errors - if we get less than "sizeof(trailer) - tbytes" bytes, that is an error and we abort.

I think the issue is that this and the CRC failure don't call inflateEnd.

Test this change:

[master 83562f7c7] Make sure we call inflateEnd when there is an error reading or comparing the stream CRC (Issue #91)