From 9889bca530a2b52615eb1cd06260e9cd0c29e001 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Fri, 27 Jul 2018 17:30:03 -0700 Subject: [PATCH 1/3] [FSE] Fix division by zero When the primary normalization method fails, and `(1 << tableLog) == (maxSymbolValue + 1)`, and every symbol gets assigned normalized weight 1 or -1 in the first loop, then the next division can raise `SIGFPE`. --- lib/compress/fse_compress.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/compress/fse_compress.c b/lib/compress/fse_compress.c index 07b3ab89b..95e7c1c7e 100644 --- a/lib/compress/fse_compress.c +++ b/lib/compress/fse_compress.c @@ -394,6 +394,9 @@ static size_t FSE_normalizeM2(short* norm, U32 tableLog, const unsigned* count, } ToDistribute = (1 << tableLog) - distributed; + if (ToDistribute == 0) + return 0; + if ((total / ToDistribute) > lowOne) { /* risk of rounding to zero */ lowOne = (U32)((total * 3) / (ToDistribute * 2)); From b9faaa1dc3b32b0e6ed248e860db54b4052f0aab Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Mon, 30 Jul 2018 12:57:11 -0700 Subject: [PATCH 2/3] [FSE] Add division by zero test --- tests/fuzzer.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index f7bacd5ca..8856a504a 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -27,6 +27,7 @@ #include /* strcmp */ #include #define ZSTD_STATIC_LINKING_ONLY /* ZSTD_compressContinue, ZSTD_compressBlock */ +#include "fse.h" #include "zstd.h" /* ZSTD_VERSION_STRING */ #include "zstd_errors.h" /* ZSTD_getErrorCode */ #include "zstdmt_compress.h" @@ -1423,6 +1424,24 @@ static int basicUnitTests(U32 seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : testing FSE_normalizeCount() PR#1255: ", testNb++); + { + short norm[32]; + unsigned count[32]; + unsigned const tableLog = 5; + size_t const nbSeq = 32; + unsigned const maxSymbolValue = 31; + size_t i; + + for (i = 0; i < 32; ++i) + count[i] = 1; + /* Calling FSE_normalizeCount() on a uniform distribution should not + * cause a division by zero. + */ + FSE_normalizeCount(norm, tableLog, count, nbSeq, maxSymbolValue); + } + DISPLAYLEVEL(3, "OK \n"); + _end: free(CNBuffer); free(compressedBuffer); From dc5a67cb7b84a86bb3729ae4a85ef61e37797ac2 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Thu, 2 Aug 2018 11:12:17 -0700 Subject: [PATCH 3/3] Disallow tableLog == srcLog --- lib/compress/fse_compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/fse_compress.c b/lib/compress/fse_compress.c index 95e7c1c7e..70daae3bc 100644 --- a/lib/compress/fse_compress.c +++ b/lib/compress/fse_compress.c @@ -331,7 +331,7 @@ void FSE_freeCTable (FSE_CTable* ct) { free(ct); } /* provides the minimum logSize to safely represent a distribution */ static unsigned FSE_minTableLog(size_t srcSize, unsigned maxSymbolValue) { - U32 minBitsSrc = BIT_highbit32((U32)(srcSize - 1)) + 1; + U32 minBitsSrc = BIT_highbit32((U32)(srcSize)) + 1; U32 minBitsSymbols = BIT_highbit32(maxSymbolValue) + 2; U32 minBits = minBitsSrc < minBitsSymbols ? minBitsSrc : minBitsSymbols; assert(srcSize > 1); /* Not supported, RLE should be used instead */