From 69ebc26b11d0ed64a8433a6318fc8e2a016e6f18 Mon Sep 17 00:00:00 2001 From: Zdenek Dohnal Date: Mon, 3 Feb 2025 16:24:24 +0100 Subject: [PATCH] Fix issues reported by OpenScanHub 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. --- cupsfilters/bannertopdf.c | 25 ++++++++++++++------- cupsfilters/fontembed/sfnt-subset.c | 8 ++++++- cupsfilters/ghostscript.c | 2 +- cupsfilters/image-jpeg.c | 22 +++++++++++++++++-- cupsfilters/image-png.c | 2 +- cupsfilters/image-tiff.c | 34 +++++++++++++++++++++++++---- cupsfilters/imagetopdf.c | 5 +++++ cupsfilters/imagetoraster.c | 2 +- cupsfilters/pdftopdf/pdftopdf.cxx | 20 ++++++++++++++--- cupsfilters/pwgtopdf.cxx | 24 +++++++++++++++----- cupsfilters/raster.c | 2 +- cupsfilters/testfilters.c | 1 + cupsfilters/texttopdf.c | 14 +++++++++++- 13 files changed, 132 insertions(+), 29 deletions(-) diff --git a/cupsfilters/bannertopdf.c b/cupsfilters/bannertopdf.c index eb9020d60..4da87ef9f 100644 --- 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 index f1803ed94..97ac35090 100644 --- 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 index 87ff5c003..fbcc6c40f 100644 --- 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 index 561d51d68..bcd6e3608 100644 --- 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 index 9690cdf5c..a6fd9a6dd 100644 --- 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 index d92cce25f..20dfbaee6 100644 --- 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 index 3ad72c433..f95c98597 100644 --- 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 index 0cb94506d..e22958623 100644 --- 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 index bdf2475d7..57e41a31e 100644 --- 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 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,9 +1000,9 @@ 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) @@ -1004,12 +1010,20 @@ cfFilterPDFToPDF(int inputfd, // I - File descriptor input stream // 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 index 06a215651..160a4e750 100644 --- 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,7 +1810,8 @@ 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; } } @@ -1813,8 +1819,8 @@ cfFilterPWGToPDF(int inputfd, // I - File descriptor input stream { 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 index 93975d3d2..528de7dda 100644 --- 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 index 9685bb803..91e2328f4 100644 --- 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 index f3f0a3e2f..eba323d6a 100644 --- 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; }