From 3588812afd4945050f7e65c79f8541366c1c8ec7 Mon Sep 17 00:00:00 2001
From: Klaus Post <klauspost@gmail.com>
Date: Wed, 8 Mar 2023 04:16:33 -0800
Subject: [PATCH] zstd: Fix ineffective block size check (#771)

When falling back to Go decoding block sizes were not checked correctly.

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56755
---
 zstd/fuzz_test.go                            |  24 +++++++------------
 zstd/seqdec.go                               |   6 +----
 zstd/seqdec_amd64.go                         |   1 -
 zstd/testdata/fuzz/decode-corpus-encoded.zip | Bin 2238161 -> 2238358 bytes
 4 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/zstd/fuzz_test.go b/zstd/fuzz_test.go
index 1099b41ce7..0c8212cc5d 100644
--- a/zstd/fuzz_test.go
+++ b/zstd/fuzz_test.go
@@ -62,17 +62,6 @@ func FuzzDecAllNoBMI2(f *testing.F) {
 func FuzzDecoder(f *testing.F) {
 	fuzz.AddFromZip(f, "testdata/fuzz/decode-corpus-raw.zip", true, testing.Short())
 	fuzz.AddFromZip(f, "testdata/fuzz/decode-corpus-encoded.zip", false, testing.Short())
-	decLow, err := NewReader(nil, WithDecoderLowmem(true), WithDecoderConcurrency(2), WithDecoderMaxMemory(20<<20), WithDecoderMaxWindow(1<<20), IgnoreChecksum(true), WithDecodeBuffersBelow(8<<10))
-	if err != nil {
-		f.Fatal(err)
-	}
-	defer decLow.Close()
-	// Test with high memory, but sync decoding
-	decHi, err := NewReader(nil, WithDecoderLowmem(false), WithDecoderConcurrency(1), WithDecoderMaxMemory(20<<20), WithDecoderMaxWindow(1<<20), IgnoreChecksum(true), WithDecodeBuffersBelow(8<<10))
-	if err != nil {
-		f.Fatal(err)
-	}
-	defer decHi.Close()
 
 	brLow := newBytesReader(nil)
 	brHi := newBytesReader(nil)
@@ -86,14 +75,19 @@ func FuzzDecoder(f *testing.F) {
 		}()
 		brLow.Reset(b)
 		brHi.Reset(b)
-		err := decLow.Reset(brLow)
+		decLow, err := NewReader(brLow, WithDecoderLowmem(true), WithDecoderConcurrency(2), WithDecoderMaxMemory(20<<20), WithDecoderMaxWindow(1<<20), IgnoreChecksum(true), WithDecodeBuffersBelow(8<<10))
 		if err != nil {
-			t.Fatal(err)
+			f.Fatal(err)
 		}
-		err = decHi.Reset(brHi)
+		defer decLow.Close()
+
+		// Test with high memory, but sync decoding
+		decHi, err := NewReader(brHi, WithDecoderLowmem(false), WithDecoderConcurrency(1), WithDecoderMaxMemory(20<<20), WithDecoderMaxWindow(1<<20), IgnoreChecksum(true), WithDecodeBuffersBelow(8<<10))
 		if err != nil {
-			t.Fatal(err)
+			f.Fatal(err)
 		}
+		defer decHi.Close()
+
 		b1, err1 := io.ReadAll(decLow)
 		b2, err2 := io.ReadAll(decHi)
 		if err1 != err2 {
diff --git a/zstd/seqdec.go b/zstd/seqdec.go
index f833d1541f..27fdf90fbc 100644
--- a/zstd/seqdec.go
+++ b/zstd/seqdec.go
@@ -314,9 +314,6 @@ func (s *sequenceDecs) decodeSync(hist []byte) error {
 		}
 		size := ll + ml + len(out)
 		if size-startSize > maxBlockSize {
-			if size-startSize == 424242 {
-				panic("here")
-			}
 			return fmt.Errorf("output bigger than max block size (%d)", maxBlockSize)
 		}
 		if size > cap(out) {
@@ -427,8 +424,7 @@ func (s *sequenceDecs) decodeSync(hist []byte) error {
 		}
 	}
 
-	// Check if space for literals
-	if size := len(s.literals) + len(s.out) - startSize; size > maxBlockSize {
+	if size := len(s.literals) + len(out) - startSize; size > maxBlockSize {
 		return fmt.Errorf("output bigger than max block size (%d)", maxBlockSize)
 	}
 
diff --git a/zstd/seqdec_amd64.go b/zstd/seqdec_amd64.go
index 191384adfd..387a30e99d 100644
--- a/zstd/seqdec_amd64.go
+++ b/zstd/seqdec_amd64.go
@@ -148,7 +148,6 @@ func (s *sequenceDecs) decodeSyncSimple(hist []byte) (bool, error) {
 	s.seqSize += ctx.litRemain
 	if s.seqSize > maxBlockSize {
 		return true, fmt.Errorf("output bigger than max block size (%d)", maxBlockSize)
-
 	}
 	err := br.close()
 	if err != nil {
diff --git a/zstd/testdata/fuzz/decode-corpus-encoded.zip b/zstd/testdata/fuzz/decode-corpus-encoded.zip
index 43a3ddde2f8e3db3333506e6150963bab9721236..bb736425355f61b39d8b5d261193dc7be807f789 100644
GIT binary patch
delta 268
zcmcb3wu5on`3}a07RDB)7UmX~7S<NF7WNj77S0x~7VZ|F7Ty-V7XB827Qq&w7U33=
z7SR^57V#E|7ReT=7U>q57TFfL7Wpj-e>JCn5SC|C<zToFl@Yd7Hoqp4fq|h3hy{Q+
z+0eu&%{a|0(cCgMEiHArU;@9ou@Qq%!W$kn-hh;YPNoK;yS^!id7KgH@J-U<;gxYz
zvf@f8H53(Vdv?x&QF+3G#z|`84DG))6}JD@R6O)2h>=NzL4<*c0lS4jI~f=m6o4d9
b2fFpC0p6@^AQcN)7P9>N!^qI21yTtB_xDOH

delta 106
zcmWN^$qj-)06<X&22@rTc2LISP^E{TNCliJ!o({L5JEj*BJcM4xf9FOi5CJB3yF=x
zfg>keJbVH|BF<d6aw8@oB_pSxq@w1|gXW`t`c138w9j#iZO7ORa**RDj8&_Be^tXF
AdjJ3c