internal/zstd: fix seek offset bounds check in skipFrame

A bug where seeking beyond the end of strings.Reader or bytes.Reader
instances did not return an error is now fixed. The Reader.skipFrame
method has been adjusted to handle zero-sized frames without performing
redundant operations, improving efficiency and correctness.

The change involves a pre-check for size == 0 to update blockOffset
and return early. For non-zero sizes, it carefully seeks one byte less
than the size before manually incrementing relativeOffset to ensure
an EOF is caught if the seek went past the object's bounds.

This behavior aligns with expectations that an out-of-bounds seek should
result in an error, preventing potential silent failures in decompression
operations which could lead to unexpected behavior or security issues.

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.

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 {
+		r.blockOffset += int64(relativeOffset)
-			return err
+		// 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)
 		}
-		r.blockOffset += int64(relativeOffset) + int64(size)
+		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(size)
 		return nil
 	}
 

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()