error on no forward progress

streaming decoders, such as ZSTD_decompressStream() or ZSTD_decompress_generic(),
may end up making no forward progress,
(aka no byte read from input __and__ no byte written to output),
due to unusual parameters conditions,
such as providing an output buffer already full.

In such case, the caller may be caught in an infinite loop,
calling the streaming decompression function again and again,
without making any progress.

This version detects such situation, and generates an error instead :
ZSTD_error_dstSize_tooSmall when output buffer is full,
ZSTD_error_srcSize_wrong when input buffer is empty.

The detection tolerates a number of attempts before triggering an error,
controlled by ZSTD_NO_FORWARD_PROGRESS_MAX macro constant,
which is set to 16 by default, and can be re-defined at compilation time.
This behavior tolerates potentially existing implementations
where such cases happen sporadically, like once or twice,
which is not dangerous (only infinite loops are),
without generating an error, hence without breaking these implementations.

diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c
index 4de98a8e6983e4117e91ef8d85bd6bc4e87b601b..8f4589d13938df181170904f92c04e207b9f0ded 100644 (file)
--- a/lib/decompress/zstd_decompress.c
+++ b/lib/decompress/zstd_decompress.c
@@ -41,6 +41,17 @@
 #endif
 
 
+/*!
+ *  NO_FORWARD_PROGRESS_MAX :
+ *  maximum allowed nb of calls to ZSTD_decompressStream() and ZSTD_decompress_generic()
+ *  without any forward progress
+ *  (defined as: no byte read from input, and no byte flushed to output)
+ *  before triggering an error.
+ */
+#ifndef ZSTD_NO_FORWARD_PROGRESS_MAX
+#  define ZSTD_NO_FORWARD_PROGRESS_MAX 16
+#endif
+
 /*-*******************************************************
 *  Dependencies
 *********************************************************/
@@ -153,6 +164,7 @@ struct ZSTD_DCtx_s
     U32 previousLegacyVersion;
     U32 legacyVersion;
     U32 hostageByte;
+    int noForwardProgress;
 
     /* workspace */
     BYTE litBuffer[ZSTD_BLOCKSIZE_MAX + WILDCOPY_OVERLENGTH];
@@ -194,6 +206,7 @@ static void ZSTD_initDCtx_internal(ZSTD_DCtx* dctx)
     dctx->streamStage = zdss_init;
     dctx->legacyContext = NULL;
     dctx->previousLegacyVersion = 0;
+    dctx->noForwardProgress = 0;
     dctx->bmi2 = ZSTD_cpuid_bmi2(ZSTD_cpuid());
 }
 
@@ -2620,6 +2633,7 @@ size_t ZSTD_initDStream_usingDict(ZSTD_DStream* zds, const void* dict, size_t di
 {
     DEBUGLOG(4, "ZSTD_initDStream_usingDict");
     zds->streamStage = zdss_init;
+    zds->noForwardProgress = 0;
     CHECK_F( ZSTD_DCtx_loadDictionary(zds, dict, dictSize) );
     return ZSTD_frameHeaderSize_prefix;
 }
@@ -2960,8 +2974,18 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB
     }   }
 
     /* result */
-    input->pos += (size_t)(ip-istart);
-    output->pos += (size_t)(op-ostart);
+    input->pos = (size_t)(ip - (const char*)(input->src));
+    output->pos = (size_t)(op - (char*)(output->dst));
+    if ((ip==istart) && (op==ostart)) {  /* no forward progress */
+        zds->noForwardProgress ++;
+        if (zds->noForwardProgress >= ZSTD_NO_FORWARD_PROGRESS_MAX) {
+            if (op==oend) return ERROR(dstSize_tooSmall);
+            if (ip==iend) return ERROR(srcSize_wrong);
+            assert(0);
+        }
+    } else {
+        zds->noForwardProgress = 0;
+    }
     {   size_t nextSrcSizeHint = ZSTD_nextSrcSizeToDecompress(zds);
         if (!nextSrcSizeHint) {   /* frame fully decoded */
             if (zds->outEnd == zds->outStart) {  /* output fully flushed */

diff --git a/tests/fuzzer.c b/tests/fuzzer.c
index 9b4cd58d51ecb5afa5c70baf3e2b343891501832..f7bacd5ca2b914342e92726fafcbb98cb9a957f5 100644 (file)
--- a/tests/fuzzer.c
+++ b/tests/fuzzer.c
@@ -66,6 +66,9 @@ static UTIL_time_t g_displayClock = UTIL_TIME_INITIALIZER;
             if (g_displayLevel>=4) fflush(stderr); } }
 
 
+/*-*******************************************************
+*  Compile time test
+*********************************************************/
 #undef MIN
 #undef MAX
 void FUZ_bug976(void)
@@ -74,6 +77,7 @@ void FUZ_bug976(void)
     assert(ZSTD_CHAINLOG_MAX < 31);
 }
 
+
 /*-*******************************************************
 *  Internal functions
 *********************************************************/

diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c
index c76622701ed3e56b4dd47ad393510fbf9279661e..70da297289ff182ce1a89930a85915a65f212336 100644 (file)
--- a/tests/zstreamtest.c
+++ b/tests/zstreamtest.c
@@ -433,11 +433,12 @@ static int basicUnitTests(U32 seed, double compressibility)
         inBuff.pos = 0;
         outBuff.pos = 0;
         while (r) {   /* skippable frame */
-            size_t const inSize = FUZ_rand(&coreSeed) & 15;
-            size_t const outSize = FUZ_rand(&coreSeed) & 15;
+            size_t const inSize = (FUZ_rand(&coreSeed) & 15) + 1;
+            size_t const outSize = (FUZ_rand(&coreSeed) & 15) + 1;
             inBuff.size = inBuff.pos + inSize;
             outBuff.size = outBuff.pos + outSize;
             r = ZSTD_decompressStream(zd, &outBuff, &inBuff);
+            if (ZSTD_isError(r)) DISPLAYLEVEL(4, "ZSTD_decompressStream on skippable frame error : %s \n", ZSTD_getErrorName(r));
             if (ZSTD_isError(r)) goto _output_error;
         }
         /* normal frame */
@@ -445,14 +446,17 @@ static int basicUnitTests(U32 seed, double compressibility)
         r=1;
         while (r) {
             size_t const inSize = FUZ_rand(&coreSeed) & 15;
-            size_t const outSize = FUZ_rand(&coreSeed) & 15;
+            size_t const outSize = (FUZ_rand(&coreSeed) & 15) + (!inSize);   /* avoid having both sizes at 0 => would trigger a no_forward_progress error */
             inBuff.size = inBuff.pos + inSize;
             outBuff.size = outBuff.pos + outSize;
             r = ZSTD_decompressStream(zd, &outBuff, &inBuff);
+            if (ZSTD_isError(r)) DISPLAYLEVEL(4, "ZSTD_decompressStream error : %s \n", ZSTD_getErrorName(r));
             if (ZSTD_isError(r)) goto _output_error;
         }
     }
+    if (outBuff.pos != CNBufferSize) DISPLAYLEVEL(4, "outBuff.pos != CNBufferSize : should have regenerated same amount ! \n");
     if (outBuff.pos != CNBufferSize) goto _output_error;   /* should regenerate the same amount */
+    if (inBuff.pos != cSize) DISPLAYLEVEL(4, "inBuff.pos != cSize : should have real all input ! \n");
     if (inBuff.pos != cSize) goto _output_error;   /* should have read the entire frame */
     DISPLAYLEVEL(3, "OK \n");
 
@@ -464,6 +468,30 @@ static int basicUnitTests(U32 seed, double compressibility)
     }   }
     DISPLAYLEVEL(3, "OK \n");
 
+    /* Decompression forward progress */
+    DISPLAYLEVEL(3, "test%3i : generate error when ZSTD_decompressStream() doesn't progress : ", testNb++);
+    {   /* skippable frame */
+        size_t r = 0;
+        int decNb = 0;
+        int const maxDec = 100;
+        inBuff.src = compressedBuffer;
+        inBuff.size = cSize;
+        inBuff.pos = 0;
+
+        outBuff.dst = decodedBuffer;
+        outBuff.pos = 0;
+        outBuff.size = CNBufferSize-1;   /* 1 byte missing */
+
+        for (decNb=0; decNb<maxDec; decNb++) {
+            if (r==0) ZSTD_initDStream_usingDict(zd, CNBuffer, dictSize);
+            r = ZSTD_decompressStream(zd, &outBuff, &inBuff);
+            if (ZSTD_isError(r)) break;
+        }
+        if (!ZSTD_isError(r)) DISPLAYLEVEL(4, "ZSTD_decompressStream should have triggered a no_forward_progress error \n");
+        if (!ZSTD_isError(r)) goto _output_error;   /* should have triggered no_forward_progress error */
+    }
+    DISPLAYLEVEL(3, "OK \n");
+
     /* _srcSize compression test */
     DISPLAYLEVEL(3, "test%3i : compress_srcSize %u bytes : ", testNb++, COMPRESSIBLE_NOISE_LENGTH);
     ZSTD_initCStream_srcSize(zc, 1, CNBufferSize);