From ae3732f054a87602caa95cc42e7b8a91b13d770b Mon Sep 17 00:00:00 2001 From: Doron Gal Date: Wed, 4 Nov 2026 06:30:23 +0000 Subject: [PATCH] cover: reject unrepresentable dmer map sizes in COVER_map_init COVER_map_init() rounded the map capacity up to the next power of two in a U32 and sized the pair array with an unchecked multiply. For size = k-d+1 >= 2^30 (accepted by COVER_checkParameters(), which only bounds k by maxDictSize) the shift in `map->size = (U32)1 << map->sizeLog` is by 32 or more, which is undefined for a 32-bit type; measured SIGSEGV on x86-64 and a UBSan shift-exponent abort. On 32-bit targets mapSize * sizeof(COVER_map_pair_t) wraps to zero for mapSize >= 2^27, so the map under-allocates and the first COVER_map_insert() writes out of bounds (measured heap corruption, SIGABRT in free()). Reject sizes that cannot be represented (size == 0, sizeLog >= 32, or a pair-array byte size that wraps size_t) instead of shifting/multiplying into undefined behaviour, and surface the same bound in COVER_checkParameters() so out-of-range k/d fail as a parameter error. Add regression tests to tests/fuzzer.c: ordinary k/d must still train, and k = 2^30 must fail cleanly (run crash-isolated via fork() so an unfixed tree reports the failure instead of killing the test runner). --- lib/dictBuilder/cover.c | 45 +++++++++++++++++++++++++++------- tests/fuzzer.c | 53 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/lib/dictBuilder/cover.c b/lib/dictBuilder/cover.c index 06d1cb93a5c..d32f74a9bd8 100644 --- a/lib/dictBuilder/cover.c +++ b/lib/dictBuilder/cover.c @@ -154,15 +154,37 @@ static void COVER_map_clear(COVER_map_t *map) { * The map is only guaranteed to be large enough to hold size elements. */ static int COVER_map_init(COVER_map_t *map, U32 size) { - map->sizeLog = ZSTD_highbit32(size) + 2; - map->size = (U32)1 << map->sizeLog; - map->sizeMask = map->size - 1; - map->data = (COVER_map_pair_t *)malloc(map->size * sizeof(COVER_map_pair_t)); - if (!map->data) { - map->sizeLog = 0; - map->size = 0; - return 0; - } + U32 sizeLog; + U32 mapSize; + size_t allocSize; + + map->sizeLog = 0; + map->size = 0; + map->sizeMask = 0; + map->data = NULL; + + /* The capacity is size rounded up to the next power of two, stored in a + * U32. Reject sizes that cannot be represented instead of computing with + * undefined behaviour and under-allocating: + * - size == 0 would make ZSTD_highbit32() undefined; + * - size >= 2^30 gives sizeLog >= 32, i.e. a shift of 32 or more in + * `(U32)1 << sizeLog`, which is undefined for a 32-bit type; + * - the byte size of the pair array must not wrap size_t (it does for + * mapSize > SIZE_MAX / sizeof(COVER_map_pair_t) on 32-bit targets). + */ + if (size == 0) return 0; + sizeLog = ZSTD_highbit32(size) + 2; + if (sizeLog >= 32) return 0; + mapSize = (U32)1U << sizeLog; + allocSize = (size_t)mapSize * sizeof(COVER_map_pair_t); + if (allocSize / sizeof(COVER_map_pair_t) != (size_t)mapSize) return 0; + + map->data = (COVER_map_pair_t *)malloc(allocSize); + if (!map->data) return 0; + + map->sizeLog = sizeLog; + map->size = mapSize; + map->sizeMask = mapSize - 1; COVER_map_clear(map); return 1; } @@ -586,6 +608,11 @@ static int COVER_checkParameters(ZDICT_cover_params_t parameters, if (parameters.d > parameters.k) { return 0; } + /* k-d+1 is the number of dmers to track in COVER_map_init(); its capacity + * is rounded up to a power of two in a U32, so it must stay below 2^30 */ + if (parameters.k - parameters.d + 1 >= ((U32)1 << 30)) { + return 0; + } /* 0 < splitPoint <= 1 */ if (parameters.splitPoint <= 0 || parameters.splitPoint > 1){ return 0; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 32360b9a58d..8391f161499 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -26,6 +26,10 @@ #include /* fgets, sscanf */ #include /* strcmp */ #include /* time(), time_t */ +#ifndef _WIN32 +# include /* fork, _exit */ +# include /* waitpid */ +#endif #undef NDEBUG /* always enable assert() */ #include #define ZSTD_STATIC_LINKING_ONLY /* ZSTD_compressContinue, ZSTD_compressBlock */ @@ -3120,6 +3124,55 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK, created dictionary of size %u \n", (unsigned)dictSize); + DISPLAYLEVEL(3, "test%3i : COVER accepts ordinary k/d : ", testNb++); + { ZDICT_cover_params_t coverParams; + memset(&coverParams, 0, sizeof(coverParams)); + coverParams.k = 64; + coverParams.d = 8; + coverParams.steps = 1; + coverParams.splitPoint = 0.75; + dictSize = ZDICT_trainFromBuffer_cover(dictBuffer, dictBufferCapacity, + CNBuffer, samplesSizes, nbSamples, + coverParams); + if (ZDICT_isError(dictSize)) goto _output_error; + } + DISPLAYLEVEL(3, "OK, created dictionary of size %u \n", (unsigned)dictSize); + + DISPLAYLEVEL(3, "test%3i : COVER rejects unrepresentable k-d+1 : ", testNb++); + { /* k-d+1 == 2^30 rounds the dmer map capacity up to 2^32 : the + * old COVER_map_init() shifted a U32 by 32 (undefined behaviour, + * SIGSEGV on common builds) and on 32-bit wrapped the pair-array + * allocation to zero. The call must fail cleanly instead. */ + ZDICT_cover_params_t coverParams; + memset(&coverParams, 0, sizeof(coverParams)); + coverParams.k = (U32)1 << 30; + coverParams.d = 1; + coverParams.steps = 1; + coverParams.splitPoint = 0.75; +#ifdef _WIN32 + { size_t const badSize = ZDICT_trainFromBuffer_cover( + dictBuffer, (size_t)1 << 30, + CNBuffer, samplesSizes, nbSamples, coverParams); + if (!ZDICT_isError(badSize)) goto _output_error; + } +#else + { /* crash-isolated : an unfixed tree dies inside this call */ + pid_t const child = fork(); + assert(child >= 0); + if (child == 0) { + size_t const badSize = ZDICT_trainFromBuffer_cover( + dictBuffer, (size_t)1 << 30, + CNBuffer, samplesSizes, nbSamples, coverParams); + _exit(ZDICT_isError(badSize) ? 0 : 1); + } + { int status = 0; + if (waitpid(child, &status, 0) != child) goto _output_error; + if (!WIFEXITED(status) || (WEXITSTATUS(status) != 0)) goto _output_error; + } } +#endif + } + DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : check dictID : ", testNb++); dictID = ZDICT_getDictID(dictBuffer, dictSize); if (dictID==0) goto _output_error;