Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix undefined behavior in ZSTD_decompressStream() #3258

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .github/workflows/dev-long-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ jobs:
- name: thread sanitizer zstreamtest
run: CC=clang ZSTREAM_TESTTIME=-T3mn make tsan-test-zstream

ubsan-zstreamtest:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: undefined behavior sanitizer zstreamtest
run: CC=clang make uasan-test-zstream

# lasts ~15mn
tsan-fuzztest:
runs-on: ubuntu-latest
Expand All @@ -69,6 +76,13 @@ jobs:
make gcc8install
CC=gcc-8 make -j uasan-test-zstd </dev/null V=1

clang-asan-ubsan-testzstd:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: clang + ASan + UBSan + Test Zstd
run: CC=clang make -j uasan-test-zstd </dev/null V=1

gcc-asan-ubsan-testzstd-32bit:
runs-on: ubuntu-latest
steps:
Expand All @@ -93,6 +107,13 @@ jobs:
make gcc8install
CC=gcc-8 FUZZER_FLAGS="--long-tests" make clean uasan-fuzztest

clang-asan-ubsan-fuzz:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: clang + ASan + UBSan + Fuzz Test
run: CC=clang FUZZER_FLAGS="--long-tests" make clean uasan-fuzztest

gcc-asan-ubsan-fuzz32:
runs-on: ubuntu-latest
steps:
Expand All @@ -103,13 +124,30 @@ jobs:
make libc6install
CFLAGS="-O3 -m32" FUZZER_FLAGS="--long-tests" make uasan-fuzztest

clang-asan-ubsan-fuzz32:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: clang + ASan + UBSan + Fuzz Test 32bit
run: |
sudo apt-get -qqq update
make libc6install
CC=clang CFLAGS="-O3 -m32" FUZZER_FLAGS="--long-tests" make uasan-fuzztest

asan-ubsan-regression:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: ASan + UBSan + Regression Test
run: make -j uasanregressiontest

clang-ubsan-regression:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: clang + ASan + UBSan + Regression Test
run: CC=clang make -j uasanregressiontest

msan-regression:
runs-on: ubuntu-latest
steps:
Expand Down
9 changes: 6 additions & 3 deletions lib/decompress/zstd_decompress.c
Original file line number Diff line number Diff line change
Expand Up @@ -2059,7 +2059,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB
if (ZSTD_isError(decompressedSize)) return decompressedSize;
DEBUGLOG(4, "shortcut to single-pass ZSTD_decompress_usingDDict()")
ip = istart + cSize;
op += decompressedSize;
op = op ? op + decompressedSize : op; /* can occur if frameContentSize = 0 (empty frame) */
zds->expected = 0;
zds->streamStage = zdss_init;
someMoreWork = 0;
Expand Down Expand Up @@ -2177,14 +2177,17 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB
break;
}
case zdss_flush:
{ size_t const toFlushSize = zds->outEnd - zds->outStart;
if (op != NULL) {
daniellerozenblit marked this conversation as resolved.
Show resolved Hide resolved
size_t const toFlushSize = zds->outEnd - zds->outStart;
size_t const flushedSize = ZSTD_limitCopy(op, (size_t)(oend-op), zds->outBuff + zds->outStart, toFlushSize);

op += flushedSize;

zds->outStart += flushedSize;
if (flushedSize == toFlushSize) { /* flush completed */
zds->streamStage = zdss_read;
if ( (zds->outBuffSize < zds->fParams.frameContentSize)
&& (zds->outStart + zds->fParams.blockSizeMax > zds->outBuffSize) ) {
&& (zds->outStart + zds->fParams.blockSizeMax > zds->outBuffSize) ) {
DEBUGLOG(5, "restart filling outBuff from beginning (left:%i, needed:%u)",
(int)(zds->outBuffSize - zds->outStart),
(U32)zds->fParams.blockSizeMax);
Expand Down
1 change: 1 addition & 0 deletions tests/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ zstreamtest
zstreamtest32
zstreamtest_asan
zstreamtest_tsan
zstreamtest_ubsan
zstreamtest-dll
datagen
paramgrill
Expand Down
5 changes: 5 additions & 0 deletions tests/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ zstreamtest_tsan : CFLAGS += -fsanitize=thread
zstreamtest_tsan : $(ZSTREAMFILES)
$(LINK.c) $(MULTITHREAD) $^ -o $@$(EXT)

zstreamtest_ubsan : CFLAGS += -fsanitize=undefined
daniellerozenblit marked this conversation as resolved.
Show resolved Hide resolved
zstreamtest_ubsan : $(ZSTREAMFILES)
$(LINK.c) $(MULTITHREAD) $^ -o $@$(EXT)

# note : broken : requires symbols unavailable from dynamic library
zstreamtest-dll : $(ZSTDDIR)/common/xxhash.c # xxh symbols not exposed from dll
zstreamtest-dll : $(ZSTREAM_LOCAL_FILES)
Expand Down Expand Up @@ -238,6 +242,7 @@ clean:
fuzzer$(EXT) fuzzer32$(EXT) \
fuzzer-dll$(EXT) zstreamtest-dll$(EXT) \
zstreamtest$(EXT) zstreamtest32$(EXT) \
zstreamtest_ubsan$(EXT) zstreamtest_asan$(EXT) zstreamtest_tsan$(EXT) \
datagen$(EXT) paramgrill$(EXT) roundTripCrash$(EXT) longmatch$(EXT) \
symbols$(EXT) invalidDictionaries$(EXT) legacy$(EXT) poolTests$(EXT) \
decodecorpus$(EXT) checkTag$(EXT) bigdict$(EXT)
Expand Down
32 changes: 31 additions & 1 deletion tests/zstreamtest.c
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ static int basicUnitTests(U32 seed, double compressibility)
}
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : NULL buffers : ", testNb++);
DISPLAYLEVEL(3, "test%3i : NULL output and NULL input : ", testNb++);
inBuff.src = NULL;
inBuff.size = 0;
inBuff.pos = 0;
Expand All @@ -548,6 +548,36 @@ static int basicUnitTests(U32 seed, double compressibility)
{ size_t const ret = ZSTD_decompressStream(zd, &outBuff, &inBuff);
if (ret != 0) goto _output_error;
}
DISPLAYLEVEL(3, "OK\n");

DISPLAYLEVEL(3, "test%3i : NULL output buffer with non-NULL input : ", testNb++);
{
daniellerozenblit marked this conversation as resolved.
Show resolved Hide resolved
const char* test = "aa";
inBuff.src = test;
inBuff.size = 2;
inBuff.pos = 0;
outBuff.dst = NULL;
outBuff.size = 0;
outBuff.pos = 0;
CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) );
CHECK(inBuff.pos != inBuff.size, "Entire input should be consumed");
CHECK_Z( ZSTD_endStream(zc, &outBuff) );
outBuff.dst = (char*)(compressedBuffer);
outBuff.size = compressedBufferSize;
outBuff.pos = 0;
{ size_t const r = ZSTD_endStream(zc, &outBuff);
CHECK(r != 0, "Error or some data not flushed (ret=%zu)", r);
}
inBuff.src = outBuff.dst;
inBuff.size = outBuff.pos;
inBuff.pos = 0;
outBuff.dst = NULL;
outBuff.size = 0;
outBuff.pos = 0;
CHECK_Z( ZSTD_initDStream(zd) );
CHECK_Z(ZSTD_decompressStream(zd, &outBuff, &inBuff));
}

DISPLAYLEVEL(3, "OK\n");
/* _srcSize compression test */
DISPLAYLEVEL(3, "test%3i : compress_srcSize %u bytes : ", testNb++, COMPRESSIBLE_NOISE_LENGTH);
Expand Down