diff --git a/Makefile.am b/Makefile.am index 8c8b44d9c..e500714e4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -97,7 +97,8 @@ lib_LTLIBRARIES = libcupsfilters.la check_SCRIPTS = \ cupsfilters/testfilters.sh \ cupsfilters/test-pclm-overflow.sh \ - cupsfilters/test-pdftoraster-copy-height.sh + cupsfilters/test-pdftoraster-copy-height.sh \ + cupsfilters/test-imagetoraster-overflow.sh check_PROGRAMS = \ testcmyk \ @@ -110,6 +111,7 @@ check_PROGRAMS = \ test-analyze \ test-pdf \ test-ps \ + testimagetoraster \ testfilters TESTS = \ @@ -119,9 +121,11 @@ TESTS = \ test-analyze \ test-pdf \ test-ps \ + testimagetoraster \ cupsfilters/testfilters.sh \ cupsfilters/test-pclm-overflow.sh \ - cupsfilters/test-pdftoraster-copy-height.sh + cupsfilters/test-pdftoraster-copy-height.sh \ + cupsfilters/test-imagetoraster-overflow.sh # testcmyk # fails as it opens some image.ppm which is nowerhe to be found. # testimage # requires also some ppm file as argument @@ -288,6 +292,18 @@ testrgb_LDADD = \ testrgb_CFLAGS = \ $(CUPS_CFLAGS) +testimagetoraster_SOURCES = \ + cupsfilters/testimagetoraster.c \ + $(pkgfiltersinclude_DATA) +testimagetoraster_CFLAGS = \ + $(LIBJPEG_CFLAGS) \ + $(CUPS_CFLAGS) +testimagetoraster_LDADD = \ + libcupsfilters.la \ + $(CUPS_LIBS) \ + $(LIBJPEG_LIBS) \ + -lm + test1284_SOURCES = \ cupsfilters/test1284.c test1284_LDADD = \ diff --git a/cupsfilters/imagetoraster.c b/cupsfilters/imagetoraster.c index eea5602da..5e81683ae 100644 --- a/cupsfilters/imagetoraster.c +++ b/cupsfilters/imagetoraster.c @@ -1259,7 +1259,7 @@ cfFilterImageToRaster(int inputfd, // I - File descriptor input stream // If size if specified by user, use it, else default size from // printer_attrs - strcpy(defSize, header.cupsPageSizeName); + snprintf(defSize, sizeof(defSize), "%s", header.cupsPageSizeName); if ((strncasecmp(defSize, "Custom", 6)) == 0 || strcasestr(defSize, "_custom_")) @@ -1308,7 +1308,7 @@ cfFilterImageToRaster(int inputfd, // I - File descriptor input stream // Set the new custom size... // - strcpy(header.cupsPageSizeName, "Custom"); + snprintf(header.cupsPageSizeName, sizeof(header.cupsPageSizeName), "%s", "Custom"); header.cupsPageSize[0] = width + 0.5; header.cupsPageSize[1] = length + 0.5; @@ -1629,7 +1629,7 @@ cfFilterImageToRaster(int inputfd, // I - File descriptor input stream "cfFilterImageToRaster: img->colorspace = %d", img->colorspace); } - row = malloc(2 * header.cupsBytesPerLine); + row = calloc(2, header.cupsBytesPerLine); ras = cupsRasterOpen(outputfd, CUPS_RASTER_WRITE); for (i = 0, page = 1; i < doc.Copies; i ++) diff --git a/cupsfilters/test-imagetoraster-overflow.sh b/cupsfilters/test-imagetoraster-overflow.sh new file mode 100755 index 000000000..427a014d0 --- /dev/null +++ b/cupsfilters/test-imagetoraster-overflow.sh @@ -0,0 +1,224 @@ +#!/usr/bin/env bash +# +# Regression test for the strcpy() buffer-overflow fix in imagetoraster.c. +# +# The vulnerable code copied header.cupsPageSizeName (64-byte field with no +# guaranteed null terminator when supplied via filter_data.header) into a +# 64-byte stack buffer with strcpy(), overflowing by an unbounded amount. +# The fix replaces strcpy() with snprintf(buf, sizeof(buf), ...). +# +# This script compiles a C harness with AddressSanitizer, runs it against a +# crafted cups_page_header_t whose cupsPageSizeName has no null terminator, +# and fails if ASAN reports any memory error. +# +# Pattern follows cupsfilters/test-pclm-overflow.sh. +# +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +BUILD_ROOT="$(cd "${ROOT}/.." && pwd)" +LIBTOOL="${BUILD_ROOT}/libtool" +CC="${CC:-cc}" +SAN_FLAGS="${SAN_FLAGS:--fsanitize=address -fno-omit-frame-pointer}" + +if [[ ! -x "${LIBTOOL}" ]]; then + echo "libtool helper not found at ${LIBTOOL}" >&2 + exit 99 +fi + +# Discover CUPS include path (may differ from /usr/include on some distros). +CUPS_INC="$(pkg-config --cflags-only-I libcups 2>/dev/null || \ + pkg-config --cflags-only-I cups 2>/dev/null || \ + cups-config --cflags 2>/dev/null || echo "")" + +TMP_PARENT="${TMPDIR:-/tmp}" +WORKDIR="$(mktemp -d "${TMP_PARENT%/}/imagetoraster-overflow.XXXXXX")" +cleanup() { rm -rf "${WORKDIR}"; } +trap cleanup EXIT + +MAKE_JPEG_SRC="${WORKDIR}/make_jpeg.c" +MAKE_JPEG_BIN="${WORKDIR}/make_jpeg" +INPUT_JPG="${WORKDIR}/test.jpg" +HARNESS_SRC="${WORKDIR}/trigger.c" +HARNESS_OBJ="${WORKDIR}/trigger.lo" +HARNESS_BIN="${WORKDIR}/trigger" +RUN_LOG="${WORKDIR}/trigger.log" + +cat > "${MAKE_JPEG_SRC}" <<'EOF_JPEG' +#include +#include +#include +#include + +int main(int argc, char **argv) { + if (argc != 2) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + + FILE *fp = fopen(argv[1], "wb"); + if (!fp) { perror("fopen"); return 1; } + + struct jpeg_compress_struct cinfo; + struct jpeg_error_mgr jerr; + cinfo.err = jpeg_std_error(&jerr); + jpeg_create_compress(&cinfo); + jpeg_stdio_dest(&cinfo, fp); + + cinfo.image_width = 8; + cinfo.image_height = 8; + cinfo.input_components = 1; + cinfo.in_color_space = JCS_GRAYSCALE; + jpeg_set_defaults(&cinfo); + jpeg_set_quality(&cinfo, 75, TRUE); + jpeg_start_compress(&cinfo, TRUE); + + JSAMPROW row[1]; + unsigned char rowdata[8]; + memset(rowdata, 255, 8); + row[0] = rowdata; + while (cinfo.next_scanline < cinfo.image_height) + jpeg_write_scanlines(&cinfo, row, 1); + + jpeg_finish_compress(&cinfo); + jpeg_destroy_compress(&cinfo); + fclose(fp); + return 0; +} +EOF_JPEG + +"${CC}" -std=c11 -O0 -o "${MAKE_JPEG_BIN}" "${MAKE_JPEG_SRC}" -ljpeg >/dev/null 2>&1 +if [[ ! -x "${MAKE_JPEG_BIN}" ]]; then + echo "Failed to compile make_jpeg" >&2 + exit 99 +fi +"${MAKE_JPEG_BIN}" "${INPUT_JPG}" + +cat > "${HARNESS_SRC}" <<'EOF' +/* + * Overflow-trigger harness for the imagetoraster strcpy() regression test. + * + * Injects a cups_page_header_t with cupsPageSizeName entirely filled with + * non-null bytes (no null terminator) via filter_data.header. The old + * strcpy(defSize, header.cupsPageSizeName) in imagetoraster.c reads past the + * end of the 64-byte field and overflows the 64-byte stack buffer defSize[]. + * AddressSanitizer will catch this. The snprintf() fix is immune. + * + * cupsPageSize is left at {0.0f, 0.0f} intentionally: cfRasterPrepareHeader() + * in raster.c only overwrites cupsPageSizeName through pwgMediaForSize() when + * cupsPageSize dimensions are positive. With {0,0} the crafted name survives + * all the way to the vulnerable copy. PageSize is set to Letter so the filter + * can compute page geometry. + */ +#include +#include +#include +#include +#include +#include +#include +#include + +int +main(int argc, char **argv) +{ + if (argc != 2) + { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + + signal(SIGPIPE, SIG_IGN); + + int inputfd = open(argv[1], O_RDONLY); + if (inputfd < 0) { perror("open input"); return 1; } + + int outputfd = open("/dev/null", O_WRONLY); + if (outputfd < 0) { perror("open /dev/null"); close(inputfd); return 1; } + + cups_page_header_t crafted; + memset(&crafted, 0, sizeof(crafted)); + crafted.PageSize[0] = 612; /* Letter: 8.5" × 11" in points */ + crafted.PageSize[1] = 792; + crafted.ImagingBoundingBox[0] = 0; + crafted.ImagingBoundingBox[1] = 0; + crafted.ImagingBoundingBox[2] = 612; + crafted.ImagingBoundingBox[3] = 792; + crafted.HWResolution[0] = 100; + crafted.HWResolution[1] = 100; + crafted.cupsWidth = 850; + crafted.cupsHeight = 1100; + crafted.cupsBitsPerColor = 8; + crafted.cupsBitsPerPixel = 8; + crafted.cupsNumColors = 1; + crafted.cupsBytesPerLine = 850; + crafted.cupsColorOrder = CUPS_ORDER_CHUNKED; + crafted.cupsColorSpace = CUPS_CSPACE_K; + /* 64 'A' bytes, no null terminator — triggers strcpy overflow */ + memset(crafted.cupsPageSizeName, 'A', sizeof(crafted.cupsPageSizeName)); + + int job_canceled = 0; + cf_filter_data_t data; + memset(&data, 0, sizeof(data)); + data.printer = "test-printer"; + data.job_id = 1; + data.job_user = "test"; + data.job_title = "imagetoraster strcpy overflow regression"; + data.copies = 1; + data.content_type = "image/jpeg"; + data.final_content_type = "application/vnd.cups-raster"; + data.header = &crafted; + data.printer_attrs = NULL; + data.logfunc = cfCUPSLogFunc; + data.iscanceledfunc = cfCUPSIsCanceledFunc; + data.iscanceleddata = &job_canceled; + data.back_pipe[0] = data.back_pipe[1] = -1; + data.side_pipe[0] = data.side_pipe[1] = -1; + + int ret = cfFilterImageToRaster(inputfd, outputfd, 1, &data, NULL); + close(inputfd); + close(outputfd); + return ret; +} +EOF + +"${LIBTOOL}" --mode=compile --tag=CC "${CC}" -std=c11 -O0 ${SAN_FLAGS} \ + -I"${BUILD_ROOT}" -I"${BUILD_ROOT}/cupsfilters" ${CUPS_INC} \ + -c "${HARNESS_SRC}" -o "${HARNESS_OBJ}" >/dev/null +if [[ ! -f "${HARNESS_OBJ}" && ! -f "${WORKDIR}/trigger.o" ]]; then + echo "test-imagetoraster-overflow: failed to compile harness" >&2 + exit 99 +fi + +"${LIBTOOL}" --mode=link --tag=CC "${CC}" ${SAN_FLAGS} "${HARNESS_OBJ}" \ + "${BUILD_ROOT}/libcupsfilters.la" -lcups -o "${HARNESS_BIN}" >/dev/null +if [[ ! -x "${HARNESS_BIN}" ]]; then + echo "test-imagetoraster-overflow: failed to link harness" >&2 + exit 99 +fi + +: > "${RUN_LOG}" +ASAN_OPTS="${ASAN_OPTIONS:-detect_leaks=0,abort_on_error=0}" + +set +e +"${LIBTOOL}" --mode=execute \ + env ASAN_OPTIONS="${ASAN_OPTS}" \ + "${HARNESS_BIN}" "${INPUT_JPG}" \ + >>"${RUN_LOG}" 2>&1 +STATUS=$? +set -e + +if [[ ${STATUS} -ne 0 ]]; then + cat "${RUN_LOG}" >&2 + echo "test-imagetoraster-overflow: harness exited with status ${STATUS}" >&2 + exit 1 +fi + +if grep -q "AddressSanitizer" "${RUN_LOG}"; then + cat "${RUN_LOG}" >&2 + echo "test-imagetoraster-overflow: AddressSanitizer reported a memory error" >&2 + exit 1 +fi + +echo "test-imagetoraster-overflow: PASSED" +exit 0 diff --git a/cupsfilters/testimagetoraster.c b/cupsfilters/testimagetoraster.c new file mode 100644 index 000000000..640fc5192 --- /dev/null +++ b/cupsfilters/testimagetoraster.c @@ -0,0 +1,168 @@ +// +// Unit test for cfFilterImageToRaster() — verifies the buffer-safe snprintf +// replacements introduced to fix strcpy() overflow risks in imagetoraster.c. +// +// Copyright 2024 by OpenPrinting. +// +// Licensed under Apache License v2.0. See the file "LICENSE" for more +// information. +// + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +// +// 'write_test_jpeg()' - Write a minimal 8×8 grayscale JPEG for testing. +// +// Generates a valid JPEG using libjpeg's compression API, ensuring proper +// Huffman tables and entropy coding. Returns 0 on success, -1 on failure. +// + +static int +write_test_jpeg(const char *path) +{ + FILE *fp = fopen(path, "wb"); + if (!fp) + return -1; + + struct jpeg_compress_struct cinfo; + struct jpeg_error_mgr jerr; + cinfo.err = jpeg_std_error(&jerr); + jpeg_create_compress(&cinfo); + jpeg_stdio_dest(&cinfo, fp); + + cinfo.image_width = 8; + cinfo.image_height = 8; + cinfo.input_components = 1; + cinfo.in_color_space = JCS_GRAYSCALE; + jpeg_set_defaults(&cinfo); + jpeg_set_quality(&cinfo, 75, TRUE); + jpeg_start_compress(&cinfo, TRUE); + + JSAMPROW row[1]; + unsigned char rowdata[8]; + memset(rowdata, 255, sizeof(rowdata)); + row[0] = rowdata; + while (cinfo.next_scanline < cinfo.image_height) + jpeg_write_scanlines(&cinfo, row, 1); + + jpeg_finish_compress(&cinfo); + jpeg_destroy_compress(&cinfo); + fclose(fp); + return 0; +} + +// +// 'main()' - Run cfFilterImageToRaster() end-to-end and verify it succeeds. +// +// The fixed code replaces strcpy(defSize, header.cupsPageSizeName) with +// snprintf(defSize, sizeof(defSize), ...) to guard against buffer overflows. +// Running the actual filter function exercises those code paths and, when +// built with -fsanitize=address, will catch any regression to the unsafe form. +// + +int // O - Exit status +main(void) +{ + int failed = 0; // Failure counter + int inputfd; // Input image file descriptor + int outputfd; // Output raster file descriptor + int job_canceled = 0; // Cancellation flag + cf_filter_data_t filter_data; // Filter job/printer data + + + signal(SIGPIPE, SIG_IGN); + + // + // Generate a test JPEG in /tmp. + // + + char tmpjpg[256]; + snprintf(tmpjpg, sizeof(tmpjpg), "/tmp/testimagetoraster_%d.jpg", getpid()); + + if (write_test_jpeg(tmpjpg) != 0) + { + fprintf(stderr, + "ERROR: testimagetoraster: Cannot write test JPEG to %s\n", + tmpjpg); + return (1); + } + + // + // Open the generated test JPEG as the filter input. + // + + inputfd = open(tmpjpg, O_RDONLY); + if (inputfd < 0) + { + fprintf(stderr, + "ERROR: testimagetoraster: Cannot open %s\n", tmpjpg); + unlink(tmpjpg); + return (1); + } + + // + // Discard the raster output — we are testing for correctness, not output. + // + + outputfd = open("/dev/null", O_WRONLY); + if (outputfd < 0) + { + fprintf(stderr, "ERROR: testimagetoraster: Cannot open /dev/null\n"); + close(inputfd); + return (1); + } + + // + // Build a minimal cf_filter_data_t. NULL printer_attrs causes the filter + // to fall back to built-in defaults, which is sufficient to reach and + // exercise the snprintf(defSize, ...) and calloc() code paths. + // + + memset(&filter_data, 0, sizeof(filter_data)); + filter_data.printer = "test-printer"; + filter_data.job_id = 1; + filter_data.job_user = "test"; + filter_data.job_title = "testimagetoraster buffer-safety check"; + filter_data.copies = 1; + filter_data.content_type = "image/jpeg"; + filter_data.final_content_type = "application/vnd.cups-raster"; + filter_data.logfunc = cfCUPSLogFunc; + filter_data.logdata = NULL; + filter_data.iscanceledfunc = cfCUPSIsCanceledFunc; + filter_data.iscanceleddata = &job_canceled; + filter_data.back_pipe[0] = -1; + filter_data.back_pipe[1] = -1; + filter_data.side_pipe[0] = -1; + filter_data.side_pipe[1] = -1; + + // + // Run the filter. A non-zero return value is a test failure. + // + + fprintf(stderr, "testimagetoraster: Testing cfFilterImageToRaster...\n"); + + if (cfFilterImageToRaster(inputfd, outputfd, 1, &filter_data, NULL) != 0) + { + fprintf(stderr, + "ERROR: testimagetoraster: cfFilterImageToRaster returned error\n"); + failed ++; + } + else + fprintf(stderr, "testimagetoraster: PASSED\n"); + + close(inputfd); + close(outputfd); + unlink(tmpjpg); + + return (failed ? 1 : 0); +}