Merge pull request #1204 from facebook/noForwardProgress

Error code on no forward progress

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);
+    input->pos = (size_t)(ip - (const char*)(input->src));
-    output->pos += (size_t)(op-ostart);
+    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 */

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
 *********************************************************/

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 inSize = (FUZ_rand(&coreSeed) & 15) + 1;
-            size_t const outSize = FUZ_rand(&coreSeed) & 15;
+            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);