From aaea4ef92400cb657a0b837b6932a7c390445980 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Wed, 12 Dec 2018 15:26:35 -0800 Subject: [PATCH] [libzstd] Fix infinite loop in decompression When we switched `ZSTD_SKIPPABLEHEADERSIZE` to a macro, the places where we do: MEM_readLE32(ptr) + ZSTD_SKIPPABLEHEADERSIZE can now overflow `(unsigned)-8` to `0` and we infinite loop. We now check the frame size and reject sizes that overflow a U32. Note that this bug never made it into a release, and was only in the dev branch for a few days. Credit to OSS-Fuzz --- lib/decompress/zstd_decompress.c | 33 +++++++++++++++++++++----------- tests/fuzzer.c | 8 ++++++++ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index d0b6063d1..510ce3c65 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -343,6 +343,21 @@ unsigned long long ZSTD_getFrameContentSize(const void *src, size_t srcSize) } } } +static size_t readSkippableFrameSize(void const* src, size_t srcSize) +{ + size_t const skippableHeaderSize = ZSTD_SKIPPABLEHEADERSIZE; + U32 sizeU32; + + if (srcSize < ZSTD_SKIPPABLEHEADERSIZE) + return ERROR(srcSize_wrong); + + sizeU32 = MEM_readLE32((BYTE const*)src + ZSTD_FRAMEIDSIZE); + if ((U32)(sizeU32 + ZSTD_SKIPPABLEHEADERSIZE) < sizeU32) + return ERROR(frameParameter_unsupported); + + return skippableHeaderSize + sizeU32; +} + /** ZSTD_findDecompressedSize() : * compatible with legacy mode * `srcSize` must be the exact length of some number of ZSTD compressed and/or @@ -356,11 +371,9 @@ unsigned long long ZSTD_findDecompressedSize(const void* src, size_t srcSize) U32 const magicNumber = MEM_readLE32(src); if ((magicNumber & ZSTD_MAGIC_SKIPPABLE_MASK) == ZSTD_MAGIC_SKIPPABLE_START) { - size_t skippableSize; - if (srcSize < ZSTD_SKIPPABLEHEADERSIZE) - return ERROR(srcSize_wrong); - skippableSize = MEM_readLE32((const BYTE *)src + ZSTD_FRAMEIDSIZE) - + ZSTD_SKIPPABLEHEADERSIZE; + size_t const skippableSize = readSkippableFrameSize(src, srcSize); + if (ZSTD_isError(skippableSize)) + return skippableSize; if (srcSize < skippableSize) { return ZSTD_CONTENTSIZE_ERROR; } @@ -436,7 +449,7 @@ size_t ZSTD_findFrameCompressedSize(const void *src, size_t srcSize) #endif if ( (srcSize >= ZSTD_SKIPPABLEHEADERSIZE) && (MEM_readLE32(src) & ZSTD_MAGIC_SKIPPABLE_MASK) == ZSTD_MAGIC_SKIPPABLE_START ) { - return ZSTD_SKIPPABLEHEADERSIZE + MEM_readLE32((const BYTE*)src + ZSTD_FRAMEIDSIZE); + return readSkippableFrameSize(src, srcSize); } else { const BYTE* ip = (const BYTE*)src; const BYTE* const ipstart = ip; @@ -660,11 +673,9 @@ static size_t ZSTD_decompressMultiFrame(ZSTD_DCtx* dctx, DEBUGLOG(4, "reading magic number %08X (expecting %08X)", (U32)magicNumber, (U32)ZSTD_MAGICNUMBER); if ((magicNumber & ZSTD_MAGIC_SKIPPABLE_MASK) == ZSTD_MAGIC_SKIPPABLE_START) { - size_t skippableSize; - if (srcSize < ZSTD_SKIPPABLEHEADERSIZE) - return ERROR(srcSize_wrong); - skippableSize = MEM_readLE32((const BYTE*)src + ZSTD_FRAMEIDSIZE) - + ZSTD_SKIPPABLEHEADERSIZE; + size_t const skippableSize = readSkippableFrameSize(src, srcSize); + if (ZSTD_isError(skippableSize)) + return skippableSize; if (srcSize < skippableSize) return ERROR(srcSize_wrong); src = (const BYTE *)src + skippableSize; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index b5872c680..ba7c0bc5a 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -353,6 +353,14 @@ static int basicUnitTests(U32 seed, double compressibility) ZSTD_freeCCtx(cctx); } + DISPLAYLEVEL(3, "test%3i : decompress skippable frame -8 size : ", testNb++); + { + char const skippable8[] = "\x50\x2a\x4d\x18\xf8\xff\xff\xff"; + size_t const size = ZSTD_decompress(NULL, 0, skippable8, 8); + if (!ZSTD_isError(size)) goto _output_error; + } + DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : ZSTD_getFrameContentSize test : ", testNb++); { unsigned long long const rSize = ZSTD_getFrameContentSize(compressedBuffer, cSize); -- 2.47.2