From d2bb5294b66b7e6850b7e6f9a88922b31453b1b0 Mon Sep 17 00:00:00 2001 From: gamedeveloper8136 Date: Tue, 11 Aug 2026 19:40:13 +0300 Subject: [PATCH 1/5] png_set_quantize: validate palette count parameters num_palette and maximum_colors are consumed as allocation sizes, loop bounds, array indices, and a memcpy length, but no range check exists before the PNG_QUANTIZE transform is enabled. The internal quantize_index array and the owned palette copy are sized for the PNG palette maximum (PNG_MAX_PALETTE_LENGTH), so an out-of-range count (num_palette <= 0, num_palette > PNG_MAX_PALETTE_LENGTH, or maximum_colors <= 0) can cause out-of-bounds accesses and oversized copies. Reject such calls with a warning before the transform is enabled, matching the validation already used by png_set_PLTE, png_set_hIST, and png_set_sCAL. Valid calls are unaffected. Add a pnggetset regression test covering the out-of-range boundary values and a valid call. --- contrib/libtests/pnggetset.c | 112 +++++++++++++++++++++++++++++++++++ pngrtran.c | 14 +++++ 2 files changed, 126 insertions(+) diff --git a/contrib/libtests/pnggetset.c b/contrib/libtests/pnggetset.c index e2c1ca556f..9f0e5250e7 100644 --- a/contrib/libtests/pnggetset.c +++ b/contrib/libtests/pnggetset.c @@ -773,6 +773,106 @@ test_plte_palette_sync(void) return 0; } +/* Test: out-of-range palette counts passed to png_set_quantize must not + * cause an out-of-bounds access into the internally sized palette buffers. + * This reproduces the heap buffer overflow that occurred when num_palette + * exceeded PNG_MAX_PALETTE_LENGTH or was non-positive, or when + * maximum_colors was non-positive. A valid call must be accepted. + */ +#ifdef PNG_READ_QUANTIZE_SUPPORTED +static int quantize_warning_count; + +static void PNGAPI +count_quantize_warning(png_structp png_ptr, png_const_charp warning_message) +{ + (void)png_ptr; + (void)warning_message; + quantize_warning_count++; +} + +static int +test_quantize_palette_length_case(const char *what, int num_palette, + int maximum_colors, png_error_ptr warning_fn) +{ + png_structp png_ptr; + png_infop info_ptr; + png_color palette[PNG_MAX_PALETTE_LENGTH + 1]; + int i; + + /* Initialize one entry beyond the PNG maximum so that an over-sized + * num_palette copy never reads past the test's own buffer. + */ + for (i = 0; i < (int)PNG_MAX_PALETTE_LENGTH + 1; i++) + { + palette[i].red = (png_byte)i; + palette[i].green = (png_byte)(i >> 1); + palette[i].blue = (png_byte)(i >> 2); + } + + png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL); + if (png_ptr == NULL) + { + fprintf(stderr, "pnggetset: png_create_read_struct failed\n"); + return 1; + } + info_ptr = png_create_info_struct(png_ptr); + if (info_ptr == NULL) + { + fprintf(stderr, "pnggetset: png_create_info_struct failed\n"); + png_destroy_read_struct(&png_ptr, NULL, NULL); + return 1; + } + if (setjmp(png_jmpbuf(png_ptr))) + { + fprintf(stderr, "pnggetset: libpng error in %s\n", what); + png_destroy_read_struct(&png_ptr, &info_ptr, NULL); + return 1; + } + if (warning_fn != NULL) + png_set_error_fn(png_ptr, NULL, NULL, warning_fn); + png_set_quantize(png_ptr, palette, num_palette, maximum_colors, NULL, 0); + png_destroy_read_struct(&png_ptr, &info_ptr, NULL); + + return 0; +} + +static int +test_quantize_palette_length(void) +{ + if (test_quantize_palette_length_case("oversized palette", + (int)PNG_MAX_PALETTE_LENGTH + 1, (int)PNG_MAX_PALETTE_LENGTH + 1, + NULL) != 0) + return 1; + + if (test_quantize_palette_length_case("negative palette", + -1, (int)PNG_MAX_PALETTE_LENGTH, NULL) != 0) + return 1; + + if (test_quantize_palette_length_case("negative maximum colors", + (int)PNG_MAX_PALETTE_LENGTH, -1, NULL) != 0) + return 1; + + if (test_quantize_palette_length_case("zero palette", + 0, (int)PNG_MAX_PALETTE_LENGTH, NULL) != 0) + return 1; + + /* A valid call must be accepted without any warning. */ + quantize_warning_count = 0; + if (test_quantize_palette_length_case("valid palette", 4, 4, + count_quantize_warning) != 0) + return 1; + if (quantize_warning_count != 0) + { + fprintf(stderr, + "pnggetset: valid png_set_quantize call generated %d warning(s)\n", + quantize_warning_count); + return 1; + } + + return 0; +} +#endif + int main(void) { @@ -858,5 +958,17 @@ main(void) else printf("PASS\n"); +#ifdef PNG_READ_QUANTIZE_SUPPORTED + printf("Testing quantize palette length validation... "); + fflush(stdout); + if (test_quantize_palette_length() != 0) + { + printf("FAIL\n"); + result = 1; + } + else + printf("PASS\n"); +#endif + return result; } diff --git a/pngrtran.c b/pngrtran.c index 38a9fb9aa6..d7d4c3815d 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -479,6 +479,20 @@ png_set_quantize(png_struct *png_ptr, png_color *palette, if (palette == NULL) return; + /* Validate the palette length. A PNG palette can hold at most + * PNG_MAX_PALETTE_LENGTH entries, and the internal arrays and the owned + * copy of the palette are sized accordingly. Reject out-of-range counts + * instead of silently truncating them, so that the copies and index arrays + * below cannot be accessed out of bounds (png_set_PLTE and png_set_hIST + * perform a similar check on their palette counts). + */ + if (num_palette <= 0 || num_palette > (int)PNG_MAX_PALETTE_LENGTH || + maximum_colors <= 0) + { + png_warning(png_ptr, "Ignoring invalid palette length in png_set_quantize"); + return; + } + png_ptr->transformations |= PNG_QUANTIZE; if (full_quantize == 0) From 2ec28bd70ada6d1502feb2db6b46d81a533e707f Mon Sep 17 00:00:00 2001 From: gamedeveloper8136 Date: Fri, 14 Aug 2026 19:13:53 +0300 Subject: [PATCH 2/5] Add standalone reproduction for png_set_quantize validation Provide a self-contained program (repro_quantize.c) that calls png_set_quantize() with out-of-range palette counts and a guard-page malloc interposer (guard_malloc.c) that makes the resulting out-of-bounds accesses fault deterministically without ASan. --- guard_malloc.c | 71 +++++++++++++++++++++++++++++++++++++++++++ repro_quantize.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 guard_malloc.c create mode 100644 repro_quantize.c diff --git a/guard_malloc.c b/guard_malloc.c new file mode 100644 index 0000000000..4da8310ff7 --- /dev/null +++ b/guard_malloc.c @@ -0,0 +1,71 @@ +/* LD_PRELOAD malloc interposer: every allocation is placed at the end of a + * page whose following page is PROT_NONE, so ANY write past the requested + * size segfaults. Detects heap OOB writes deterministically without ASan. + */ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include + +typedef struct { void *user; void *base; size_t size; int guard; } slot; +static slot slots[65536]; +static int nslots = 0; + +static void *guarded_malloc(size_t size) +{ + long page = sysconf(_SC_PAGESIZE); + if (size == 0) size = 1; + size_t chunk = (size + page - 1) / page * page; + unsigned char *base = mmap(NULL, chunk + page, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (base == MAP_FAILED) return NULL; + if (mprotect(base + chunk, page, PROT_NONE) != 0) { munmap(base, chunk + page); return NULL; } + void *user = base + chunk - size; + if (nslots < 65536) { slots[nslots].user = user; slots[nslots].base = base; slots[nslots].size = size; nslots++; } + return user; +} + +void *malloc(size_t size) { return guarded_malloc(size); } + +void *calloc(size_t n, size_t size) +{ + size_t total = n * size; + void *p = guarded_malloc(total); + if (p) memset(p, 0, total); + return p; +} + +void *realloc(void *ptr, size_t size) +{ + void *np = guarded_malloc(size); + if (np && ptr) + { + int i; + size_t old = 0; + for (i = 0; i < nslots; i++) if (slots[i].user == ptr) { old = slots[i].size; break; } + if (old > size) old = size; + memcpy(np, ptr, old); + } + free(ptr); + return np; +} + +void free(void *ptr) +{ + if (ptr == NULL) return; + int i; + for (i = 0; i < nslots; i++) + { + if (slots[i].user == ptr) + { + munmap(slots[i].base, ((slots[i].size + sysconf(_SC_PAGESIZE) - 1) / sysconf(_SC_PAGESIZE)) * sysconf(_SC_PAGESIZE) + sysconf(_SC_PAGESIZE)); + slots[i].user = NULL; + return; + } + } + /* not tracked; fall back to a plain free */ + /* nothing to do - unmapped unknowns are leaks in this interposer */ +} diff --git a/repro_quantize.c b/repro_quantize.c new file mode 100644 index 0000000000..c91d61b35c --- /dev/null +++ b/repro_quantize.c @@ -0,0 +1,78 @@ +/* + * repro_quantize.c — deterministic reproduction of the png_set_quantize() + * out-of-bounds access on unpatched libpng. + * + * Usage: + * ./repro_quantize [histogram 0|1] + * + * Cases that crash on unpatched libpng (SIGSEGV): + * num_palette > 256 (e.g. 257 257) -> owned-palette memcpy overflows + * its 768-byte buffer + * num_palette < 0 (e.g. -1 256) -> memcpy of ~12.9 GB + * maximum_colors <= 0 (e.g. 256 -1) -> num_palette becomes negative, + * then the same oversized memcpy + * + * Boundary cases that are SAFE on both patched and unpatched libpng: + * 1 256, 256 256, 256 255 + * + * On unpatched libpng the -1 and >256 cases fault immediately (the huge + * memcpy reads past a small buffer). With the guard-page malloc interposer + * (guard_malloc.c) every out-of-bounds case faults deterministically. + * + * Note: no PNG file is required; png_set_quantize() is called on a fresh + * read struct. + */ +#include "png.h" +#include +#include + +int main(int argc, char **argv) +{ + int num_palette = (argc > 1) ? atoi(argv[1]) : 257; + int maximum_colors = (argc > 2) ? atoi(argv[2]) : 257; + int use_hist = (argc > 3) ? atoi(argv[3]) : 0; + png_structp png_ptr; + png_infop info_ptr; + png_color *palette; + png_uint_16 *hist = NULL; + int i, n; + + n = (num_palette < 0) ? 4 : (num_palette > 0 ? num_palette : 1); + palette = (png_color *)malloc((size_t)n * sizeof(png_color)); + if (palette == NULL) return 2; + for (i = 0; i < n; i++) + { + palette[i].red = (png_byte)i; + palette[i].green = (png_byte)(i >> 1); + palette[i].blue = (png_byte)(i >> 2); + } + if (use_hist) + { + hist = (png_uint_16 *)malloc((size_t)n * sizeof(png_uint_16)); + if (hist == NULL) return 2; + for (i = 0; i < n; i++) + hist[i] = (png_uint_16)(n - i); + } + + png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL); + if (png_ptr == NULL) return 2; + info_ptr = png_create_info_struct(png_ptr); + if (info_ptr == NULL) { png_destroy_read_struct(&png_ptr, NULL, NULL); return 2; } + + if (setjmp(png_jmpbuf(png_ptr))) + { + png_destroy_read_struct(&png_ptr, &info_ptr, NULL); + fprintf(stderr, "libpng raised an error (png_error/longjmp)\n"); + return 1; + } + + fprintf(stderr, "png_set_quantize(num_palette=%d, maximum_colors=%d, hist=%d)\n", + num_palette, maximum_colors, use_hist); + png_set_quantize(png_ptr, palette, num_palette, maximum_colors, hist, 0); + + png_destroy_read_struct(&png_ptr, &info_ptr, NULL); + free(palette); + free(hist); + fprintf(stderr, "OK: call completed without fault\n"); + return 0; +} From 62048f7fcad44d65b989e885b3ac7c1657cbda28 Mon Sep 17 00:00:00 2001 From: gamedeveloper8136 Date: Tue, 18 Aug 2026 04:18:42 +0300 Subject: [PATCH 3/5] png_set_quantize: address review feedback A palette that is larger than maximum_colors is valid and is reduced to fit, so do not reject num_palette > PNG_MAX_PALETTE_LENGTH. Instead validate maximum_colors (positive and no more than the PNG palette maximum, which bounds the 8-bit quantized output) and reject non-positive counts. Use png_app_error for invalid counts so the application does not silently assume the palette mapping was set up. Size quantize_index for the input palette as well as the output: the pixel loop reads indices 0..255, but the reduction code indexes it with input palette entries, so with num_palette > PNG_MAX_PALETTE_LENGTH the previous fixed-size array could be accessed out of bounds. Rework the regression test to treat an oversized input palette as valid (reduction) and to require an application error for invalid counts. --- contrib/libtests/pnggetset.c | 98 +++++++++++++++++++----------------- pngrtran.c | 35 +++++++------ 2 files changed, 73 insertions(+), 60 deletions(-) diff --git a/contrib/libtests/pnggetset.c b/contrib/libtests/pnggetset.c index 9f0e5250e7..63a6555879 100644 --- a/contrib/libtests/pnggetset.c +++ b/contrib/libtests/pnggetset.c @@ -773,36 +773,26 @@ test_plte_palette_sync(void) return 0; } -/* Test: out-of-range palette counts passed to png_set_quantize must not - * cause an out-of-bounds access into the internally sized palette buffers. - * This reproduces the heap buffer overflow that occurred when num_palette - * exceeded PNG_MAX_PALETTE_LENGTH or was non-positive, or when - * maximum_colors was non-positive. A valid call must be accepted. +/* Test: png_set_quantize must accept a palette that is larger than + * maximum_colors (it is reduced to fit), must reject invalid counts with an + * application error, and must not access its internally sized buffers out of + * bounds. */ #ifdef PNG_READ_QUANTIZE_SUPPORTED -static int quantize_warning_count; - -static void PNGAPI -count_quantize_warning(png_structp png_ptr, png_const_charp warning_message) -{ - (void)png_ptr; - (void)warning_message; - quantize_warning_count++; -} - static int test_quantize_palette_length_case(const char *what, int num_palette, - int maximum_colors, png_error_ptr warning_fn) + int maximum_colors, const png_uint_16 *histogram, int expect_error) { png_structp png_ptr; png_infop info_ptr; png_color palette[PNG_MAX_PALETTE_LENGTH + 1]; int i; + volatile int error_seen = 0; - /* Initialize one entry beyond the PNG maximum so that an over-sized - * num_palette copy never reads past the test's own buffer. + /* Initialize one entry beyond the PNG maximum so that a larger-than-max + * palette never reads past the test's own buffer. */ - for (i = 0; i < (int)PNG_MAX_PALETTE_LENGTH + 1; i++) + for (i = 0; i < PNG_MAX_PALETTE_LENGTH + 1; i++) { palette[i].red = (png_byte)i; palette[i].green = (png_byte)(i >> 1); @@ -824,14 +814,23 @@ test_quantize_palette_length_case(const char *what, int num_palette, } if (setjmp(png_jmpbuf(png_ptr))) { - fprintf(stderr, "pnggetset: libpng error in %s\n", what); + error_seen = 1; png_destroy_read_struct(&png_ptr, &info_ptr, NULL); + } + else + { + png_set_quantize(png_ptr, palette, num_palette, maximum_colors, + histogram, 0); + png_destroy_read_struct(&png_ptr, &info_ptr, NULL); + } + + if (error_seen != expect_error) + { + fprintf(stderr, "pnggetset: %s: expected %s, got %s\n", what, + expect_error ? "an error" : "no error", + error_seen ? "an error" : "no error"); return 1; } - if (warning_fn != NULL) - png_set_error_fn(png_ptr, NULL, NULL, warning_fn); - png_set_quantize(png_ptr, palette, num_palette, maximum_colors, NULL, 0); - png_destroy_read_struct(&png_ptr, &info_ptr, NULL); return 0; } @@ -839,35 +838,44 @@ test_quantize_palette_length_case(const char *what, int num_palette, static int test_quantize_palette_length(void) { - if (test_quantize_palette_length_case("oversized palette", - (int)PNG_MAX_PALETTE_LENGTH + 1, (int)PNG_MAX_PALETTE_LENGTH + 1, - NULL) != 0) - return 1; + png_uint_16 histogram[PNG_MAX_PALETTE_LENGTH + 1]; + int i; - if (test_quantize_palette_length_case("negative palette", - -1, (int)PNG_MAX_PALETTE_LENGTH, NULL) != 0) - return 1; + for (i = 0; i < PNG_MAX_PALETTE_LENGTH + 1; i++) + histogram[i] = (png_uint_16)(PNG_MAX_PALETTE_LENGTH + 1 - i); - if (test_quantize_palette_length_case("negative maximum colors", - (int)PNG_MAX_PALETTE_LENGTH, -1, NULL) != 0) + /* Valid calls, including a palette larger than maximum_colors, which is + * reduced to fit. */ + if (test_quantize_palette_length_case("valid palette", 4, 4, NULL, 0) != 0) + return 1; + if (test_quantize_palette_length_case("boundary palette", + PNG_MAX_PALETTE_LENGTH, PNG_MAX_PALETTE_LENGTH, NULL, 0) != 0) + return 1; + if (test_quantize_palette_length_case("oversized input palette", + PNG_MAX_PALETTE_LENGTH + 1, PNG_MAX_PALETTE_LENGTH, + NULL, 0) != 0) + return 1; + if (test_quantize_palette_length_case("oversized input palette, histogram", + PNG_MAX_PALETTE_LENGTH + 1, PNG_MAX_PALETTE_LENGTH, + histogram, 0) != 0) return 1; + /* Invalid calls must raise an application error. */ if (test_quantize_palette_length_case("zero palette", - 0, (int)PNG_MAX_PALETTE_LENGTH, NULL) != 0) + 0, PNG_MAX_PALETTE_LENGTH, NULL, 1) != 0) return 1; - - /* A valid call must be accepted without any warning. */ - quantize_warning_count = 0; - if (test_quantize_palette_length_case("valid palette", 4, 4, - count_quantize_warning) != 0) + if (test_quantize_palette_length_case("negative palette", + -1, PNG_MAX_PALETTE_LENGTH, NULL, 1) != 0) return 1; - if (quantize_warning_count != 0) - { - fprintf(stderr, - "pnggetset: valid png_set_quantize call generated %d warning(s)\n", - quantize_warning_count); + if (test_quantize_palette_length_case("negative maximum colors", + PNG_MAX_PALETTE_LENGTH, -1, NULL, 1) != 0) + return 1; + if (test_quantize_palette_length_case("zero maximum colors", + 4, 0, NULL, 1) != 0) + return 1; + if (test_quantize_palette_length_case("oversized maximum colors", + 4, PNG_MAX_PALETTE_LENGTH + 1, NULL, 1) != 0) return 1; - } return 0; } diff --git a/pngrtran.c b/pngrtran.c index d7d4c3815d..7f4a7878cf 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -479,17 +479,18 @@ png_set_quantize(png_struct *png_ptr, png_color *palette, if (palette == NULL) return; - /* Validate the palette length. A PNG palette can hold at most - * PNG_MAX_PALETTE_LENGTH entries, and the internal arrays and the owned - * copy of the palette are sized accordingly. Reject out-of-range counts - * instead of silently truncating them, so that the copies and index arrays - * below cannot be accessed out of bounds (png_set_PLTE and png_set_hIST - * perform a similar check on their palette counts). + /* Validate the palette counts. A palette larger than maximum_colors is + * valid and is reduced below, but num_palette must be positive, + * maximum_colors must be positive, and the reduced palette cannot exceed + * PNG_MAX_PALETTE_LENGTH entries, because the quantized output is an 8-bit + * palette. Reject invalid values as an application error instead of + * silently skipping the transform, so that the application does not + * assume the palette mapping has been set up. */ - if (num_palette <= 0 || num_palette > (int)PNG_MAX_PALETTE_LENGTH || - maximum_colors <= 0) + if (num_palette <= 0 || maximum_colors <= 0 || + maximum_colors > PNG_MAX_PALETTE_LENGTH) { - png_warning(png_ptr, "Ignoring invalid palette length in png_set_quantize"); + png_app_error(png_ptr, "Invalid palette length in png_set_quantize"); return; } @@ -498,21 +499,25 @@ png_set_quantize(png_struct *png_ptr, png_color *palette, if (full_quantize == 0) { int i; + png_alloc_size_t index_size; /* Initialize the array to index colors. * - * Ensure quantize_index can fit 256 elements (PNG_MAX_PALETTE_LENGTH) - * rather than num_palette elements. This is to prevent buffer overflows - * caused by malformed PNG files with out-of-range palette indices. + * The pixel loop reads indices 0..255, so at least + * PNG_MAX_PALETTE_LENGTH elements are needed, but the reduction code + * below also indexes quantize_index with input palette entries, so + * size it for the larger of the two. * * Be careful to avoid leaking memory. Applications are allowed to call * this function more than once per png_struct. */ + index_size = PNG_MAX_PALETTE_LENGTH; + if (num_palette > PNG_MAX_PALETTE_LENGTH) + index_size = (png_alloc_size_t)num_palette; png_free(png_ptr, png_ptr->quantize_index); png_ptr->quantize_index = NULL; - png_ptr->quantize_index = (png_byte *)png_malloc(png_ptr, - PNG_MAX_PALETTE_LENGTH); - for (i = 0; i < PNG_MAX_PALETTE_LENGTH; i++) + png_ptr->quantize_index = (png_byte *)png_malloc(png_ptr, index_size); + for (i = 0; i < (int)index_size; i++) png_ptr->quantize_index[i] = (png_byte)i; } From 3c9ca0af31c5fcc6ee7b783f9e95487ef69814a7 Mon Sep 17 00:00:00 2001 From: gamedeveloper8136 Date: Tue, 18 Aug 2026 15:43:55 +0300 Subject: [PATCH 4/5] png_set_quantize: clarify quantize_index sizing rationale Explain that quantize_index must cover both the output palette indices 0..255, read by the pixel loop in png_do_quantize (which can exceed num_palette for a malformed palette index), and the input palette indices read by the reduction code, so it is sized for the larger of num_palette and PNG_MAX_PALETTE_LENGTH. --- pngrtran.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pngrtran.c b/pngrtran.c index 7f4a7878cf..c9e05b9a99 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -503,10 +503,13 @@ png_set_quantize(png_struct *png_ptr, png_color *palette, /* Initialize the array to index colors. * - * The pixel loop reads indices 0..255, so at least - * PNG_MAX_PALETTE_LENGTH elements are needed, but the reduction code - * below also indexes quantize_index with input palette entries, so - * size it for the larger of the two. + * quantize_index serves two readers: the pixel loop in + * png_do_quantize reads indices 0..255 (an 8-bit palette index from + * the image data, which can exceed num_palette in a malformed file), + * so the array must hold at least PNG_MAX_PALETTE_LENGTH entries; + * and the reduction code below indexes it with input palette entries, + * so it must also hold at least num_palette entries. Size it for the + * larger of the two. * * Be careful to avoid leaking memory. Applications are allowed to call * this function more than once per png_struct. From cb6a828ee9832d2eed374c4df7bbb3266fbaca90 Mon Sep 17 00:00:00 2001 From: gamedeveloper8136 Date: Fri, 28 Aug 2026 22:36:26 +0530 Subject: [PATCH 5/5] png_set_quantize: enforce PNG_MAX_PALETTE_LENGTH upper bound on num_palette Ensure both num_palette and maximum_colors are strictly bounded in [1, PNG_MAX_PALETTE_LENGTH] (256) as required by the PNG specification and libpng API contract. Revert quantize_index to fixed allocation size of PNG_MAX_PALETTE_LENGTH and update test cases accordingly. --- contrib/libtests/pnggetset.c | 17 +++++++++-------- pngrtran.c | 34 +++++++++++++--------------------- 2 files changed, 22 insertions(+), 29 deletions(-) diff --git a/contrib/libtests/pnggetset.c b/contrib/libtests/pnggetset.c index 63a6555879..2578367136 100644 --- a/contrib/libtests/pnggetset.c +++ b/contrib/libtests/pnggetset.c @@ -844,20 +844,18 @@ test_quantize_palette_length(void) for (i = 0; i < PNG_MAX_PALETTE_LENGTH + 1; i++) histogram[i] = (png_uint_16)(PNG_MAX_PALETTE_LENGTH + 1 - i); - /* Valid calls, including a palette larger than maximum_colors, which is - * reduced to fit. */ + /* Valid calls, including a palette larger than maximum_colors (reduced to fit), + * provided both counts are within PNG_MAX_PALETTE_LENGTH. */ if (test_quantize_palette_length_case("valid palette", 4, 4, NULL, 0) != 0) return 1; if (test_quantize_palette_length_case("boundary palette", PNG_MAX_PALETTE_LENGTH, PNG_MAX_PALETTE_LENGTH, NULL, 0) != 0) return 1; - if (test_quantize_palette_length_case("oversized input palette", - PNG_MAX_PALETTE_LENGTH + 1, PNG_MAX_PALETTE_LENGTH, - NULL, 0) != 0) + if (test_quantize_palette_length_case("valid reduction palette", + PNG_MAX_PALETTE_LENGTH, 128, NULL, 0) != 0) return 1; - if (test_quantize_palette_length_case("oversized input palette, histogram", - PNG_MAX_PALETTE_LENGTH + 1, PNG_MAX_PALETTE_LENGTH, - histogram, 0) != 0) + if (test_quantize_palette_length_case("valid reduction palette, histogram", + PNG_MAX_PALETTE_LENGTH, 128, histogram, 0) != 0) return 1; /* Invalid calls must raise an application error. */ @@ -867,6 +865,9 @@ test_quantize_palette_length(void) if (test_quantize_palette_length_case("negative palette", -1, PNG_MAX_PALETTE_LENGTH, NULL, 1) != 0) return 1; + if (test_quantize_palette_length_case("oversized input palette", + PNG_MAX_PALETTE_LENGTH + 1, PNG_MAX_PALETTE_LENGTH, NULL, 1) != 0) + return 1; if (test_quantize_palette_length_case("negative maximum colors", PNG_MAX_PALETTE_LENGTH, -1, NULL, 1) != 0) return 1; diff --git a/pngrtran.c b/pngrtran.c index c9e05b9a99..9962ce9627 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -480,15 +480,15 @@ png_set_quantize(png_struct *png_ptr, png_color *palette, return; /* Validate the palette counts. A palette larger than maximum_colors is - * valid and is reduced below, but num_palette must be positive, - * maximum_colors must be positive, and the reduced palette cannot exceed - * PNG_MAX_PALETTE_LENGTH entries, because the quantized output is an 8-bit - * palette. Reject invalid values as an application error instead of - * silently skipping the transform, so that the application does not - * assume the palette mapping has been set up. + * valid and is reduced below, but num_palette and maximum_colors must be + * positive and cannot exceed PNG_MAX_PALETTE_LENGTH entries, because PNG + * palettes and 8-bit quantized output indices are bounded at 256. + * Reject invalid values as an application error instead of silently + * skipping the transform, so that the application does not assume the + * palette mapping has been set up. */ - if (num_palette <= 0 || maximum_colors <= 0 || - maximum_colors > PNG_MAX_PALETTE_LENGTH) + if (num_palette <= 0 || num_palette > PNG_MAX_PALETTE_LENGTH || + maximum_colors <= 0 || maximum_colors > PNG_MAX_PALETTE_LENGTH) { png_app_error(png_ptr, "Invalid palette length in png_set_quantize"); return; @@ -499,28 +499,20 @@ png_set_quantize(png_struct *png_ptr, png_color *palette, if (full_quantize == 0) { int i; - png_alloc_size_t index_size; /* Initialize the array to index colors. * - * quantize_index serves two readers: the pixel loop in - * png_do_quantize reads indices 0..255 (an 8-bit palette index from - * the image data, which can exceed num_palette in a malformed file), - * so the array must hold at least PNG_MAX_PALETTE_LENGTH entries; - * and the reduction code below indexes it with input palette entries, - * so it must also hold at least num_palette entries. Size it for the - * larger of the two. + * quantize_index holds PNG_MAX_PALETTE_LENGTH (256) entries to cover + * all 8-bit palette indices (0..255). * * Be careful to avoid leaking memory. Applications are allowed to call * this function more than once per png_struct. */ - index_size = PNG_MAX_PALETTE_LENGTH; - if (num_palette > PNG_MAX_PALETTE_LENGTH) - index_size = (png_alloc_size_t)num_palette; png_free(png_ptr, png_ptr->quantize_index); png_ptr->quantize_index = NULL; - png_ptr->quantize_index = (png_byte *)png_malloc(png_ptr, index_size); - for (i = 0; i < (int)index_size; i++) + png_ptr->quantize_index = (png_byte *)png_malloc(png_ptr, + PNG_MAX_PALETTE_LENGTH); + for (i = 0; i < PNG_MAX_PALETTE_LENGTH; i++) png_ptr->quantize_index[i] = (png_byte)i; }