From 450feb0f95636a7622e3c4b1e14ba33253a8c5c9 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Wed, 17 Apr 2019 11:14:49 -0700 Subject: [PATCH 1/5] [libzstd] Fix ZSTD_decompressBound() on bad skippable frames The function didn't verify that the skippable frame size is correct. --- lib/decompress/zstd_decompress.c | 5 ++++- lib/legacy/zstd_legacy.h | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 675596f5..b839a274 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -467,6 +467,9 @@ static ZSTD_frameSizeInfo ZSTD_findFrameSizeInfo(const void* src, size_t srcSize if ((srcSize >= ZSTD_SKIPPABLEHEADERSIZE) && (MEM_readLE32(src) & ZSTD_MAGIC_SKIPPABLE_MASK) == ZSTD_MAGIC_SKIPPABLE_START) { frameSizeInfo.compressedSize = readSkippableFrameSize(src, srcSize); + if (frameSizeInfo.compressedSize > srcSize) { + return ZSTD_errorFrameSizeInfo(ERROR(srcSize_wrong)); + } return frameSizeInfo; } else { const BYTE* ip = (const BYTE*)src; @@ -529,7 +532,6 @@ size_t ZSTD_findFrameCompressedSize(const void *src, size_t srcSize) return frameSizeInfo.compressedSize; } - /** ZSTD_decompressBound() : * compatible with legacy mode * `src` must point to the start of a ZSTD frame or a skippeable frame @@ -546,6 +548,7 @@ unsigned long long ZSTD_decompressBound(const void* src, size_t srcSize) unsigned long long const decompressedBound = frameSizeInfo.decompressedBound; if (ZSTD_isError(compressedSize) || decompressedBound == ZSTD_CONTENTSIZE_ERROR) return ZSTD_CONTENTSIZE_ERROR; + assert(srcSize >= compressedSize); src = (const BYTE*)src + compressedSize; srcSize -= compressedSize; bound += decompressedBound; diff --git a/lib/legacy/zstd_legacy.h b/lib/legacy/zstd_legacy.h index e5b383ee..27f88702 100644 --- a/lib/legacy/zstd_legacy.h +++ b/lib/legacy/zstd_legacy.h @@ -238,6 +238,10 @@ MEM_STATIC ZSTD_frameSizeInfo ZSTD_findFrameSizeInfoLegacy(const void *src, size frameSizeInfo.decompressedBound = ZSTD_CONTENTSIZE_ERROR; break; } + if (frameSizeInfo.compressedSize > srcSize) { + frameSizeInfo.compressedSize = ERROR(srcSize_wrong); + frameSizeInfo.decompressedBound = ZSTD_CONTENTSIZE_ERROR; + } return frameSizeInfo; } From 09caa4d8008b33eb1ead919c3ada7ab217376df7 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Wed, 17 Apr 2019 11:24:16 -0700 Subject: [PATCH 2/5] [fuzzer] Add a fuzzer for frame info functions Add a fuzzer that fuzzes all helper functions that take compressed input. This fuzzer caught one out of bounds read in `ZSTD_decompressBound()`. --- tests/fuzz/Makefile | 6 ++++- tests/fuzz/fuzz.py | 1 + tests/fuzz/zstd_frame_info.c | 43 ++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/fuzz/zstd_frame_info.c diff --git a/tests/fuzz/Makefile b/tests/fuzz/Makefile index 31b151b8..37568e41 100644 --- a/tests/fuzz/Makefile +++ b/tests/fuzz/Makefile @@ -69,7 +69,8 @@ FUZZ_TARGETS := \ stream_decompress \ block_decompress \ dictionary_round_trip \ - dictionary_decompress + dictionary_decompress \ + zstd_frame_info all: $(FUZZ_TARGETS) @@ -100,6 +101,9 @@ dictionary_round_trip: $(FUZZ_HEADERS) $(FUZZ_OBJ) dictionary_round_trip.o dictionary_decompress: $(FUZZ_HEADERS) $(FUZZ_OBJ) dictionary_decompress.o $(CXX) $(FUZZ_TARGET_FLAGS) $(FUZZ_OBJ) dictionary_decompress.o $(LIB_FUZZING_ENGINE) -o $@ +zstd_frame_info: $(FUZZ_HEADERS) $(FUZZ_OBJ) zstd_frame_info.o + $(CXX) $(FUZZ_TARGET_FLAGS) $(FUZZ_OBJ) zstd_frame_info.o $(LIB_FUZZING_ENGINE) -o $@ + libregression.a: $(FUZZ_HEADERS) $(PRGDIR)/util.h $(PRGDIR)/util.c regression_driver.o $(AR) $(FUZZ_ARFLAGS) $@ regression_driver.o diff --git a/tests/fuzz/fuzz.py b/tests/fuzz/fuzz.py index cd2a5b4d..489ef9f9 100755 --- a/tests/fuzz/fuzz.py +++ b/tests/fuzz/fuzz.py @@ -36,6 +36,7 @@ TARGETS = [ 'block_decompress', 'dictionary_round_trip', 'dictionary_decompress', + 'zstd_frame_info', ] ALL_TARGETS = TARGETS + ['all'] FUZZ_RNG_SEED_SIZE = 4 diff --git a/tests/fuzz/zstd_frame_info.c b/tests/fuzz/zstd_frame_info.c new file mode 100644 index 00000000..7512d5f4 --- /dev/null +++ b/tests/fuzz/zstd_frame_info.c @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2016-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under both the BSD-style license (found in the + * LICENSE file in the root directory of this source tree) and the GPLv2 (found + * in the COPYING file in the root directory of this source tree). + */ + +/** + * This fuzz target fuzzes all of the helper functions that consume compressed + * input. + */ + +#include +#include +#include +#include "fuzz_helpers.h" +#include "zstd_helpers.h" + +int LLVMFuzzerTestOneInput(const uint8_t *src, size_t size) +{ + ZSTD_frameHeader zfh; + /* Consume the seed to be compatible with the corpora of other decompression + * fuzzers. + */ + FUZZ_seed(&src, &size); + /* You can fuzz any helper functions here that are fast, and take zstd + * compressed data as input. E.g. don't expect the input to be a dictionary, + * so don't fuzz ZSTD_getDictID_fromDict(). + */ + ZSTD_getFrameContentSize(src, size); + ZSTD_getDecompressedSize(src, size); + ZSTD_findFrameCompressedSize(src, size); + ZSTD_getDictID_fromFrame(src, size); + ZSTD_findDecompressedSize(src, size); + ZSTD_decompressBound(src, size); + ZSTD_frameHeaderSize(src, size); + ZSTD_isFrame(src, size); + ZSTD_getFrameHeader(&zfh, src, size); + ZSTD_getFrameHeader_advanced(&zfh, src, size, ZSTD_f_zstd1); + return 0; +} From 5922f4e2ae8829f54c5ac842a1b43e8f11dbf3af Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Wed, 17 Apr 2019 11:34:52 -0700 Subject: [PATCH 3/5] [legacy] Return the right error code --- lib/legacy/zstd_legacy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/legacy/zstd_legacy.h b/lib/legacy/zstd_legacy.h index 27f88702..0dbd3c7a 100644 --- a/lib/legacy/zstd_legacy.h +++ b/lib/legacy/zstd_legacy.h @@ -238,7 +238,7 @@ MEM_STATIC ZSTD_frameSizeInfo ZSTD_findFrameSizeInfoLegacy(const void *src, size frameSizeInfo.decompressedBound = ZSTD_CONTENTSIZE_ERROR; break; } - if (frameSizeInfo.compressedSize > srcSize) { + if (!ZSTD_isError(frameSizeInfo.compressedSize) && frameSizeInfo.compressedSize > srcSize) { frameSizeInfo.compressedSize = ERROR(srcSize_wrong); frameSizeInfo.decompressedBound = ZSTD_CONTENTSIZE_ERROR; } From ee130a9889d251471cac6fefeefbf57139ad33a3 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Wed, 17 Apr 2019 11:41:55 -0700 Subject: [PATCH 4/5] [libzstd] Check the size in readSkippableFrameSize() --- lib/decompress/zstd_decompress.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index b839a274..e42872ad 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -360,8 +360,11 @@ static size_t readSkippableFrameSize(void const* src, size_t srcSize) sizeU32 = MEM_readLE32((BYTE const*)src + ZSTD_FRAMEIDSIZE); RETURN_ERROR_IF((U32)(sizeU32 + ZSTD_SKIPPABLEHEADERSIZE) < sizeU32, frameParameter_unsupported); - - return skippableHeaderSize + sizeU32; + { + size_t const skippableSize = skippableHeaderSize + sizeU32; + RETURN_ERROR_IF(skippableSize > srcSize, srcSize_wrong); + return skippableSize; + } } /** ZSTD_findDecompressedSize() : @@ -378,11 +381,10 @@ unsigned long long ZSTD_findDecompressedSize(const void* src, size_t srcSize) if ((magicNumber & ZSTD_MAGIC_SKIPPABLE_MASK) == ZSTD_MAGIC_SKIPPABLE_START) { size_t const skippableSize = readSkippableFrameSize(src, srcSize); - if (ZSTD_isError(skippableSize)) - return skippableSize; - if (srcSize < skippableSize) { + if (ZSTD_isError(skippableSize)) { return ZSTD_CONTENTSIZE_ERROR; } + assert(skippableSize <= srcSize); src = (const BYTE *)src + skippableSize; srcSize -= skippableSize; @@ -467,9 +469,8 @@ static ZSTD_frameSizeInfo ZSTD_findFrameSizeInfo(const void* src, size_t srcSize if ((srcSize >= ZSTD_SKIPPABLEHEADERSIZE) && (MEM_readLE32(src) & ZSTD_MAGIC_SKIPPABLE_MASK) == ZSTD_MAGIC_SKIPPABLE_START) { frameSizeInfo.compressedSize = readSkippableFrameSize(src, srcSize); - if (frameSizeInfo.compressedSize > srcSize) { - return ZSTD_errorFrameSizeInfo(ERROR(srcSize_wrong)); - } + assert(ZSTD_isError(frameSizeInfo.compressedSize) || + frameSizeInfo.compressedSize <= srcSize); return frameSizeInfo; } else { const BYTE* ip = (const BYTE*)src; @@ -741,9 +742,8 @@ static size_t ZSTD_decompressMultiFrame(ZSTD_DCtx* dctx, (unsigned)magicNumber, ZSTD_MAGICNUMBER); if ((magicNumber & ZSTD_MAGIC_SKIPPABLE_MASK) == ZSTD_MAGIC_SKIPPABLE_START) { size_t const skippableSize = readSkippableFrameSize(src, srcSize); - if (ZSTD_isError(skippableSize)) - return skippableSize; - RETURN_ERROR_IF(srcSize < skippableSize, srcSize_wrong); + FORWARD_IF_ERROR(skippableSize); + assert(skippableSize <= srcSize); src = (const BYTE *)src + skippableSize; srcSize -= skippableSize; From 58bcc328a4f0367a9c5f07f16574f0087f12b06d Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Wed, 17 Apr 2019 12:13:06 -0700 Subject: [PATCH 5/5] [fuzz] Add a seedcorpora target for oss-fuzz --- tests/fuzz/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/fuzz/Makefile b/tests/fuzz/Makefile index 37568e41..54e450a4 100644 --- a/tests/fuzz/Makefile +++ b/tests/fuzz/Makefile @@ -126,6 +126,9 @@ corpora/%: corpora/%_seed_corpus.zip .PHONY: corpora corpora: $(patsubst %,corpora/%,$(FUZZ_TARGETS)) +.PHONY: seedcorpora +seedcorpora: $(patsubst %,corpora/%_seed_corpus.zip,$(FUZZ_TARGETS)) + regressiontest: corpora CC="$(CC)" CXX="$(CXX)" CFLAGS="$(CFLAGS)" CXXFLAGS="$(CXXFLAGS)" LDFLAGS="$(LDFLAGS)" $(PYTHON) ./fuzz.py build all $(PYTHON) ./fuzz.py regression all