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

Yann Droneaud yann at droneaud.fr
Tue Oct 23 13:31:57 UTC 2012


- 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




More information about the sane-devel mailing list