Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buffer overflow in cupsRasterReadPixels #4551

Closed
michaelrsweet opened this issue Dec 29, 2014 · 7 comments
Labels
Milestone

Comments

@michaelrsweet
Copy link
Collaborator

@michaelrsweet michaelrsweet commented Dec 29, 2014

Version: 2.0.1
CUPS.org User: pdewacht

A malformed compressed raster file can trigger a buffer overflow in cupsRasterReadPixels. This is issue was found while testing my brlaser printer driver using american fuzzy lop.

Attached are:

  • testraster.c, a trivial test program
  • bogus.raster, one of the demonstration inputs found by afl
  • raster-buffer-overflow.patch, a proposed patch
@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

@michaelrsweet michaelrsweet commented Jan 4, 2015

CUPS.org User: mike

Um, the proposed patch isn't correct. The repeat count refers to the number of pixels when bitsPerPixel >= 8 vs. the number of bytes.

Will investigate further, but since compression isn't used between filters and since we use sandboxing whenever possible, it is unlikely that this will be exploitable (thus priority 2).

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

@michaelrsweet michaelrsweet commented Jan 30, 2015

CUPS.org User: mike

Fix is attached; the cause is that cupsBytesPerLine is not evenly divisible by cupsBitsPerColor/Pixel, which causes count (which is unsigned) to wrap around and extra memcpy's to be performed.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

@michaelrsweet michaelrsweet commented Feb 1, 2015

CUPS.org User: pdewacht

Looks ok, I can confirm that that patch fixes the buffer overflow. But can you add a (r->bpp > 0) condition before that modulo operator? Right now there's nothing that guarantees that r->bpp is non-zero.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

@michaelrsweet michaelrsweet commented Feb 17, 2015

CUPS.org User: odyx

Mike: What's your take on Peter's question on r->bpp guarantee to be non-zero? I'm discussing the patch with the Debian Security Team and the question popped up.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

@michaelrsweet michaelrsweet commented Feb 17, 2015

CUPS.org User: mike

Please file a new bug to track that change; we can certainly add a bpp check (or probably a check of both cupsBitsPerColor and cupsBitsPerPixel, which is what we use to calculate bpp), but without such a change all you'll manage to do is either crash the filter or (if for some reason the filter catches SIGFPE - I'm not aware of any that do) fail on the first read of 0 bytes.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

@michaelrsweet michaelrsweet commented Feb 17, 2015

"raster-buffer-overflow.patch":

--- a/filter/raster.c
+++ b/filter/raster.c
@@ -460,9 +460,9 @@

  while (count > 0)
  {
  •   memcpy(temp, temp - r->bpp, r->bpp);
    
  •   temp  += r->bpp;
    
  •   count -= r->bpp;
    
  •   *temp = *(temp - r->bpp);
    
  •   temp  += 1;
    
  •   count -= 1;
       }
    
    }
    }
@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

@michaelrsweet michaelrsweet commented Feb 17, 2015

"str4551.patch":

Index: filter/raster.c

--- filter/raster.c (revision 12451)
+++ filter/raster.c (working copy)
@@ -3,7 +3,7 @@
*

  • Raster file routines for CUPS.
    *

    • * Copyright 2007-2014 by Apple Inc.
    • * Copyright 2007-2015 by Apple Inc.
  • Copyright 1997-2006 by Easy Software Products.
    *

  • This file is part of the CUPS Imaging library.
    @@ -256,7 +256,10 @@
    */

    if (!cups_raster_read_header(r))

    • {
    • memset(h, 0, sizeof(cups_page_header_t));
      return (0);
    • }

    /*

    • Copy the header to the user-supplied buffer...
      @@ -285,7 +288,10 @@
      */

    if (!cups_raster_read_header(r))

    • {
    • memset(h, 0, sizeof(cups_page_header2_t));
      return (0);
    • }

    /*

    • Copy the header to the user-supplied buffer...
      @@ -964,7 +970,7 @@

    cups_raster_update(r);

  • return (r->header.cupsBytesPerLine != 0 && r->header.cupsHeight != 0);

  • return (r->header.cupsBytesPerLine != 0 && r->header.cupsHeight != 0 && (r->header.cupsBytesPerLine % r->bpp) == 0);
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.