[fix] Always return dstSize_tooSmall when it is the case

diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c
index be5c7cfc3347da0291d839379b8ca15aab809e7c..4b04218f7bf96d754a9b32cd3305e71d8985cb62 100644 (file)
--- a/lib/decompress/zstd_decompress.c
+++ b/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;
 }

diff --git a/lib/decompress/zstd_decompress_block.c b/lib/decompress/zstd_decompress_block.c
index e93d6febe93f5548bc7fed0984e562656012ac8e..ca0de662ec895d5e1f08e19e8023cd03b1d14ce6 100644 (file)
--- a/lib/decompress/zstd_decompress_block.c
+++ b/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 save an error and report it at the end.
-             * When there is an error, don't increment op, so we don't
-             * overwrite.
+             * Instead break and check for an error at the end of the loop.
              */
-            if (UNLIKELY(ZSTD_isError(oneSeqSize))) error = oneSeqSize;
-            else op += oneSeqSize;
+            if (UNLIKELY(ZSTD_isError(oneSeqSize))) {
+                error = oneSeqSize;
+                break;
+            }
             if (UNLIKELY(!--nbSeq)) break;
         }
 

diff --git a/tests/fuzz/Makefile b/tests/fuzz/Makefile
index 1af3dc734192352cef85e46e261e8d028ad0d45b..42988c3478de86bcad3902a3cb55e4197cb41e7d 100644 (file)
--- a/tests/fuzz/Makefile
+++ b/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
 

diff --git a/tests/fuzz/decompress_dstSize_tooSmall.c b/tests/fuzz/decompress_dstSize_tooSmall.c
new file mode 100644 (file)
index 0000000..e47b3d0
--- /dev/null
+++ b/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;
+}

diff --git a/tests/fuzz/fuzz.py b/tests/fuzz/fuzz.py
index 6875d1d60361bac54886d4f93a291b3d8f4d33d0..6332eeb912131858b311146f528f8bebd37c271d 100755 (executable)
--- a/tests/fuzz/fuzz.py
+++ b/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']

diff --git a/tests/fuzz/simple_compress.c b/tests/fuzz/simple_compress.c
index b64f373eb8a410e5b6c2a20302c41639f10587fc..620177fb0efff0b3c42875de037188cf3494adf3 100644 (file)
--- a/tests/fuzz/simple_compress.c
+++ b/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

diff --git a/tests/fuzzer.c b/tests/fuzzer.c
index 8ac2864f3e30db6a0ea2c531eaf991f8caa23682..abb4e86189098b5c3a6e60df5c4503da44c947de 100644 (file)
--- a/tests/fuzzer.c
+++ b/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");
         }