Skip to content

Commit 9960386

Browse files
authored
zstd: Validate SnappyConverter literal copies (#1170)
Enable disabled length check on `SnappyConverter.Convert` that could result in out-of-bounds reads buffer reads - or in case or re-use return values from the previous run. Reported by @mfazrinizar
1 parent e29d4d3 commit 9960386

2 files changed

Lines changed: 44 additions & 3 deletions

File tree

zstd/snappy.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,10 @@ func decodeSnappy(blk *blockEnc, src []byte) error {
334334

335335
return errUnsupportedLiteralLength
336336
}
337-
//if length > snappyMaxBlockSize-d || uint32(length) > len(src)-s {
338-
// return ErrSnappyCorrupt
339-
//}
337+
if length > len(src)-s {
338+
println("length > len(src)-s", length, len(src)-s)
339+
return ErrSnappyCorrupt
340+
}
340341

341342
blk.literals = append(blk.literals, src[s:s+length]...)
342343
//println(length, "litLen")

zstd/snappy_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,46 @@ func TestSnappy_ConvertEnwik9(t *testing.T) {
210210
t.Log("Encoded content matched")
211211
}
212212

213+
// TestSnappy_ConvertMalformedLiteralStale verifies that a malformed compressed
214+
// chunk declaring a literal longer than the bytes present in the chunk is
215+
// rejected as corrupt, instead of reading stale bytes from the reused r.buf.
216+
func TestSnappy_ConvertMalformedLiteralStale(t *testing.T) {
217+
framedCompressedBlock := func(payload []byte) []byte {
218+
stream := []byte{0xff, 0x06, 0x00, 0x00, 's', 'N', 'a', 'P', 'p', 'Y'}
219+
chunkLen := snappyChecksumSize + len(payload)
220+
stream = append(stream, 0x00, byte(chunkLen), byte(chunkLen>>8), byte(chunkLen>>16))
221+
stream = append(stream, 0x00, 0x00, 0x00, 0x00) // checksum, not verified for compressed chunks
222+
return append(stream, payload...)
223+
}
224+
convert := func(conv *SnappyConverter, stream []byte) ([]byte, error) {
225+
var comp bytes.Buffer
226+
if _, err := conv.Convert(bytes.NewReader(stream), &comp); err != nil && err != io.EOF {
227+
return nil, err
228+
}
229+
dec, err := NewReader(nil)
230+
if err != nil {
231+
t.Fatal(err)
232+
}
233+
defer dec.Close()
234+
return dec.DecodeAll(comp.Bytes(), nil)
235+
}
236+
237+
var conv SnappyConverter
238+
239+
// Valid block: decoded length 4, 4-byte literal "PREV".
240+
first := []byte{0x04, 0x0c, 'P', 'R', 'E', 'V'}
241+
if _, err := convert(&conv, framedCompressedBlock(first)); err != nil {
242+
t.Fatal(err)
243+
}
244+
245+
// Malformed block: declares a 4-byte literal but supplies none.
246+
// Must be rejected instead of returning "PREV" from the reused buffer.
247+
malformed := []byte{0x04, 0x0c}
248+
if _, err := convert(&conv, framedCompressedBlock(malformed)); err != ErrSnappyCorrupt {
249+
t.Fatalf("expected ErrSnappyCorrupt, got %v", err)
250+
}
251+
}
252+
213253
func BenchmarkSnappy_ConvertXML(b *testing.B) {
214254
f, err := os.Open("testdata/xml.zst")
215255
if err != nil {

0 commit comments

Comments
 (0)