zstd: Fix quick reject on long backmatches (#787)

When m.s was extended far back we could read out of bounds. Fixes #786 Re-enables accidentally disabled fuzz tests.
klauspost · Mar 24, 2023 · 4bac43a · 4bac43a
1 parent 31b99fb
commit 4bac43a
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 15 deletions.
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
@@ -35,10 +35,15 @@ jobs:
     - name: Test Noasm
       run: go test -tags=noasm ./...
 
-    - name: Test Race
+    - name: Test Race 1 CPU
       env:
         CGO_ENABLED: 1
-      run: go test -cpu="1,4" -short -race -v ./...
+      run: go test -cpu=1 -short -race -v ./...
+
+    - name: Test Race 4 CPU
+      env:
+        CGO_ENABLED: 1
+      run: go test -cpu=4 -short -race -v ./...
 
   generate:
     strategy:
@@ -163,7 +168,7 @@ jobs:
         run: go test -run=none -fuzz=FuzzNoBMI2Dec -fuzztime=500000x -test.fuzzminimizetime=10ms ./zstd/.
 
       - name: zstd/FuzzEncoding
-        run: go test -run=none -fuzz=FuzzEncoding -fuzztime=250000x -test.fuzzminimizetime=10ms ./zstd/.
+        run: cd zstd&&go test -run=none -fuzz=FuzzEncoding -fuzztime=250000x -test.fuzzminimizetime=10ms -fuzz-end=3&&cd ..
 
       - name: zstd/FuzzDecodeAll/noasm
         run: go test -tags=noasm -run=none -fuzz=FuzzDecodeAll -fuzztime=500000x -test.fuzzminimizetime=10ms ./zstd/.
@@ -172,7 +177,10 @@ jobs:
         run: go test -tags=noasm -run=none -fuzz=FuzzDecoder -fuzztime=500000x -test.fuzzminimizetime=10ms ./zstd/.
 
       - name: zstd/FuzzEncoding/noasm
-        run: go test -tags=noasm -run=none -fuzz=FuzzEncoding -fuzztime=250000x -test.fuzzminimizetime=10ms ./zstd/.
+        run: cd zstd&&go test -tags=noasm -run=none -fuzz=FuzzEncoding -fuzztime=250000x -test.fuzzminimizetime=10ms -fuzz-end=3&&cd ..
+
+      - name: zstd/FuzzEncodingBest
+        run: cd zstd&&go test -run=none -fuzz=FuzzEncoding -fuzztime=25000x -test.fuzzminimizetime=10ms -fuzz-start=4&&cd ..
 
   fuzz-other:
     env:

diff --git a/zstd/enc_best.go b/zstd/enc_best.go
@@ -212,10 +212,11 @@ encodeLoop:
 				if left <= 0 {
 					return
 				}
-				if left > 2 {
+				checkLen := m.length - (s - m.s) - 8
+				if left > 2 && checkLen > 4 {
 					// Check 4 bytes, 4 bytes from the end of the current match.
-					a := load3232(src, offset+m.length-8)
-					b := load3232(src, s+m.length-8)
+					a := load3232(src, offset+checkLen)
+					b := load3232(src, s+checkLen)
 					if a != b {
 						return
 					}

diff --git a/zstd/fuzz_test.go b/zstd/fuzz_test.go
@@ -133,13 +133,10 @@ func FuzzEncoding(f *testing.F) {
 	const (
 		// Test a subset of encoders.
 		startFuzz = SpeedFastest
-		endFuzz   = SpeedBetterCompression
+		endFuzz   = SpeedBestCompression
 
 		// Also tests with dictionaries...
 		testDicts = true
-
-		// Max input size:
-		maxSize = 1 << 20
 	)
 
 	var dec *Decoder
@@ -152,16 +149,20 @@ func FuzzEncoding(f *testing.F) {
 		dicts = readDicts(f, zr)
 	}
 
+	if testing.Short() && *fuzzEndF > int(SpeedBetterCompression) {
+		*fuzzEndF = int(SpeedBetterCompression)
+	}
+
 	initEnc := func() func() {
 		var err error
-		dec, err = NewReader(nil, WithDecoderConcurrency(2), WithDecoderDicts(dicts...), WithDecoderMaxWindow(64<<10), WithDecoderMaxMemory(maxSize))
+		dec, err = NewReader(nil, WithDecoderConcurrency(2), WithDecoderDicts(dicts...), WithDecoderMaxWindow(64<<10), WithDecoderMaxMemory(uint64(*fuzzMaxF)))
 		if err != nil {
 			panic(err)
 		}
 		for level := startFuzz; level <= endFuzz; level++ {
 			encs[level], err = NewWriter(nil, WithEncoderCRC(true), WithEncoderLevel(level), WithEncoderConcurrency(2), WithWindowSize(64<<10), WithZeroFrames(true), WithLowerEncoderMem(true))
 			if testDicts {
-				encsD[level], err = NewWriter(nil, WithEncoderCRC(true), WithEncoderLevel(level), WithEncoderConcurrency(2), WithWindowSize(64<<10), WithZeroFrames(true), WithEncoderDict(dicts[level]), WithLowerEncoderMem(true), WithLowerEncoderMem(true))
+				encsD[level], err = NewWriter(nil, WithEncoderCRC(true), WithEncoderLevel(level), WithEncoderConcurrency(2), WithWindowSize(64<<10), WithZeroFrames(true), WithEncoderDict(dicts[int(level)%len(dicts)]), WithLowerEncoderMem(true), WithLowerEncoderMem(true))
 			}
 		}
 		return func() {
@@ -193,7 +194,7 @@ func FuzzEncoding(f *testing.F) {
 				t.Fatalf("%v:\n%v", r, string(stack))
 			}
 		}()
-		if len(data) > maxSize {
+		if len(data) > *fuzzMaxF {
 			return
 		}
 		var bufSize = len(data)
@@ -205,7 +206,7 @@ func FuzzEncoding(f *testing.F) {
 			}
 		}
 
-		for level := startFuzz; level <= endFuzz; level++ {
+		for level := *fuzzStartF; level <= *fuzzEndF; level++ {
 			enc := encs[level]
 			dst.Reset()
 			enc.Reset(&dst)

diff --git a/zstd/zstd_test.go b/zstd/zstd_test.go
@@ -4,6 +4,7 @@
 package zstd
 
 import (
+	"flag"
 	"fmt"
 	"os"
 	"runtime"
@@ -14,7 +15,13 @@ import (
 
 var isRaceTest bool
 
+// Fuzzing tweaks:
+var fuzzStartF = flag.Int("fuzz-start", int(SpeedFastest), "Start fuzzing at this level")
+var fuzzEndF = flag.Int("fuzz-end", int(SpeedBestCompression), "End fuzzing at this level (inclusive)")
+var fuzzMaxF = flag.Int("fuzz-max", 1<<20, "Maximum input size")
+
 func TestMain(m *testing.M) {
+	flag.Parse()
 	ec := m.Run()
 	if ec == 0 && runtime.NumGoroutine() > 2 {
 		n := 0