internal/zstd: fix seek offset bounds check in skipFrame

This change enhances the zstd Reader's skipFrame function to validate
the new offset when skipping frames in a seekable stream, preventing
invalid offsets that could occur previously.

A set of "bad" test strings has been added to fuzz_test.go to extend
the robustness checks against potential decompression panics.

Additionally, a new test named TestReaderBad is introduced in
zstd_test.go to verify proper error handling with corrupted input
strings.

The BenchmarkLarge function has also been refactored for clarity,
removing unnecessary timer stops and resets.

Updates #63824

Change-Id: Iccd248756ad6348afa1395c7799350d07402868a
GitHub-Last-Rev: 63055b91e9413491fe8039ea42d55b823c89ec15
GitHub-Pull-Request: golang/go#64056
Reviewed-on: https://go-review.googlesource.com/c/go/+/541220
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Klaus Post <klauspost@gmail.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

3 files changed, 37 insertions, 3 deletions

diff --git a/src/internal/zstd/fuzz_test.go b/src/internal/zstd/fuzz_test.go
index 4c0e9cf7b9..4b5c9961d8 100644
--- a/src/internal/zstd/fuzz_test.go
+++ b/src/internal/zstd/fuzz_test.go
@@ -24,6 +24,7 @@ var badStrings = []string{
 	"(\xb5/\xfd001\x00\x0000000000000000000",
 	"(\xb5/\xfd00\xec\x00\x00&@\x05\x05A7002\x02\x00\x02\x00\x02\x0000000000000000",
 	"(\xb5/\xfd00\xec\x00\x00V@\x05\x0517002\x02\x00\x02\x00\x02\x0000000000000000",
+	"\x50\x2a\x4d\x18\x02\x00\x00\x00",
 }
 
 // This is a simple fuzzer to see if the decompressor panics.
diff --git a/src/internal/zstd/zstd.go b/src/internal/zstd/zstd.go
index 9cf62a6bac..0230076f50 100644
--- a/src/internal/zstd/zstd.go
+++ b/src/internal/zstd/zstd.go
@@ -326,12 +326,34 @@ func (r *Reader) skipFrame() error {
 	relativeOffset += 4
 
 	size := binary.LittleEndian.Uint32(r.scratch[:4])
+	if size == 0 {
+		r.blockOffset += int64(relativeOffset)
+		return nil
+	}
 
 	if seeker, ok := r.r.(io.Seeker); ok {
-		if _, err := seeker.Seek(int64(size), io.SeekCurrent); err != nil {
-			return err
+		r.blockOffset += int64(relativeOffset)
+		// Implementations of Seeker do not always detect invalid offsets,
+		// so check that the new offset is valid by comparing to the end.
+		prev, err := seeker.Seek(0, io.SeekCurrent)
+		if err != nil {
+			return r.wrapError(0, err)
+		}
+		end, err := seeker.Seek(0, io.SeekEnd)
+		if err != nil {
+			return r.wrapError(0, err)
+		}
+		if prev > end-int64(size) {
+			r.blockOffset += end - prev
+			return r.makeEOFError(0)
+		}
+
+		// The new offset is valid, so seek to it.
+		_, err = seeker.Seek(prev+int64(size), io.SeekStart)
+		if err != nil {
+			return r.wrapError(0, err)
 		}
-		r.blockOffset += int64(relativeOffset) + int64(size)
+		r.blockOffset += int64(size)
 		return nil
 	}
 
diff --git a/src/internal/zstd/zstd_test.go b/src/internal/zstd/zstd_test.go
index 4ae6f2b398..f2a2e1b585 100644
--- a/src/internal/zstd/zstd_test.go
+++ b/src/internal/zstd/zstd_test.go
@@ -304,6 +304,17 @@ func TestFileSamples(t *testing.T) {
 	}
 }
 
+func TestReaderBad(t *testing.T) {
+	for i, s := range badStrings {
+		t.Run(fmt.Sprintf("badStrings#%d", i), func(t *testing.T) {
+			_, err := io.Copy(io.Discard, NewReader(strings.NewReader(s)))
+			if err == nil {
+				t.Error("expected error")
+			}
+		})
+	}
+}
+
 func BenchmarkLarge(b *testing.B) {
 	b.StopTimer()
 	b.ReportAllocs()