[lib] Fix NULL pointer dereference

When the output buffer is `NULL` with size 0, but the frame content size
is non-zero, we will write to the NULL pointer because our bounds check
underflowed.

This was exposed by a recent PR that allowed an empty frame into the
single-pass shortcut in streaming mode.

* Fix the bug.
* Fix another NULL dereference in zstd-v1.
* Overflow checks in 32-bit mode.
* Add a dedicated test.
* Expose the bug in the dedicated simple_decompress fuzzer.
* Switch all mallocs in fuzzers to return NULL for size=0.
* Fix a new timeout in a fuzzer.

Neither clang nor gcc show a decompression speed regression on x86-64.
On x86-32 clang is slightly positive and gcc loses 2.5% of speed.

Credit to OSS-Fuzz.
This commit is contained in:
Nick Terrell
2020-05-06 12:09:02 -07:00
committed by Nick Terrell
parent ad8dbae1b7
commit 5717bd39ee
26 changed files with 167 additions and 80 deletions
+3 -1
View File
@@ -709,7 +709,9 @@ FORCE_INLINE_TEMPLATE XXH_errorcode XXH64_update_endian (XXH64_state_t* state, c
state->total_len += len;
if (state->memsize + len < 32) { /* fill in tmp buffer */
XXH_memcpy(((BYTE*)state->mem64) + state->memsize, input, len);
if (input != NULL) {
XXH_memcpy(((BYTE*)state->mem64) + state->memsize, input, len);
}
state->memsize += (U32)len;
return XXH_OK;
}
+32 -14
View File
@@ -665,15 +665,15 @@ size_t ZSTD_execSequenceEnd(BYTE* op,
{
BYTE* const oLitEnd = op + sequence.litLength;
size_t const sequenceLength = sequence.litLength + sequence.matchLength;
BYTE* const oMatchEnd = op + sequenceLength; /* risk : address space overflow (32-bits) */
const BYTE* const iLitEnd = *litPtr + sequence.litLength;
const BYTE* match = oLitEnd - sequence.offset;
BYTE* const oend_w = oend - WILDCOPY_OVERLENGTH;
/* bounds checks */
assert(oLitEnd < oMatchEnd);
RETURN_ERROR_IF(oMatchEnd > oend, dstSize_tooSmall, "last match must fit within dstBuffer");
RETURN_ERROR_IF(iLitEnd > litLimit, corruption_detected, "try to read beyond literal buffer");
/* bounds checks : careful of address space overflow in 32-bit mode */
RETURN_ERROR_IF(sequenceLength > (size_t)(oend - op), dstSize_tooSmall, "last match must fit within dstBuffer");
RETURN_ERROR_IF(sequence.litLength > (size_t)(litLimit - *litPtr), corruption_detected, "try to read beyond literal buffer");
assert(op < op + sequenceLength);
assert(oLitEnd < op + sequenceLength);
/* copy literals */
ZSTD_safecopy(op, oend_w, *litPtr, sequence.litLength, ZSTD_no_overlap);
@@ -709,16 +709,27 @@ size_t ZSTD_execSequence(BYTE* op,
BYTE* const oLitEnd = op + sequence.litLength;
size_t const sequenceLength = sequence.litLength + sequence.matchLength;
BYTE* const oMatchEnd = op + sequenceLength; /* risk : address space overflow (32-bits) */
BYTE* const oend_w = oend - WILDCOPY_OVERLENGTH;
BYTE* const oend_w = oend - WILDCOPY_OVERLENGTH; /* risk : address space underflow on oend=NULL */
const BYTE* const iLitEnd = *litPtr + sequence.litLength;
const BYTE* match = oLitEnd - sequence.offset;
/* Errors and uncommon cases handled here. */
assert(oLitEnd < oMatchEnd);
if (UNLIKELY(iLitEnd > litLimit || oMatchEnd > oend_w))
assert(op != NULL /* Precondition */);
assert(oend_w < oend /* No underflow */);
/* Handle edge cases in a slow path:
* - Read beyond end of literals
* - Match end is within WILDCOPY_OVERLIMIT of oend
* - 32-bit mode and the match length overflows
*/
if (UNLIKELY(
iLitEnd > litLimit ||
oMatchEnd > oend_w ||
(MEM_32bits() && (size_t)(oend - op) < sequenceLength + WILDCOPY_OVERLENGTH)))
return ZSTD_execSequenceEnd(op, oend, sequence, litPtr, litLimit, prefixStart, virtualStart, dictEnd);
/* Assumptions (everything else goes into ZSTD_execSequenceEnd()) */
assert(op <= oLitEnd /* No overflow */);
assert(oLitEnd < oMatchEnd /* Non-zero match & no overflow */);
assert(oMatchEnd <= oend /* No underflow */);
assert(iLitEnd <= litLimit /* Literal length is in bounds */);
assert(oLitEnd <= oend_w /* Can wildcopy literals */);
assert(oMatchEnd <= oend_w /* Can wildcopy matches */);
@@ -968,6 +979,7 @@ ZSTD_decompressSequences_body( ZSTD_DCtx* dctx,
ZSTD_initFseState(&seqState.stateLL, &seqState.DStream, dctx->LLTptr);
ZSTD_initFseState(&seqState.stateOffb, &seqState.DStream, dctx->OFTptr);
ZSTD_initFseState(&seqState.stateML, &seqState.DStream, dctx->MLTptr);
assert(dst != NULL);
ZSTD_STATIC_ASSERT(
BIT_DStream_unfinished < BIT_DStream_completed &&
@@ -1043,8 +1055,10 @@ ZSTD_decompressSequences_body( ZSTD_DCtx* dctx,
/* last literal segment */
{ size_t const lastLLSize = litEnd - litPtr;
RETURN_ERROR_IF(lastLLSize > (size_t)(oend-op), dstSize_tooSmall, "");
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
if (op != NULL) {
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
return op-ostart;
@@ -1093,6 +1107,7 @@ ZSTD_decompressSequencesLong_body(
seqState.prefixStart = prefixStart;
seqState.pos = (size_t)(op-prefixStart);
seqState.dictEnd = dictEnd;
assert(dst != NULL);
assert(iend >= ip);
RETURN_ERROR_IF(
ERR_isError(BIT_initDStream(&seqState.DStream, ip, iend-ip)),
@@ -1134,8 +1149,10 @@ ZSTD_decompressSequencesLong_body(
/* last literal segment */
{ size_t const lastLLSize = litEnd - litPtr;
RETURN_ERROR_IF(lastLLSize > (size_t)(oend-op), dstSize_tooSmall, "");
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
if (op != NULL) {
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
return op-ostart;
@@ -1255,7 +1272,6 @@ ZSTD_getLongOffsetsShare(const ZSTD_seqSymbol* offTable)
}
#endif
size_t
ZSTD_decompressBlock_internal(ZSTD_DCtx* dctx,
void* dst, size_t dstCapacity,
@@ -1297,6 +1313,8 @@ ZSTD_decompressBlock_internal(ZSTD_DCtx* dctx,
ip += seqHSize;
srcSize -= seqHSize;
RETURN_ERROR_IF(dst == NULL && nbSeq > 0, dstSize_tooSmall, "NULL not handled");
#if !defined(ZSTD_FORCE_DECOMPRESS_SEQUENCES_SHORT) && \
!defined(ZSTD_FORCE_DECOMPRESS_SEQUENCES_LONG)
if ( !usePrefetchDecoder
+11 -5
View File
@@ -1078,7 +1078,7 @@ static size_t HUF_decompress_usingDTable( /* -3% slower when non static */
BYTE* const ostart = (BYTE*) dst;
BYTE* op = ostart;
BYTE* const omax = op + maxDstSize;
BYTE* const olimit = omax-15;
BYTE* const olimit = maxDstSize < 15 ? op : omax-15;
const void* ptr = DTable;
const HUF_DElt* const dt = (const HUF_DElt*)(ptr)+1;
@@ -1483,7 +1483,9 @@ static size_t ZSTDv01_getcBlockSize(const void* src, size_t srcSize, blockProper
static size_t ZSTD_copyUncompressedBlock(void* dst, size_t maxDstSize, const void* src, size_t srcSize)
{
if (srcSize > maxDstSize) return ERROR(dstSize_tooSmall);
memcpy(dst, src, srcSize);
if (srcSize > 0) {
memcpy(dst, src, srcSize);
}
return srcSize;
}
@@ -1541,7 +1543,9 @@ static size_t ZSTDv01_decodeLiteralsBlock(void* ctx,
size_t rleSize = litbp.origSize;
if (rleSize>maxDstSize) return ERROR(dstSize_tooSmall);
if (!srcSize) return ERROR(srcSize_wrong);
memset(oend - rleSize, *ip, rleSize);
if (rleSize > 0) {
memset(oend - rleSize, *ip, rleSize);
}
*litStart = oend - rleSize;
*litSize = rleSize;
ip++;
@@ -1901,8 +1905,10 @@ static size_t ZSTD_decompressSequences(
{
size_t lastLLSize = litEnd - litPtr;
if (op+lastLLSize > oend) return ERROR(dstSize_tooSmall);
if (op != litPtr) memmove(op, litPtr, lastLLSize);
op += lastLLSize;
if (lastLLSize > 0) {
if (op != litPtr) memmove(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
}
+7 -3
View File
@@ -2836,7 +2836,9 @@ static size_t ZSTD_getcBlockSize(const void* src, size_t srcSize, blockPropertie
static size_t ZSTD_copyUncompressedBlock(void* dst, size_t maxDstSize, const void* src, size_t srcSize)
{
if (srcSize > maxDstSize) return ERROR(dstSize_tooSmall);
memcpy(dst, src, srcSize);
if (srcSize > 0) {
memcpy(dst, src, srcSize);
}
return srcSize;
}
@@ -3229,8 +3231,10 @@ static size_t ZSTD_decompressSequences(
size_t lastLLSize = litEnd - litPtr;
if (litPtr > litEnd) return ERROR(corruption_detected);
if (op+lastLLSize > oend) return ERROR(dstSize_tooSmall);
if (op != litPtr) memmove(op, litPtr, lastLLSize);
op += lastLLSize;
if (lastLLSize > 0) {
if (op != litPtr) memmove(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
}
+7 -3
View File
@@ -2477,7 +2477,9 @@ static size_t ZSTD_getcBlockSize(const void* src, size_t srcSize, blockPropertie
static size_t ZSTD_copyUncompressedBlock(void* dst, size_t maxDstSize, const void* src, size_t srcSize)
{
if (srcSize > maxDstSize) return ERROR(dstSize_tooSmall);
memcpy(dst, src, srcSize);
if (srcSize > 0) {
memcpy(dst, src, srcSize);
}
return srcSize;
}
@@ -2870,8 +2872,10 @@ static size_t ZSTD_decompressSequences(
size_t lastLLSize = litEnd - litPtr;
if (litPtr > litEnd) return ERROR(corruption_detected);
if (op+lastLLSize > oend) return ERROR(dstSize_tooSmall);
if (op != litPtr) memmove(op, litPtr, lastLLSize);
op += lastLLSize;
if (lastLLSize > 0) {
if (op != litPtr) memmove(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
}
+7 -3
View File
@@ -2603,7 +2603,9 @@ static size_t ZSTD_getcBlockSize(const void* src, size_t srcSize, blockPropertie
static size_t ZSTD_copyRawBlock(void* dst, size_t maxDstSize, const void* src, size_t srcSize)
{
if (srcSize > maxDstSize) return ERROR(dstSize_tooSmall);
memcpy(dst, src, srcSize);
if (srcSize > 0) {
memcpy(dst, src, srcSize);
}
return srcSize;
}
@@ -3008,8 +3010,10 @@ static size_t ZSTD_decompressSequences(
size_t lastLLSize = litEnd - litPtr;
if (litPtr > litEnd) return ERROR(corruption_detected);
if (op+lastLLSize > oend) return ERROR(dstSize_tooSmall);
if (op != litPtr) memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
if (lastLLSize > 0) {
if (op != litPtr) memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
}
+4 -2
View File
@@ -3362,8 +3362,10 @@ static size_t ZSTDv05_decompressSequences(
size_t lastLLSize = litEnd - litPtr;
if (litPtr > litEnd) return ERROR(corruption_detected); /* too many literals already used */
if (op+lastLLSize > oend) return ERROR(dstSize_tooSmall);
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
if (lastLLSize > 0) {
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
return op-ostart;
+4 -2
View File
@@ -3501,8 +3501,10 @@ static size_t ZSTDv06_decompressSequences(
{ size_t const lastLLSize = litEnd - litPtr;
if (litPtr > litEnd) return ERROR(corruption_detected); /* too many literals already used */
if (op+lastLLSize > oend) return ERROR(dstSize_tooSmall);
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
if (lastLLSize > 0) {
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
return op-ostart;
+10 -4
View File
@@ -3272,7 +3272,9 @@ static size_t ZSTDv07_getcBlockSize(const void* src, size_t srcSize, blockProper
static size_t ZSTDv07_copyRawBlock(void* dst, size_t dstCapacity, const void* src, size_t srcSize)
{
if (srcSize > dstCapacity) return ERROR(dstSize_tooSmall);
memcpy(dst, src, srcSize);
if (srcSize > 0) {
memcpy(dst, src, srcSize);
}
return srcSize;
}
@@ -3714,8 +3716,10 @@ static size_t ZSTDv07_decompressSequences(
{ size_t const lastLLSize = litEnd - litPtr;
/* if (litPtr > litEnd) return ERROR(corruption_detected); */ /* too many literals already used */
if (lastLLSize > (size_t)(oend-op)) return ERROR(dstSize_tooSmall);
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
if (lastLLSize > 0) {
memcpy(op, litPtr, lastLLSize);
op += lastLLSize;
}
}
return op-ostart;
@@ -3776,7 +3780,9 @@ ZSTDLIBv07_API size_t ZSTDv07_insertBlock(ZSTDv07_DCtx* dctx, const void* blockS
static size_t ZSTDv07_generateNxBytes(void* dst, size_t dstCapacity, BYTE byte, size_t length)
{
if (length > dstCapacity) return ERROR(dstSize_tooSmall);
memset(dst, byte, length);
if (length > 0) {
memset(dst, byte, length);
}
return length;
}