From 806a5c84e4488d3eac54bc1971e07e3e39427783 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 24 Oct 2018 16:34:35 -0700 Subject: [PATCH] support decompressing an empty frame into NULL fix #1385 decompressing into NULL was an automatic error. It is now allowed, as long as the content of the frame is empty. Seems to simplify things for `arrow`. Maybe some other projects rely on this behavior ? --- lib/decompress/zstd_decompress.c | 49 ++++++++++++++++++-------------- tests/fuzzer.c | 46 +++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 22 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 54b9bf70d..7f721e20b 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -404,9 +404,9 @@ unsigned long long ZSTD_findDecompressedSize(const void* src, size_t srcSize) } /** ZSTD_getDecompressedSize() : -* compatible with legacy mode -* @return : decompressed size if known, 0 otherwise - note : 0 can mean any of the following : + * compatible with legacy mode + * @return : decompressed size if known, 0 otherwise + note : 0 can mean any of the following : - frame content is empty - decompressed size field is not present in frame header - frame header unknown / not supported @@ -420,8 +420,8 @@ unsigned long long ZSTD_getDecompressedSize(const void* src, size_t srcSize) /** ZSTD_decodeFrameHeader() : -* `headerSize` must be the size provided by ZSTD_frameHeaderSize(). -* @return : 0 if success, or an error code, which can be tested using ZSTD_isError() */ + * `headerSize` must be the size provided by ZSTD_frameHeaderSize(). + * @return : 0 if success, or an error code, which can be tested using ZSTD_isError() */ static size_t ZSTD_decodeFrameHeader(ZSTD_DCtx* dctx, const void* src, size_t headerSize) { size_t const result = ZSTD_getFrameHeader_advanced(&(dctx->fParams), src, headerSize, dctx->format); @@ -439,7 +439,7 @@ static size_t ZSTD_decodeFrameHeader(ZSTD_DCtx* dctx, const void* src, size_t he ***************************************************************/ /*! ZSTD_getcBlockSize() : -* Provides the size of compressed block from block header `src` */ + * Provides the size of compressed block from block header `src`. */ size_t ZSTD_getcBlockSize(const void* src, size_t srcSize, blockProperties_t* bpPtr) { @@ -459,9 +459,10 @@ size_t ZSTD_getcBlockSize(const void* src, size_t srcSize, static size_t ZSTD_copyRawBlock(void* dst, size_t dstCapacity, const void* src, size_t srcSize) { - if (dst==NULL) return ERROR(dstSize_tooSmall); + DEBUGLOG(5, "ZSTD_copyRawBlock"); + if (dst == NULL) dstCapacity = 0; /* better safe than sorry */ if (srcSize > dstCapacity) return ERROR(dstSize_tooSmall); - memcpy(dst, src, srcSize); + if (dst) memcpy(dst, src, srcSize); return srcSize; } @@ -1761,7 +1762,9 @@ size_t ZSTD_findFrameCompressedSize(const void *src, size_t srcSize) } /*! ZSTD_decompressFrame() : -* @dctx must be properly initialized */ + * @dctx must be properly initialized + * will update *srcPtr and *srcSizePtr, + * to make *srcPtr progress by one frame. */ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx, void* dst, size_t dstCapacity, const void** srcPtr, size_t *srcSizePtr) @@ -1770,31 +1773,33 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx, BYTE* const ostart = (BYTE* const)dst; BYTE* const oend = ostart + dstCapacity; BYTE* op = ostart; - size_t remainingSize = *srcSizePtr; + size_t remainingSrcSize = *srcSizePtr; + + DEBUGLOG(4, "ZSTD_decompressFrame (srcSize:%i)", (int)*srcSizePtr); /* check */ - if (remainingSize < ZSTD_frameHeaderSize_min+ZSTD_blockHeaderSize) + if (remainingSrcSize < ZSTD_frameHeaderSize_min+ZSTD_blockHeaderSize) return ERROR(srcSize_wrong); /* Frame Header */ { size_t const frameHeaderSize = ZSTD_frameHeaderSize(ip, ZSTD_frameHeaderSize_prefix); if (ZSTD_isError(frameHeaderSize)) return frameHeaderSize; - if (remainingSize < frameHeaderSize+ZSTD_blockHeaderSize) + if (remainingSrcSize < frameHeaderSize+ZSTD_blockHeaderSize) return ERROR(srcSize_wrong); CHECK_F( ZSTD_decodeFrameHeader(dctx, ip, frameHeaderSize) ); - ip += frameHeaderSize; remainingSize -= frameHeaderSize; + ip += frameHeaderSize; remainingSrcSize -= frameHeaderSize; } /* Loop on each block */ while (1) { size_t decodedSize; blockProperties_t blockProperties; - size_t const cBlockSize = ZSTD_getcBlockSize(ip, remainingSize, &blockProperties); + size_t const cBlockSize = ZSTD_getcBlockSize(ip, remainingSrcSize, &blockProperties); if (ZSTD_isError(cBlockSize)) return cBlockSize; ip += ZSTD_blockHeaderSize; - remainingSize -= ZSTD_blockHeaderSize; - if (cBlockSize > remainingSize) return ERROR(srcSize_wrong); + remainingSrcSize -= ZSTD_blockHeaderSize; + if (cBlockSize > remainingSrcSize) return ERROR(srcSize_wrong); switch(blockProperties.blockType) { @@ -1817,7 +1822,7 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx, XXH64_update(&dctx->xxhState, op, decodedSize); op += decodedSize; ip += cBlockSize; - remainingSize -= cBlockSize; + remainingSrcSize -= cBlockSize; if (blockProperties.lastBlock) break; } @@ -1828,16 +1833,16 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx, if (dctx->fParams.checksumFlag) { /* Frame content checksum verification */ U32 const checkCalc = (U32)XXH64_digest(&dctx->xxhState); U32 checkRead; - if (remainingSize<4) return ERROR(checksum_wrong); + if (remainingSrcSize<4) return ERROR(checksum_wrong); checkRead = MEM_readLE32(ip); if (checkRead != checkCalc) return ERROR(checksum_wrong); ip += 4; - remainingSize -= 4; + remainingSrcSize -= 4; } /* Allow caller to get size read */ *srcPtr = ip; - *srcSizePtr = remainingSize; + *srcSizePtr = remainingSrcSize; return op-ostart; } @@ -1869,7 +1874,9 @@ static size_t ZSTD_decompressMultiFrame(ZSTD_DCtx* dctx, if (dctx->staticSize) return ERROR(memory_allocation); decodedSize = ZSTD_decompressLegacy(dst, dstCapacity, src, frameSize, dict, dictSize); + if (ZSTD_isError(decodedSize)) return decodedSize; + assert(decodedSize <=- dstCapacity); dst = (BYTE*)dst + decodedSize; dstCapacity -= decodedSize; @@ -1922,7 +1929,7 @@ static size_t ZSTD_decompressMultiFrame(ZSTD_DCtx* dctx, return ERROR(srcSize_wrong); } if (ZSTD_isError(res)) return res; - /* no need to bound check, ZSTD_decompressFrame already has */ + assert(res <= dstCapacity); dst = (BYTE*)dst + res; dstCapacity -= res; } diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 5616285b9..d17140392 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -411,11 +411,55 @@ static int basicUnitTests(U32 seed, double compressibility) DISPLAYLEVEL(3, "OK \n"); DISPLAYLEVEL(3, "test%3d : check CCtx size after compressing empty input : ", testNb++); - { ZSTD_CCtx* cctx = ZSTD_createCCtx(); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); size_t const r = ZSTD_compressCCtx(cctx, compressedBuffer, compressedBufferSize, NULL, 0, 19); if (ZSTD_isError(r)) goto _output_error; if (ZSTD_sizeof_CCtx(cctx) > (1U << 20)) goto _output_error; ZSTD_freeCCtx(cctx); + cSize = r; + } + DISPLAYLEVEL(3, "OK \n"); + + DISPLAYLEVEL(3, "test%3d : decompress empty frame into NULL : ", testNb++); + { size_t const r = ZSTD_decompress(NULL, 0, compressedBuffer, cSize); + if (ZSTD_isError(r)) goto _output_error; + if (r != 0) goto _output_error; + } + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + ZSTD_outBuffer output; + if (cctx==NULL) goto _output_error; + output.dst = compressedBuffer; + output.size = compressedBufferSize; + output.pos = 0; + CHECK_Z( ZSTD_initCStream(cctx, 1) ); /* content size unknown */ + CHECK_Z( ZSTD_flushStream(cctx, &output) ); /* ensure no possibility to "concatenate" and determine the content size */ + CHECK_Z( ZSTD_endStream(cctx, &output) ); + ZSTD_freeCCtx(cctx); + /* single scan decompression */ + { size_t const r = ZSTD_decompress(NULL, 0, compressedBuffer, output.pos); + if (ZSTD_isError(r)) goto _output_error; + if (r != 0) goto _output_error; + } + /* streaming decompression */ + { ZSTD_DCtx* const dstream = ZSTD_createDStream(); + ZSTD_inBuffer dinput; + ZSTD_outBuffer doutput; + size_t ipos; + if (dstream==NULL) goto _output_error; + dinput.src = compressedBuffer; + dinput.size = 0; + dinput.pos = 0; + doutput.dst = NULL; + doutput.size = 0; + doutput.pos = 0; + CHECK_Z ( ZSTD_initDStream(dstream) ); + for (ipos=1; ipos<=output.pos; ipos++) { + dinput.size = ipos; + CHECK_Z ( ZSTD_decompressStream(dstream, &doutput, &dinput) ); + } + if (doutput.pos != 0) goto _output_error; + ZSTD_freeDStream(dstream); + } } DISPLAYLEVEL(3, "OK \n");