michaelrsweet / pappl

PAPPL - Printer Application Framework
https://www.msweet.org/pappl
Apache License 2.0
309 stars 49 forks source link

When printing JPG image with Printer Application the Printer Application crashes #61

Closed tillkamppeter closed 3 years ago

tillkamppeter commented 3 years ago

Describe the bug After having added raster callbacks to my PostScript Printer Application to support PWG/Apple Raster I have tried to switch image printing from the cups-filters-based approach to PAPPL's built-in image input support which uses the raster callbacks. I send a photo in landscape orientation. PAPPL's papplJobFilterImage() function automatically rotates the image counterclockwise to fit it into the page. When sending the raster image the Printer Application crashes mid-way. From the image data I have found out that for landscape-oriented pictures the pointer to the first pixel to grab in the original image buffer is wrong and so only the last forth of the image is sent and after that the pointer goes beyond the buffer and the Printer Application crashes. Following patch fixes this:

--- a/pappl/job-filter.c
+++ b/pappl/job-filter.c
@@ -180,7 +180,7 @@ papplJobFilterImage(
    break;

     case IPP_ORIENT_LANDSCAPE : // 90 counter-clockwise
-        pixbase    = pixels + width - depth;
+        pixbase    = pixels + width * depth - depth;
         img_width  = height;
         img_height = width;
         xdir       = (int)depth * (int)width;
@@ -329,7 +329,16 @@ papplJobFilterImage(
    for (x = xstart, lineptr = line + x * bpp, xerr = -xmod / 2; x < xend; x ++)
    {
      // Copy a grayscale or RGB pixel...
-     memcpy(lineptr, pixptr, bpp);
+     if (pixptr < pixels || pixptr > pixels + width * height * bpp)
+     {
+       if (pixptr < pixels)
+         papplLogJob(job, PAPPL_LOGLEVEL_DEBUG, "Accessed %ld bytes below original image space", pixels - pixptr);
+       else
+         papplLogJob(job, PAPPL_LOGLEVEL_DEBUG, "Accessed %ld bytes above original image space", pixptr - pixels - width * height * bpp);        
+       papplLogJob(job, PAPPL_LOGLEVEL_DEBUG, "width=%d, height=%d, bpp=%d, y=%d, ysize=%d, ydir=%d, x=%d, xstart=%d, xend=%d, xsize=%d, xdir=%d, xstep=%d, xmod=%d, xerr=%d", width, height, bpp, y, ysize, ydir, x, xstart, xend, xsize, xdir, xstep, xmod, xerr);
+     }
+     else
+       memcpy(lineptr, pixptr, bpp);
      lineptr += bpp;

      // Advance to the next pixel...

The actual fix is the first part of the patch, the second part is a crash guard which prevents a pixel from being grabbed from the original image if the pointer is beyond the buffer. In this case a debug message is shown. Even with the fix I recommend the crash guard being applied, to assure that rounding errors cannot crash the Printer Application.

To Reproduce Take a Printer Application and print a photo in landscape orientation, some time into the process the Printer Application crashes. Note that the PostScript Printer Application as currently on the GIT uses (still) cups-filters to print photos and so the crash does not occur.

System Information:

tillkamppeter commented 3 years ago

Note about the crash guard, I have only applied one for the standard RGB/Gray color space case, not for the reverse color spaces or 1-bit dithering which probably would need a crash guard, too.

tillkamppeter commented 3 years ago

Small correction for the crash guard: The "pixels" buffer size is width height depth, not width height bpp:

      if (pixptr < pixels || pixptr > pixels + width * height * depth)
      {
        if (pixptr < pixels)
          papplLogJob(job, PAPPL_LOGLEVEL_DEBUG, "Accessed %ld bytes below original image space", pixels - pixptr);
        else
          papplLogJob(job, PAPPL_LOGLEVEL_DEBUG, "Accessed %ld bytes above original image space", pixptr - pixels - width * height * depth);
        papplLogJob(job, PAPPL_LOGLEVEL_DEBUG, "width=%d, height=%d, depth=%d, bpp=%d, y=%d, ysize=%d, ydir=%d, x=%d, xstart=%d, xend=%d, xsize=%d, xdir=%d, xstep=%d, xmod=%d, xerr=%d", width, height, depth, bpp, y, ysize, ydir, x, xstart, xend, xsize, xdir, xstep, xmod, xerr);
      }
      else
        memcpy(lineptr, pixptr, bpp);
michaelrsweet commented 3 years ago

If I add the guard, I'll calculate the bounds up front and not for each line...

michaelrsweet commented 3 years ago

[master 35c8af8] Fix landscape image printing (Issue #61)