]> git.ipfire.org Git - thirdparty/zstd.git/commitdiff
fix one UB pointer arithmetic in encoder
authorYann Collet <cyan@fb.com>
Fri, 17 Nov 2017 20:55:37 +0000 (12:55 -0800)
committerYann Collet <cyan@fb.com>
Fri, 17 Nov 2017 21:24:51 +0000 (13:24 -0800)
Instead of calculating distance between 2 memory objects, which is UB,
we extract the offset from object 1, and transfer it into object 2.

lib/compress/zstd_compress.c
lib/decompress/zstd_decompress.c
tests/Makefile

index f0cebe7a39665f0b8f32c9c62bc61d68aa427e72..858eaca301a12c1b68b5d6f9a3c391508d0de497 100644 (file)
@@ -1819,17 +1819,21 @@ static size_t ZSTD_compressContinue_internal (ZSTD_CCtx* cctx,
         cctx->stage = ZSTDcs_ongoing;
     }
 
+    if (!srcSize) return fhSize;  /* do not generate an empty block if no input */
+
     /* Check if blocks follow each other */
     if (src != cctx->nextSrc) {
         /* not contiguous */
-        ptrdiff_t const delta = cctx->nextSrc - ip;
+        size_t const distanceFromBase = (size_t)(cctx->nextSrc - cctx->base);
         cctx->lowLimit = cctx->dictLimit;
-        cctx->dictLimit = (U32)(cctx->nextSrc - cctx->base);
+        assert(distanceFromBase == (size_t)(U32)distanceFromBase);  /* should never overflow */
+        cctx->dictLimit = (U32)distanceFromBase;
         cctx->dictBase = cctx->base;
-        cctx->base -= delta;
+        cctx->base = ip - distanceFromBase;
         cctx->nextToUpdate = cctx->dictLimit;
         if (cctx->dictLimit - cctx->lowLimit < HASH_READ_SIZE) cctx->lowLimit = cctx->dictLimit;   /* too small extDict */
     }
+    cctx->nextSrc = ip + srcSize;
 
     /* if input and dictionary overlap : reduce dictionary (area presumed modified by input) */
     if ((ip+srcSize > cctx->dictBase + cctx->lowLimit) & (ip < cctx->dictBase + cctx->dictLimit)) {
@@ -1838,17 +1842,13 @@ static size_t ZSTD_compressContinue_internal (ZSTD_CCtx* cctx,
         cctx->lowLimit = lowLimitMax;
     }
 
-    cctx->nextSrc = ip + srcSize;
-
-    if (srcSize) {
-        size_t const cSize = frame ?
+    {   size_t const cSize = frame ?
                              ZSTD_compress_frameChunk (cctx, dst, dstCapacity, src, srcSize, lastFrameChunk) :
                              ZSTD_compressBlock_internal (cctx, dst, dstCapacity, src, srcSize);
         if (ZSTD_isError(cSize)) return cSize;
         cctx->consumedSrcSize += srcSize;
         return cSize + fhSize;
-    } else
-        return fhSize;
+    }
 }
 
 size_t ZSTD_compressContinue (ZSTD_CCtx* cctx,
index 4ded3656f64f428a5198a659516a631074f93057..f0cf967cb7b631e4b2f3c0f1a6cd2143bbb2cead 100644 (file)
@@ -1225,7 +1225,8 @@ seq_t ZSTD_decodeSequenceLong(seqState_t* seqState, ZSTD_longOffset_e const long
 
     {   size_t const pos = seqState->pos + seq.litLength;
         const BYTE* const matchBase = (seq.offset > pos) ? seqState->dictEnd : seqState->prefixStart;
-        seq.match = matchBase + pos - seq.offset;
+        seq.match = matchBase + pos - seq.offset;  /* note : this operation can overflow when seq.offset is really too large, which can only happen when input is corrupted.
+                                                    * No consequence though : no memory access will occur, overly large offset will be detected in ZSTD_execSequenceLong() */
         seqState->pos = pos + seq.matchLength;
     }
 
@@ -1372,7 +1373,7 @@ static size_t ZSTD_decompressSequencesLong(
             seq_t const sequence = ZSTD_decodeSequenceLong(&seqState, isLongOffset);
             size_t const oneSeqSize = ZSTD_execSequenceLong(op, oend, sequences[(seqNb-ADVANCED_SEQS) & STOSEQ_MASK], &litPtr, litEnd, prefixStart, dictStart, dictEnd);
             if (ZSTD_isError(oneSeqSize)) return oneSeqSize;
-            PREFETCH(sequence.match);
+            PREFETCH(sequence.match);  /* note : it's safe to invoke PREFETCH() on any memory address, including invalid ones */
             sequences[seqNb&STOSEQ_MASK] = sequence;
             op += oneSeqSize;
         }
index eb8bb3dd6b9d0d29cb10cbd28bb526a9751a8303..11d55a75d4dbbc98862a4ef76cddb1193c0228ef 100644 (file)
@@ -131,14 +131,11 @@ zbufftest-dll : $(ZSTDDIR)/common/xxhash.c $(PRGDIR)/datagen.c zbufftest.c
        $(CC) $(CPPFLAGS) $(CFLAGS) $^ $(LDFLAGS) -o $@$(EXT)
 
 ZSTREAMFILES := $(ZSTD_FILES) $(ZDICT_FILES) $(PRGDIR)/datagen.c seqgen.c zstreamtest.c
-zstreamtest : CPPFLAGS += $(MULTITHREAD_CPP)
-zstreamtest : LDFLAGS += $(MULTITHREAD_LD)
-zstreamtest : $(ZSTREAMFILES)
-       $(CC) $(FLAGS) $^ -o $@$(EXT)
-
 zstreamtest32 : CFLAGS += -m32
-zstreamtest32 : $(ZSTREAMFILES)
-       $(CC) $(FLAGS) $(MULTITHREAD) $^ -o $@$(EXT)
+zstreamtest zstreamtest32 : CPPFLAGS += $(MULTITHREAD_CPP)
+zstreamtest zstreamtest32 : LDFLAGS += $(MULTITHREAD_LD)
+zstreamtest zstreamtest32 : $(ZSTREAMFILES)
+       $(CC) $(FLAGS) $^ -o $@$(EXT)
 
 zstreamtest_asan : CFLAGS += -fsanitize=address
 zstreamtest_asan : $(ZSTREAMFILES)