[sane-devel] [PATCH] fix xsane PNM decoder

m. allan noah kitno455 at gmail.com
Tue Oct 23 20:45:59 UTC 2012


The author of XSane is not a member of this list. You should contact
him directly, to ensure he sees your patch.

allan

On Tue, Oct 23, 2012 at 9:31 AM, Yann Droneaud <yann at droneaud.fr> wrote:
> - be sure to read return codes not to enter in infinite loop
> - be resilient to malformed or unrecognized PNM file
> - report errors in debug mode
>
> Signed-off-by: Yann Droneaud <yann at droneaud.fr>
> ---
>  src/xsane-save.c | 300 +++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 223 insertions(+), 77 deletions(-)
>
> Hi,
>
> While trying to create/modify a multipage project, my XSanes window became
> unresponsive. This weird behavor was related to my modifications:
> I made changes on the images using GIMP behind XSane's back.
>
> This is related in bug #869175
> https://bugzilla.redhat.com/show_bug.cgi?id=869175
>
> After digging inside the source code, I've found that XSane was relying on
> a specific PNM format[1][2][3] with crafted headers.
> If those headers were not present, the decoder get stuck in an infinite loop.
>
> It would be great if XSane allows me to use an external tool to modify the
> file inside a multipage project.
>
> While I understand the need for the extra information stored in the XSane PNM
> files, I wander if they shouldn't be put in a associate meta data file,
> especially, in the case of multipage project, in the project description,
> so that outside modification can be detected.
>
> In the mean time, I improved the PNM parser to make the XSane header really
> optional and not block the tool when they are not present.
> This way I can do any horrible thing on the images before creating the final
> multipage PDF.
>
> Here's the patch.
>
> Regards.
> [1] http://netpbm.sourceforge.net/doc/ppm.html
> [2] http://netpbm.sourceforge.net/doc/pnm.html
> [3] http://en.wikipedia.org/wiki/Netpbm_format
>
> ---
> Yann Droneaud
>
> diff --git a/src/xsane-save.c b/src/xsane-save.c
> index f53f5e6..a4b24a7 100644
> --- a/src/xsane-save.c
> +++ b/src/xsane-save.c
> @@ -26,6 +26,7 @@
>  #include "xsane-back-gtk.h"
>  #include "xsane-front-gtk.h"
>  #include "xsane-save.h"
> +#include <ctype.h>
>  #include <time.h>
>  #include <sys/wait.h>
>
> @@ -421,107 +422,224 @@ void xsane_update_counter_in_filename(char **filename, int skip, int step, int m
>
>  void xsane_read_pnm_header(FILE *file, Image_info *image_info)
>  {
> - int max_val, filetype_nr;
> - char buf[TEXTBUFSIZE];
> - int items_done;
> +  int filetype_nr;
> +  int width;
> +  int height;
> +  int max_val;
> +  char buf[TEXTBUFSIZE];
> +  int items_done;
> +  char *ret;
> +  int c;
> +  int xsane_header_parsing = 0;
> +  int xsane_header_count = 0;
> +
> +  memset(image_info, 0, sizeof(*image_info));
>
> -  fgets(buf, sizeof(buf)-1, file);
> -  DBG(DBG_info, "filetype header :%s", buf);
> +  ret = fgets(buf, sizeof(buf)-1, file);
> +  if (ret == NULL)
> +  {
> +    DBG(DBG_error, "can't read PNM header: %s\n", strerror(errno));
> +    goto error;
> +  }
>
>    if (buf[0] == 'P')
>    {
> -    filetype_nr = atoi(buf+1); /* get filetype number */
> +    /* get filetype number and check PNM header format */
> +    items_done = sscanf(buf, "P%d %d %d %d", &filetype_nr, &width, &height, &max_val);
> +    if (items_done != 1)
> +    {
> +      DBG(DBG_error, "unsupported PNM header format\n");
> +      goto error;
> +    }
> +
> +    DBG(DBG_info, "filetype header :P%d\n", filetype_nr);
> +
> +    if (filetype_nr > 6)
> +    {
> +      DBG(DBG_error, "unrecognized PNM type\n");
> +      goto error;
> +    }
>
>      image_info->resolution_x = 72.0;
>      image_info->resolution_y = 72.0;
>      image_info->reduce_to_lineart = FALSE;
>      image_info->enable_color_management = FALSE;
>
> -    while (strcmp(buf, "# XSANE data follows\n"))
> +    /* parse header:
> +     *
> +     * # comment
> +     * width height
> +     * max
> +     *
> +     */
> +    while (1)
>      {
> -      fgets(buf, sizeof(buf)-1, file);
> -
> -      if (!strncmp(buf, "#  resolution_x    =", 20))
> -      {
> -        sscanf(buf+20, "%lf", &image_info->resolution_x);
> -      }
> -      else if (!strncmp(buf, "#  resolution_y    =", 20))
> -      {
> -        sscanf(buf+20, "%lf", &image_info->resolution_y);
> -      }
> -      else if (!strncmp(buf, "#  threshold       =", 20))
> -      {
> -        sscanf(buf+20, "%lf", &image_info->threshold);
> -      }
> -      else if (!strncmp(buf, "#  gamma           =", 20))
> -      {
> -        sscanf(buf+20, "%lf", &image_info->gamma);
> -      }
> -      else if (!strncmp(buf, "#  gamma      IRGB =", 20))
> -      {
> -        sscanf(buf+20, "%lf %lf %lf %lf",
> -                        &image_info->gamma,
> -                             &image_info->gamma_red,
> -                                &image_info->gamma_green,
> -                                    &image_info->gamma_blue);
> -      }
> -      else if (!strncmp(buf, "#  brightness      =", 20))
> -      {
> -        sscanf(buf+20, "%lf", &image_info->brightness);
> -      }
> -      else if (!strncmp(buf, "#  brightness IRGB =", 20))
> -      {
> -        sscanf(buf+20, "%lf %lf %lf %lf",
> -                        &image_info->brightness,
> -                             &image_info->brightness_red,
> -                                &image_info->brightness_green,
> -                                    &image_info->brightness_blue);
> -      }
> -      else if (!strncmp(buf, "#  contrast        =", 20))
> -      {
> -        sscanf(buf+20, "%lf", &image_info->contrast);
> -      }
> -      else if (!strncmp(buf, "#  contrast   IRGB =", 20))
> +      /* read just one char */
> +      c = fgetc(file);
> +      if (c == EOF)
>        {
> -        sscanf(buf+20, "%lf %lf %lf %lf",
> -                        &image_info->contrast,
> -                             &image_info->contrast_red,
> -                                &image_info->contrast_green,
> -                                    &image_info->contrast_blue);
> +       DBG(DBG_error, "error while reading PNM header: %s\n", strerror(errno));
> +       goto error;
>        }
> -      else if (!strncmp(buf, "#  color-management=", 20))
> -      {
> -        sscanf(buf+20, "%d", &image_info->enable_color_management);
> -      }
> -      else if (!strncmp(buf, "#  cms-function    =", 20))
> -      {
> -        sscanf(buf+20, "%d", &image_info->cms_function);
> -      }
> -      else if (!strncmp(buf, "#  cms-intent      =", 20))
> +
> +      /* push back to be used by fgets() or fscanf() below */
> +      ungetc(c, file);
> +
> +      /* end of header */
> +      if (c != '#')
>        {
> -        sscanf(buf+20, "%d", &image_info->cms_intent);
> +       break;
>        }
> -      else if (!strncmp(buf, "#  cms-bpc         =", 20))
> +
> +      /* read the comment line */
> +      ret = fgets(buf, sizeof(buf)-1, file);
> +      if (ret == NULL)
>        {
> -        sscanf(buf+20, "%d", &image_info->cms_bpc);
> +       DBG(DBG_error, "error while reading PNM header: %s\n", strerror(errno));
> +       goto error;
>        }
> -      else if (!strncmp(buf, "#  icm-profile     =", 20))
> +
> +      if (!xsane_header_parsing)
>        {
> -        sscanf(buf+20, "%s", image_info->icm_profile);
> +       if (!strcmp(buf, "# XSane settings:\n"))
> +       {
> +         /* start of xsane heaser */
> +         xsane_header_parsing = 1;
> +         xsane_header_count++;
> +       }
> +       else if (!strcmp(buf, "# XSANE data follows\n"))
> +       {
> +         DBG(DBG_info, "premature end of XSANE PNM header\n");
> +       }
>        }
> -      else if (!strncmp(buf, "#  reduce to lineart", 20))
> +      else /* xsane_header_parsing */
>        {
> -        image_info->reduce_to_lineart = TRUE;
> +       if (!strcmp(buf, "# XSANE data follows\n"))
> +       {
> +         /* end of xsane header */
> +         xsane_header_parsing = 0;
> +       }
> +       else if (!strcmp(buf, "# XSane settings:\n"))
> +       {
> +         DBG(DBG_info, "XSANE header inside XSANE header !\n");
> +       }
> +       else if (!strncmp(buf, "#  resolution_x    =", 20))
> +       {
> +         sscanf(buf+20, "%lf", &image_info->resolution_x);
> +       }
> +       else if (!strncmp(buf, "#  resolution_y    =", 20))
> +       {
> +         sscanf(buf+20, "%lf", &image_info->resolution_y);
> +       }
> +       else if (!strncmp(buf, "#  threshold       =", 20))
> +       {
> +         sscanf(buf+20, "%lf", &image_info->threshold);
> +       }
> +       else if (!strncmp(buf, "#  gamma           =", 20))
> +       {
> +         sscanf(buf+20, "%lf", &image_info->gamma);
> +       }
> +       else if (!strncmp(buf, "#  gamma      IRGB =", 20))
> +       {
> +         sscanf(buf+20, "%lf %lf %lf %lf",
> +                &image_info->gamma,
> +                &image_info->gamma_red,
> +                &image_info->gamma_green,
> +                &image_info->gamma_blue);
> +       }
> +       else if (!strncmp(buf, "#  brightness      =", 20))
> +       {
> +         sscanf(buf+20, "%lf", &image_info->brightness);
> +       }
> +       else if (!strncmp(buf, "#  brightness IRGB =", 20))
> +       {
> +         sscanf(buf+20, "%lf %lf %lf %lf",
> +                &image_info->brightness,
> +                &image_info->brightness_red,
> +                &image_info->brightness_green,
> +                &image_info->brightness_blue);
> +       }
> +       else if (!strncmp(buf, "#  contrast        =", 20))
> +       {
> +         sscanf(buf+20, "%lf", &image_info->contrast);
> +       }
> +       else if (!strncmp(buf, "#  contrast   IRGB =", 20))
> +       {
> +         sscanf(buf+20, "%lf %lf %lf %lf",
> +                &image_info->contrast,
> +                &image_info->contrast_red,
> +                &image_info->contrast_green,
> +                &image_info->contrast_blue);
> +       }
> +       else if (!strncmp(buf, "#  color-management=", 20))
> +       {
> +         sscanf(buf+20, "%d", &image_info->enable_color_management);
> +       }
> +       else if (!strncmp(buf, "#  cms-function    =", 20))
> +       {
> +         sscanf(buf+20, "%d", &image_info->cms_function);
> +       }
> +       else if (!strncmp(buf, "#  cms-intent      =", 20))
> +       {
> +         sscanf(buf+20, "%d", &image_info->cms_intent);
> +       }
> +       else if (!strncmp(buf, "#  cms-bpc         =", 20))
> +       {
> +         sscanf(buf+20, "%d", &image_info->cms_bpc);
> +       }
> +       else if (!strncmp(buf, "#  icm-profile     =", 20))
> +       {
> +         sscanf(buf+20, "%s", image_info->icm_profile);
> +       }
> +       else if (!strncmp(buf, "#  reduce to lineart", 20))
> +       {
> +         image_info->reduce_to_lineart = TRUE;
> +       }
> +       else
> +       {
> +         DBG(DBG_info, "unrecognized XSANE PNM header: %s", buf);
> +       }
>        }
>      }
>
> -    items_done = fscanf(file, "%d %d", &image_info->image_width, &image_info->image_height);
> +    if (xsane_header_parsing)
> +    {
> +      DBG(DBG_info, "unfinished XSANE PNM header\n");
> +    }
> +    if (xsane_header_count == 0)
> +    {
> +      DBG(DBG_info, "no XSANE PNM header\n");
> +    }
> +    else if (xsane_header_count > 1)
> +    {
> +      DBG(DBG_info, "too many XSANE PNM headers\n");
> +    }
>
> -    image_info->depth = 1;
> +    /* read image size */
> +    if (filetype_nr == 1 || filetype_nr == 4) /* P4 = lineart */
> +    {
> +      items_done = fscanf(file, "%d %d", &width, &height);
> +      if (items_done != 2)
> +      {
> +       DBG(DBG_error, "unrecognized PNM width/height: %s", buf);
> +       goto error;
> +      }
>
> -    if (filetype_nr != 4) /* P4 = lineart */
> +      image_info->image_width = width;
> +      image_info->image_height = height;
> +      image_info->depth = 1;
> +    }
> +    else
>      {
> -      items_done = fscanf(file, "%d", &max_val);
> +      items_done = fscanf(file, "%d %d %d", &width, &height, &max_val);
> +      if (items_done != 3)
> +      {
> +       DBG(DBG_error, "unrecognized PNM width/height: %s", buf);
> +       goto error;
> +      }
> +
> +      image_info->image_width = width;
> +      image_info->image_height = height;
>
>        if (max_val == 255)
>        {
> @@ -531,10 +649,20 @@ void xsane_read_pnm_header(FILE *file, Image_info *image_info)
>        {
>          image_info->depth = 16;
>        }
> +      else
> +      {
> +       DBG(DBG_error, "unrecognized PNM maximum value: %d\n", max_val);
> +       goto error;
> +      }
>      }
>
> -    fgetc(file); /* read exactly one newline character */
> -
> +    c = fgetc(file); /* read exactly one whitespace character */
> +
> +    if (c == EOF || !isspace(c))
> +    {
> +      DBG(DBG_error, "unrecognized PNM header\n");
> +      goto error;
> +    }
>
>      image_info->channels = 1;
>
> @@ -546,6 +674,8 @@ void xsane_read_pnm_header(FILE *file, Image_info *image_info)
>  #ifdef SUPPORT_RGBA
>    else if (buf[0] == 'S') /* RGBA format */
>    {
> +    DBG(DBG_info, "filetype header :%s", buf);
> +
>      items_done = fscanf(file, "%d %d\n%d", &image_info->image_width, &image_info->image_height, &max_val);
>      fgetc(file); /* read exactly one newline character */
>
> @@ -563,10 +693,26 @@ void xsane_read_pnm_header(FILE *file, Image_info *image_info)
>      image_info->channels = 4;
>    }
>  #endif
> +  else
> +  {
> +    DBG(DBG_error, "error: unrecognized filetype header :%s", buf);
> +    goto error;
> +  }
>
>    DBG(DBG_info, "xsane_read_pnm_header: width=%d, height=%d, depth=%d, colors=%d, resolution_x=%f, resolution_y=%f\n",
>        image_info->image_width, image_info->image_height, image_info->depth, image_info->channels,
>        image_info->resolution_x, image_info->resolution_y);
> +
> +  return;
> +
> + error:
> +
> +  rewind(file);
> +  memset(image_info, 0, sizeof(*image_info));
> +
> +  DBG(DBG_info, "xsane_read_pnm_header: failed to parse image\n");
> +
> +  return;
>  }
>
>  /* ---------------------------------------------------------------------------------------------------------------------- */
> --
> 1.7.11.7
>
>
> --
> sane-devel mailing list: sane-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/sane-devel
> Unsubscribe: Send mail with subject "unsubscribe your_password"
>              to sane-devel-request at lists.alioth.debian.org



-- 
"The truth is an offense, but not a sin"



More information about the sane-devel mailing list