[fix] Always return dstSize_tooSmall when it is the case

lib/decompress/zstd_decompress.c

@@ -579,11 +579,11 @@ static size_t ZSTD_copyRawBlock(void* dst, size_t dstCapacity,
                           const void* src, size_t srcSize)
 {
     DEBUGLOG(5, "ZSTD_copyRawBlock");
+    RETURN_ERROR_IF(srcSize > dstCapacity, dstSize_tooSmall, "");
     if (dst == NULL) {
         if (srcSize == 0) return 0;
         RETURN_ERROR(dstBuffer_null, "");
     }
-    RETURN_ERROR_IF(srcSize > dstCapacity, dstSize_tooSmall, "");
     memcpy(dst, src, srcSize);
     return srcSize;
 }
@@ -592,11 +592,11 @@ static size_t ZSTD_setRleBlock(void* dst, size_t dstCapacity,
                                BYTE b,
                                size_t regenSize)
 {
+    RETURN_ERROR_IF(regenSize > dstCapacity, dstSize_tooSmall, "");
     if (dst == NULL) {
         if (regenSize == 0) return 0;
         RETURN_ERROR(dstBuffer_null, "");
     }
-    RETURN_ERROR_IF(regenSize > dstCapacity, dstSize_tooSmall, "");
     memset(dst, b, regenSize);
     return regenSize;
 }

lib/decompress/zstd_decompress_block.c

@@ -1084,14 +1084,14 @@ ZSTD_decompressSequences_body( ZSTD_DCtx* dctx,
 #endif
             DEBUGLOG(6, "regenerated sequence size : %u", (U32)oneSeqSize);
             BIT_reloadDStream(&(seqState.DStream));
+            op += oneSeqSize;
             /* gcc and clang both don't like early returns in this loop.
-             * gcc doesn't like early breaks either.
+             * Instead break and check for an error at the end of the loop.
-             * Instead save an error and report it at the end.
-             * When there is an error, don't increment op, so we don't
-             * overwrite.
              */
-            if (UNLIKELY(ZSTD_isError(oneSeqSize))) error = oneSeqSize;
+            if (UNLIKELY(ZSTD_isError(oneSeqSize))) {
-            else op += oneSeqSize;
+                error = oneSeqSize;
+                break;
+            }
             if (UNLIKELY(!--nbSeq)) break;
         }
 

tests/fuzz/Makefile

@@ -95,7 +95,8 @@ FUZZ_TARGETS :=       \
 	simple_compress \
 	dictionary_loader \
 	raw_dictionary_round_trip \
-	dictionary_stream_round_trip
+	dictionary_stream_round_trip \
+	decompress_dstSize_tooSmall
 
 all: $(FUZZ_TARGETS)
 
@@ -180,6 +181,9 @@ zstd_frame_info: $(FUZZ_HEADERS) $(FUZZ_DECOMPRESS_OBJ) d_fuzz_zstd_frame_info.o
 dictionary_loader: $(FUZZ_HEADERS) $(FUZZ_ROUND_TRIP_OBJ) rt_fuzz_dictionary_loader.o
 	$(CXX) $(FUZZ_TARGET_FLAGS) $(FUZZ_ROUND_TRIP_OBJ) rt_fuzz_dictionary_loader.o $(LIB_FUZZING_ENGINE) -o $@
 
+decompress_dstSize_tooSmall: $(FUZZ_HEADERS) $(FUZZ_DECOMPRESS_OBJ) d_fuzz_decompress_dstSize_tooSmall.o
+	$(CXX) $(FUZZ_TARGET_FLAGS) $(FUZZ_DECOMPRESS_OBJ) d_fuzz_decompress_dstSize_tooSmall.o $(LIB_FUZZING_ENGINE) -o $@
+
 libregression.a: $(FUZZ_HEADERS) $(PRGDIR)/util.h $(PRGDIR)/util.c d_fuzz_regression_driver.o
 	$(AR) $(FUZZ_ARFLAGS) $@ d_fuzz_regression_driver.o
 

tests/fuzz/decompress_dstSize_tooSmall.c

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2016-2020, 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).
+ * You may select, at your option, one of the above-listed licenses.
+ */
+
+/**
+ * This fuzz target attempts to decompress a valid compressed frame into
+ * an output buffer that is too small to ensure we always get
+ * ZSTD_error_dstSize_tooSmall.
+ */
+
+#include <stddef.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include "fuzz_helpers.h"
+#include "zstd.h"
+#include "zstd_errors.h"
+#include "zstd_helpers.h"
+#include "fuzz_data_producer.h"
+
+static ZSTD_CCtx *cctx = NULL;
+static ZSTD_DCtx *dctx = NULL;
+
+int LLVMFuzzerTestOneInput(const uint8_t *src, size_t size)
+{
+    /* Give a random portion of src data to the producer, to use for
+    parameter generation. The rest will be used for (de)compression */
+    FUZZ_dataProducer_t *producer = FUZZ_dataProducer_create(src, size);
+    size_t rBufSize = FUZZ_dataProducer_uint32Range(producer, 0, size);
+    size = FUZZ_dataProducer_remainingBytes(producer);
+    /* Ensure the round-trip buffer is too small. */
+    if (rBufSize >= size) {
+        rBufSize = size > 0 ? size - 1 : 0;
+    }
+    size_t const cBufSize = ZSTD_compressBound(size);
+
+    if (!cctx) {
+        cctx = ZSTD_createCCtx();
+        FUZZ_ASSERT(cctx);
+    }
+    if (!dctx) {
+        dctx = ZSTD_createDCtx();
+        FUZZ_ASSERT(dctx);
+    }
+
+    void *cBuf = FUZZ_malloc(cBufSize);
+    void *rBuf = FUZZ_malloc(rBufSize);
+    size_t const cSize = ZSTD_compressCCtx(cctx, cBuf, cBufSize, src, size, 1);
+    FUZZ_ZASSERT(cSize);
+    size_t const rSize = ZSTD_decompressDCtx(dctx, rBuf, rBufSize, cBuf, cSize);
+    if (size == 0) {
+        FUZZ_ASSERT(rSize == 0);
+    } else {
+        FUZZ_ASSERT(ZSTD_isError(rSize));
+        FUZZ_ASSERT(ZSTD_getErrorCode(rSize) == ZSTD_error_dstSize_tooSmall);
+    }
+    free(cBuf);
+    free(rBuf);
+    FUZZ_dataProducer_free(producer);
+#ifndef STATEFUL_FUZZING
+    ZSTD_freeCCtx(cctx); cctx = NULL;
+    ZSTD_freeDCtx(dctx); dctx = NULL;
+#endif
+    return 0;
+}

tests/fuzz/fuzz.py

@@ -59,6 +59,7 @@ TARGET_INFO = {
     'dictionary_loader': TargetInfo(InputType.DICTIONARY_DATA),
     'raw_dictionary_round_trip': TargetInfo(InputType.RAW_DATA),
     'dictionary_stream_round_trip': TargetInfo(InputType.RAW_DATA),
+    'decompress_dstSize_tooSmall': TargetInfo(InputType.RAW_DATA),
 }
 TARGETS = list(TARGET_INFO.keys())
 ALL_TARGETS = TARGETS + ['all']

tests/fuzz/simple_compress.c

@@ -19,6 +19,7 @@
 #include <stdio.h>
 #include "fuzz_helpers.h"
 #include "zstd.h"
+#include "zstd_errors.h"
 #include "zstd_helpers.h"
 #include "fuzz_data_producer.h"
 
@@ -42,7 +43,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *src, size_t size)
     }
 
     void *rBuf = FUZZ_malloc(bufSize);
-    ZSTD_compressCCtx(cctx, rBuf, bufSize, src, size, cLevel);
+    size_t const ret = ZSTD_compressCCtx(cctx, rBuf, bufSize, src, size, cLevel);
+    if (ZSTD_isError(ret)) {
+        FUZZ_ASSERT(ZSTD_getErrorCode(ret) == ZSTD_error_dstSize_tooSmall);
+    }
     free(rBuf);
     FUZZ_dataProducer_free(producer);
 #ifndef STATEFUL_FUZZING

tests/fuzzer.c

@@ -3053,7 +3053,7 @@ static int fuzzerTests(U32 seed, unsigned nbTests, unsigned startTest, U32 const
                 DISPLAYLEVEL(5, "fuzzer t%u: compress into too small buffer of size %u (missing %u bytes) \n",
                             testNb, (unsigned)tooSmallSize, (unsigned)missing);
                 { size_t const errorCode = ZSTD_compressCCtx(ctx, dstBuffer, tooSmallSize, sampleBuffer, sampleSize, cLevel);
-                  CHECK(!ZSTD_isError(errorCode), "ZSTD_compressCCtx should have failed ! (buffer too small : %u < %u)", (unsigned)tooSmallSize, (unsigned)cSize); }
+                  CHECK(ZSTD_getErrorCode(errorCode) != ZSTD_error_dstSize_tooSmall, "ZSTD_compressCCtx should have failed ! (buffer too small : %u < %u)", (unsigned)tooSmallSize, (unsigned)cSize); }
                 { unsigned endCheck; memcpy(&endCheck, dstBuffer+tooSmallSize, sizeof(endCheck));
                   CHECK(endCheck != endMark, "ZSTD_compressCCtx : dst buffer overflow  (check.%08X != %08X.mark)", endCheck, endMark); }
         }   }
@@ -3100,7 +3100,7 @@ static int fuzzerTests(U32 seed, unsigned nbTests, unsigned startTest, U32 const
             static const BYTE token = 0xA9;
             dstBuffer[tooSmallSize] = token;
             { size_t const errorCode = ZSTD_decompress(dstBuffer, tooSmallSize, cBuffer, cSize);
-              CHECK(!ZSTD_isError(errorCode), "ZSTD_decompress should have failed : %u > %u (dst buffer too small)", (unsigned)errorCode, (unsigned)tooSmallSize); }
+              CHECK(ZSTD_getErrorCode(errorCode) != ZSTD_error_dstSize_tooSmall, "ZSTD_decompress should have failed : %u > %u (dst buffer too small)", (unsigned)errorCode, (unsigned)tooSmallSize); }
             CHECK(dstBuffer[tooSmallSize] != token, "ZSTD_decompress : dst buffer overflow");
         }