From 8157a4c3ccc4d9b0de3f712a7eb752d49be4cf28 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 20 Dec 2016 10:47:52 -0800 Subject: [PATCH 1/2] Fix dictionary loading bug causing an MSAN failure Offset rep codes must be in the range `[1, dictSize)`. Fix dictionary loading to reject `0` as a offset rep code. --- lib/compress/zstd_compress.c | 6 +++--- lib/decompress/zstd_decompress.c | 6 +++--- lib/legacy/zstd_v07.c | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 7fd73baa..e88f6a1d 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2557,9 +2557,9 @@ static size_t ZSTD_loadDictEntropyStats(ZSTD_CCtx* cctx, const void* dict, size_ } if (dictPtr+12 > dictEnd) return ERROR(dictionary_corrupted); - cctx->rep[0] = MEM_readLE32(dictPtr+0); if (cctx->rep[0] >= dictSize) return ERROR(dictionary_corrupted); - cctx->rep[1] = MEM_readLE32(dictPtr+4); if (cctx->rep[1] >= dictSize) return ERROR(dictionary_corrupted); - cctx->rep[2] = MEM_readLE32(dictPtr+8); if (cctx->rep[2] >= dictSize) return ERROR(dictionary_corrupted); + cctx->rep[0] = MEM_readLE32(dictPtr+0); if (cctx->rep[0] == 0 || cctx->rep[0] >= dictSize) return ERROR(dictionary_corrupted); + cctx->rep[1] = MEM_readLE32(dictPtr+4); if (cctx->rep[1] == 0 || cctx->rep[1] >= dictSize) return ERROR(dictionary_corrupted); + cctx->rep[2] = MEM_readLE32(dictPtr+8); if (cctx->rep[2] == 0 || cctx->rep[2] >= dictSize) return ERROR(dictionary_corrupted); dictPtr += 12; { U32 offcodeMax = MaxOff; diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 085477bf..7addff8d 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1671,9 +1671,9 @@ static size_t ZSTD_loadEntropy(ZSTD_DCtx* dctx, const void* const dict, size_t c } if (dictPtr+12 > dictEnd) return ERROR(dictionary_corrupted); - dctx->rep[0] = MEM_readLE32(dictPtr+0); if (dctx->rep[0] >= dictSize) return ERROR(dictionary_corrupted); - dctx->rep[1] = MEM_readLE32(dictPtr+4); if (dctx->rep[1] >= dictSize) return ERROR(dictionary_corrupted); - dctx->rep[2] = MEM_readLE32(dictPtr+8); if (dctx->rep[2] >= dictSize) return ERROR(dictionary_corrupted); + dctx->rep[0] = MEM_readLE32(dictPtr+0); if (dctx->rep[0] == 0 || dctx->rep[0] >= dictSize) return ERROR(dictionary_corrupted); + dctx->rep[1] = MEM_readLE32(dictPtr+4); if (dctx->rep[1] == 0 || dctx->rep[1] >= dictSize) return ERROR(dictionary_corrupted); + dctx->rep[2] = MEM_readLE32(dictPtr+8); if (dctx->rep[2] == 0 || dctx->rep[2] >= dictSize) return ERROR(dictionary_corrupted); dictPtr += 12; dctx->litEntropy = dctx->fseEntropy = 1; diff --git a/lib/legacy/zstd_v07.c b/lib/legacy/zstd_v07.c index b607ec37..b1fcdc76 100644 --- a/lib/legacy/zstd_v07.c +++ b/lib/legacy/zstd_v07.c @@ -4134,9 +4134,9 @@ static size_t ZSTDv07_loadEntropy(ZSTDv07_DCtx* dctx, const void* const dict, si } if (dictPtr+12 > dictEnd) return ERROR(dictionary_corrupted); - dctx->rep[0] = MEM_readLE32(dictPtr+0); if (dctx->rep[0] >= dictSize) return ERROR(dictionary_corrupted); - dctx->rep[1] = MEM_readLE32(dictPtr+4); if (dctx->rep[1] >= dictSize) return ERROR(dictionary_corrupted); - dctx->rep[2] = MEM_readLE32(dictPtr+8); if (dctx->rep[2] >= dictSize) return ERROR(dictionary_corrupted); + dctx->rep[0] = MEM_readLE32(dictPtr+0); if (dctx->rep[0] == 0 || dctx->rep[0] >= dictSize) return ERROR(dictionary_corrupted); + dctx->rep[1] = MEM_readLE32(dictPtr+4); if (dctx->rep[1] == 0 || dctx->rep[1] >= dictSize) return ERROR(dictionary_corrupted); + dctx->rep[2] = MEM_readLE32(dictPtr+8); if (dctx->rep[2] == 0 || dctx->rep[2] >= dictSize) return ERROR(dictionary_corrupted); dictPtr += 12; dctx->litEntropy = dctx->fseEntropy = 1; From 9d085973646782eb2e74c8b19538437860c8aa5e Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 20 Dec 2016 11:13:45 -0800 Subject: [PATCH 2/2] Add test for invalid offset rep codes --- tests/.gitignore | 1 + tests/Makefile | 10 ++++++-- tests/invalidDictionaries.c | 51 +++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 tests/invalidDictionaries.c diff --git a/tests/.gitignore b/tests/.gitignore index 586c17b6..e932ad91 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -15,6 +15,7 @@ paramgrill32 roundTripCrash longmatch symbols +invalidDictionaries # Tmp test directory zstdtest diff --git a/tests/Makefile b/tests/Makefile index abf89f2f..f1c196ba 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -146,6 +146,9 @@ roundTripCrash : $(ZSTD_FILES) roundTripCrash.c longmatch : $(ZSTD_FILES) longmatch.c $(CC) $(FLAGS) $^ -o $@$(EXT) +invalidDictionaries : $(ZSTD_FILES) invalidDictionaries.c + $(CC) $(FLAGS) $^ -o $@$(EXT) + symbols : symbols.c $(MAKE) -C $(ZSTDDIR) libzstd ifneq (,$(filter Windows%,$(OS))) @@ -173,7 +176,7 @@ clean: fuzzer-dll$(EXT) zstreamtest-dll$(EXT) zbufftest-dll$(EXT)\ zstreamtest$(EXT) zstreamtest32$(EXT) \ datagen$(EXT) paramgrill$(EXT) roundTripCrash$(EXT) longmatch$(EXT) \ - symbols$(EXT) + symbols$(EXT) invalidDictionaries$(EXT) @echo Cleaning completed @@ -213,7 +216,7 @@ zstd-playTests: datagen file $(ZSTD) ZSTD="$(QEMU_SYS) $(ZSTD)" ./playTests.sh $(ZSTDRTTEST) -test: test-zstd test-fullbench test-fuzzer test-zstream test-longmatch +test: test-zstd test-fullbench test-fuzzer test-zstream test-longmatch test-invalidDictionaries test32: test-zstd32 test-fullbench32 test-fuzzer32 test-zstream32 @@ -273,6 +276,9 @@ test-zstream32: zstreamtest32 test-longmatch: longmatch $(QEMU_SYS) ./longmatch +test-invalidDictionaries: invalidDictionaries + $(QEMU_SYS) ./invalidDictionaries + test-symbols: symbols $(QEMU_SYS) ./symbols diff --git a/tests/invalidDictionaries.c b/tests/invalidDictionaries.c new file mode 100644 index 00000000..fe8b23b5 --- /dev/null +++ b/tests/invalidDictionaries.c @@ -0,0 +1,51 @@ +#include +#include "zstd.h" + +static const char invalidRepCode[] = { + 0x37, 0xa4, 0x30, 0xec, 0x2a, 0x00, 0x00, 0x00, 0x39, 0x10, 0xc0, 0xc2, + 0xa6, 0x00, 0x0c, 0x30, 0xc0, 0x00, 0x03, 0x0c, 0x30, 0x20, 0x72, 0xf8, + 0xb4, 0x6d, 0x4b, 0x9f, 0xfc, 0x97, 0x29, 0x49, 0xb2, 0xdf, 0x4b, 0x29, + 0x7d, 0x4a, 0xfc, 0x83, 0x18, 0x22, 0x75, 0x23, 0x24, 0x44, 0x4d, 0x02, + 0xb7, 0x97, 0x96, 0xf6, 0xcb, 0xd1, 0xcf, 0xe8, 0x22, 0xea, 0x27, 0x36, + 0xb7, 0x2c, 0x40, 0x46, 0x01, 0x08, 0x23, 0x01, 0x00, 0x00, 0x06, 0x1e, + 0x3c, 0x83, 0x81, 0xd6, 0x18, 0xd4, 0x12, 0x3a, 0x04, 0x00, 0x80, 0x03, + 0x08, 0x0e, 0x12, 0x1c, 0x12, 0x11, 0x0d, 0x0e, 0x0a, 0x0b, 0x0a, 0x09, + 0x10, 0x0c, 0x09, 0x05, 0x04, 0x03, 0x06, 0x06, 0x06, 0x02, 0x00, 0x03, + 0x00, 0x00, 0x02, 0x02, 0x00, 0x04, 0x06, 0x03, 0x06, 0x08, 0x24, 0x6b, + 0x0d, 0x01, 0x10, 0x04, 0x81, 0x07, 0x00, 0x00, 0x04, 0xb9, 0x58, 0x18, + 0x06, 0x59, 0x92, 0x43, 0xce, 0x28, 0xa5, 0x08, 0x88, 0xc0, 0x80, 0x88, + 0x8c, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, + 0x08, 0x00, 0x00, 0x00 +}; + +typedef struct dictionary_s { + const char *data; + size_t size; +} dictionary; + +static const dictionary dictionaries[] = { + {invalidRepCode, sizeof(invalidRepCode)}, + {NULL, 0}, +}; + +int main(int argc, const char** argv) { + const dictionary *dict; + for (dict = dictionaries; dict->data != NULL; ++dict) { + ZSTD_CDict *cdict; + ZSTD_DDict *ddict; + cdict = ZSTD_createCDict(dict->data, dict->size, 1); + if (cdict) { + ZSTD_freeCDict(cdict); + return 1; + } + ddict = ZSTD_createDDict(dict->data, dict->size); + if (ddict) { + ZSTD_freeDDict(ddict); + return 2; + } + } + + (void)argc; + (void)argv; + return 0; +}