Fix issues reported by OpenScanHub (#79)

Open source static analyzer OpenScanHub found several issues regarding resource leaks, security, buffer overflows etc., which this PR aims to address. libcupsfilters passes sanity testing with the changes.
OpenPrinting · Feb 11, 2025 · d21dd1c · d21dd1c
1 parent 0158844
commit d21dd1c
Show file tree

Hide file tree

Showing 13 changed files with 132 additions and 29 deletions.
diff --git a/cupsfilters/bannertopdf.c b/cupsfilters/bannertopdf.c
@@ -63,9 +63,12 @@ banner_free(banner_t *banner)
 {
   if (banner)
   {
-    free(banner->template_file);
-    free(banner->header);
-    free(banner->footer);
+    if (banner->template_file)
+      free(banner->template_file);
+    if (banner->header)
+      free(banner->header);
+    if (banner->footer)
+      free(banner->footer);
     free(banner);
   }
 }
@@ -224,7 +227,12 @@ banner_new_from_file(const char *filename,
       break;
   }
 
-  banner = calloc(1, sizeof *banner);
+  if ((banner = calloc(1, sizeof *banner)) == NULL)
+  {
+    if (log)
+      log(ld, CF_LOGLEVEL_ERROR, "cfFilterBannerToPDF: Not enough memory");
+    goto out;
+  }
 
   while (getline(&line, &len, f) != -1)
   {
@@ -248,12 +256,12 @@ banner_new_from_file(const char *filename,
       while (*key == '%')
 	key ++;
 
-    if (!strcasecmp(key, "template"))
+    if (!banner->template_file && !strcasecmp(key, "template"))
       banner->template_file = template_path(value, datadir);
-    else if (!strcasecmp(key, "header"))
-      banner->header = strdup(value);
-    else if (!strcasecmp(key, "footer"))
+    else if (!banner->header && !strcasecmp(key, "header"))
       banner->header = strdup(value);
+    else if (!banner->footer && !strcasecmp(key, "footer"))
+      banner->footer = strdup(value);
     else if (!strcasecmp(key, "font"))
       *num_options = cupsAddOption("banner-font",
 				   strdup(value), *num_options, options);
@@ -1003,6 +1011,7 @@ cfFilterBannerToPDF(int inputfd,         // I - File descriptor input stream
   banner_free(banner);
   if (options) cupsFreeOptions(num_options, options);
   unlink(tempfile);
+  fclose(inputfp);
 
   return (ret);
 }
diff --git a/cupsfilters/fontembed/sfnt-subset.c b/cupsfilters/fontembed/sfnt-subset.c
@@ -229,7 +229,13 @@ _cfFontEmbedOTFSubSet(_cf_fontembed_otf_file_t *otf,
     if (glyphs[b] & c)
     {
       const int len = _cfFontEmbedOTFGetGlyph(otf, iA);
-      DEBUG_assert(len >= 0);
+      if (len < 0)
+      {
+        fprintf(stderr, "Cannot get the glyph.\n");
+        free(new_loca);
+        free(new_glyf);
+        return (-1);
+      }
       memcpy(new_glyf + offset, otf->gly, len);
       offset += len;
     }

diff --git a/cupsfilters/ghostscript.c b/cupsfilters/ghostscript.c
@@ -780,7 +780,7 @@ cfFilterGhostscript(int inputfd,            // I - File descriptor input
 {
   cf_filter_out_format_t outformat;
   char buf[BUFSIZ];
-  char *filename;
+  char *filename = NULL;
   char *icc_profile = NULL;
   char *tmp;
   char tmpstr[1024],

diff --git a/cupsfilters/image-jpeg.c b/cupsfilters/image-jpeg.c
@@ -175,8 +175,26 @@ _cfImageReadJPEG(
 
   cfImageSetMaxTiles(img, 0);
 
-  in  = malloc(img->xsize * cinfo.output_components);
-  out = malloc(img->xsize * cfImageGetDepth(img));
+  if ((in  = (cf_ib_t*)calloc(img->xsize * cinfo.output_components, sizeof(cf_ib_t))) == NULL)
+  {
+    DEBUG_printf("DEBUG: Not enough memory.");
+
+    jpeg_destroy_decompress(&cinfo);
+
+    fclose(fp);
+    return (1);
+  }
+
+  if ((out = (cf_ib_t*)calloc(img->xsize * cfImageGetDepth(img), sizeof(cf_ib_t))) == NULL)
+  {
+    DEBUG_printf("DEBUG: Not enough memory.");
+
+    jpeg_destroy_decompress(&cinfo);
+
+    free(in);
+    fclose(fp);
+    return (1);
+  }
 
   jpeg_start_decompress(&cinfo);
 

diff --git a/cupsfilters/image-png.c b/cupsfilters/image-png.c
@@ -211,7 +211,7 @@ _cfImageReadPNG(
   }
 
   bpp = cfImageGetDepth(img);
-  out = malloc(img->xsize * bpp);
+  out = (cf_ib_t*)calloc(img->xsize * bpp, sizeof(cf_ib_t));
 
   if (!in || !out)
   {

diff --git a/cupsfilters/image-tiff.c b/cupsfilters/image-tiff.c
@@ -316,8 +316,21 @@ _cfImageReadTIFF(
     else
       pstep = xdir;
 
-    in  = malloc(img->xsize * 3 + 3);
-    out = malloc(img->xsize * bpp);
+    if ((in  = (cf_ib_t*)calloc(img->xsize * 3 + 3, sizeof(cf_ib_t))) == NULL)
+    {
+      DEBUG_puts("DEBUG: No enough memory.\n");
+      TIFFClose(tif);
+      fclose(fp);
+      return (1);
+    }
+    if ((out = (cf_ib_t*)calloc(img->xsize * bpp, sizeof(cf_ib_t))) == NULL)
+    {
+      DEBUG_puts("DEBUG: No enough memory.\n");
+      free(in);
+      TIFFClose(tif);
+      fclose(fp);
+      return (1);
+    }
   }
   else
   {
@@ -326,8 +339,21 @@ _cfImageReadTIFF(
     else
       pstep = ydir;
 
-    in  = malloc(img->ysize * 3 + 3);
-    out = malloc(img->ysize * bpp);
+    if ((in  = (cf_ib_t*)calloc(img->ysize * 3 + 3, sizeof(cf_ib_t))) == NULL)
+    {
+      DEBUG_puts("DEBUG: No enough memory.\n");
+      TIFFClose(tif);
+      fclose(fp);
+      return (1);
+    }
+    if ((out = (cf_ib_t*)calloc(img->ysize * bpp, sizeof(cf_ib_t))) == NULL)
+    {
+      DEBUG_puts("DEBUG: No enough memory.\n");
+      free(in);
+      TIFFClose(tif);
+      fclose(fp);
+      return (1);
+    }
   }
 
   //

diff --git a/cupsfilters/imagetopdf.c b/cupsfilters/imagetopdf.c
@@ -656,6 +656,7 @@ cfFilterImageToPDF(int inputfd,         // I - File descriptor input stream
   doc.pageObjects = NULL;
   doc.gammaval = 1.0;
   doc.brightness = 1.0;
+  doc.row = NULL;
 
   //
   // Open the input data stream specified by the inputfd ...
@@ -1871,6 +1872,8 @@ cfFilterImageToPDF(int inputfd,         // I - File descriptor input stream
   //
 
   cfImageClose(doc.img);
+  free(doc.row);
+  free(doc.pageObjects);
   fclose(doc.outputfp);
   close(outputfd);
   return (0);
@@ -1881,6 +1884,8 @@ cfFilterImageToPDF(int inputfd,         // I - File descriptor input stream
 	       "cfFilterImageToPDF: Cannot allocate any more memory.");
   free_all_obj(&doc);
   cfImageClose(doc.img);
+  free(doc.row);
+  free(doc.pageObjects);
   fclose(doc.outputfp);
   close(outputfd);
   return (2);

diff --git a/cupsfilters/imagetoraster.c b/cupsfilters/imagetoraster.c
@@ -259,7 +259,7 @@ cfFilterImageToRaster(int inputfd,         // I - File descriptor input stream
         		customBottom = 0.0,
 			customRight = 0.0,
 			customTop = 0.0;
-  char 			defSize[41];
+  char 			defSize[64];
   cf_filter_out_format_t outformat;
 
   //

diff --git a/cupsfilters/pdftopdf/pdftopdf.cxx b/cupsfilters/pdftopdf/pdftopdf.cxx
@@ -859,8 +859,8 @@ cfFilterPDFToPDF(int inputfd,            // I - File descriptor input stream
 {
   pdftopdf_doc_t     doc;                // Document information
   char               *final_content_type = data->final_content_type;
-  FILE               *inputfp,
-                     *outputfp;
+  FILE               *inputfp = NULL,
+                     *outputfp = NULL;
   const char         *t;
   int                streaming = 0;
   size_t             bytes;
@@ -953,7 +953,10 @@ cfFilterPDFToPDF(int inputfd,            // I - File descriptor input stream
 
       // Process the PDF input data
       if (!_cfProcessPDFToPDF(*proc, param, &doc))
+      {
+	fclose(inputfp);
 	return (2);
+      }
 
       // Pass information to subsequent filters via PDF comments
       std::vector<std::string> output;
@@ -978,7 +981,10 @@ cfFilterPDFToPDF(int inputfd,            // I - File descriptor input stream
 
     outputfp = fdopen(outputfd, "w");
     if (outputfp == NULL)
+    {
+      fclose(inputfp);
       return (1);
+    }
 
     if (!streaming)
     {
@@ -994,22 +1000,30 @@ cfFilterPDFToPDF(int inputfd,            // I - File descriptor input stream
       while ((bytes = fread(buf, 1, sizeof(buf), inputfp)) > 0)
 	if (fwrite(buf, 1, bytes, outputfp) != bytes)
 	  break;
-      fclose(inputfp);
     }
 
+    fclose(inputfp);
     fclose(outputfp);
   }
   catch (std::exception &e)
   {
     // TODO? exception type
     if (log) log(ld, CF_LOGLEVEL_ERROR,
 		 "cfFilterPDFToPDF: Exception: %s", e.what());
+    if (inputfp)
+      fclose(inputfp);
+    if (outputfp)
+      fclose(outputfp);
     return (5);
   }
   catch (...)
   {
     if (log) log(ld, CF_LOGLEVEL_ERROR,
 		 "cfFilterPDFToPDF: Unknown exception caught. Exiting.");
+    if (inputfp)
+      fclose(inputfp);
+    if (outputfp)
+      fclose(outputfp);
     return (6);
   }
 

diff --git a/cupsfilters/pwgtopdf.cxx b/cupsfilters/pwgtopdf.cxx
@@ -1536,6 +1536,7 @@ cfFilterPWGToPDF(int inputfd,  // I - File descriptor input stream
   int			total_attrs;
   char			buf[1024];
   const char		*kw;
+  int			ret = -1;	// Return value
 
 
   (void)inputseekable;
@@ -1602,6 +1603,7 @@ cfFilterPWGToPDF(int inputfd,  // I - File descriptor input stream
   {
     if (log) log(ld, CF_LOGLEVEL_ERROR,
       "cfFilterPWGToPDF: PCLm output: No printer IPP attributes are supplied, PCLm output not possible.");
+    fclose(outputfp);
     return (1);
   }
 
@@ -1703,7 +1705,8 @@ cfFilterPWGToPDF(int inputfd,  // I - File descriptor input stream
     {
       if (log) log(ld, CF_LOGLEVEL_ERROR,
 		   "cfFilterPWGToPDF: PCLm output: Printer IPP attributes do not contain printer resolution information for PCLm.");
-      return (1);
+      ret = 1;
+      goto error;
     }
 
     attr_name = (char *)"pclm-compression-method-preferred";
@@ -1759,7 +1762,8 @@ cfFilterPWGToPDF(int inputfd,  // I - File descriptor input stream
       {
 	if (log) log(ld, CF_LOGLEVEL_ERROR,
 		     "cfFilterPWGToPDF: Unable to create PDF file");
-	return (1);
+	ret = 1;
+	goto error;
       }
     }
 
@@ -1795,7 +1799,8 @@ cfFilterPWGToPDF(int inputfd,  // I - File descriptor input stream
     {
       if (log) log(ld, CF_LOGLEVEL_ERROR,
 		    "cfFilterPWGToPDF: Unable to start new PDF page");
-      return (1);
+      ret = 1;
+      goto error;
     }
 
     // Write the bit map into the PDF file
@@ -1805,16 +1810,17 @@ cfFilterPWGToPDF(int inputfd,  // I - File descriptor input stream
     {
       if (log) log(ld, CF_LOGLEVEL_ERROR,
 		   "cfFilterPWGToPDF: Failed to convert page bitmap");
-      return (1);
+      ret = 1;
+      goto error;
     }
   }
 
   if (empty)
   {
     if (log) log(ld, CF_LOGLEVEL_DEBUG,
 		 "cfFilterPWGToPDF: Input is empty, outputting empty file.");
-    cupsRasterClose(ras);
-    return (0);
+    ret = 0;
+    goto error;
   }
 
   close_pdf_file(&pdf, &doc); // output to outputfp
@@ -1826,4 +1832,10 @@ cfFilterPWGToPDF(int inputfd,  // I - File descriptor input stream
   fclose(outputfp);
 
   return (Page == 0);
+
+error:
+  cupsRasterClose(ras);
+  fclose(outputfp);
+
+  return (ret);
 }
diff --git a/cupsfilters/raster.c b/cupsfilters/raster.c
@@ -604,7 +604,7 @@ cfRasterSetColorSpace(cups_page_header_t *h,   // I  - Raster header
   int cspace_fallback = 0; // 0: originally requested color space
 			   // 1: sRGB for color, sGray for mono
 			   // 2: sRGB for mono
-  const char *p, *q;
+  const char *p, *q = NULL;
 
   // Range-check
   if (!h || !available || !cspace)

diff --git a/cupsfilters/testfilters.c b/cupsfilters/testfilters.c
@@ -189,6 +189,7 @@ test_wrapper(
 		strerror(errno));
 	fprintf(stderr, "ERROR: Unable to open Write file\n");
       }
+      close(inputfd);
 
       return (1);
     }

diff --git a/cupsfilters/texttopdf.c b/cupsfilters/texttopdf.c
@@ -621,7 +621,9 @@ cfFilterTextToPDF(int inputfd,  	// I - File descriptor input stream
   doc.PageTop = 756.0f;		// Top margin
   doc.PageWidth = 612.0f;	// Total page width
   doc.PageLength = 792.0f;	// Total page length
-  doc.pdf = NULL;
+  doc.pdf = NULL;		// PDF file contents
+  doc.Date = NULL;		// Date string
+  doc.Title = NULL;		// Title string
 
   if (parameters)
     doc.env_vars = *((cf_filter_texttopdf_parameter_t *)parameters);
@@ -1512,6 +1514,15 @@ cfFilterTextToPDF(int inputfd,  	// I - File descriptor input stream
     free(doc.Page);
   }
 
+  if (doc.PrettyPrint)
+  {
+    free(doc.Date);
+    free(doc.Title);
+  }
+
+  if (doc.pdf)
+    free(doc.pdf);
+
   return (ret);
 }
 
@@ -1766,6 +1777,7 @@ write_epilogue(texttopdf_doc_t *doc)
   _cfPDFOutFinishPDF(doc->pdf);
 
   _cfPDFOutFree(doc->pdf);
+  doc->pdf = NULL;
 }