OpenPrinting / cups

OpenPrinting CUPS Sources
https://openprinting.github.io/cups
Apache License 2.0
1.1k stars 195 forks source link

Use of uninitialized memory of trailer list #1070

Closed fish98 closed 1 week ago

fish98 commented 1 month ago

Description

Similar to the reported issue in OpenPrinting/libcups#91. 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.

michaelrsweet commented 1 month ago

[master 7a2d383ee] Make sure we call inflateEnd when there is an error reading or comparing the stream CRC (Issue #1070)

[2.4.x fe04be501] Make sure we call inflateEnd when there is an error reading or comparing the stream CRC (Issue #1070)

michaelrsweet commented 1 month ago

To reiterate my comments on the libcups issue of the same name, this isn't caused by a lack of read validation but the fact that on error we didn't call inflateEnd...