diff --git a/backend/riff/riff.go b/backend/riff/riff.go index e60d265..60e2b79 100644 --- a/backend/riff/riff.go +++ b/backend/riff/riff.go @@ -63,12 +63,15 @@ func Parse(r io.Reader) ([]Chunk, error) { return nil, err } - data := make([]byte, size) - if _, err := io.ReadFull(r, data); err != nil { + // Copied rather than allocated up front, as ID3Chunk does: the + // size is four bytes off the file, so a truncated one is free to + // declare a chunk larger than the whole of itself. + var data bytes.Buffer + if _, err := io.CopyN(&data, r, int64(size)); err != nil { return nil, fmt.Errorf("read chunk data for %q: %w", id, err) } - chunks = append(chunks, Chunk{ID: id, Data: data}) + chunks = append(chunks, Chunk{ID: id, Data: data.Bytes()}) // Odd-length chunks have a padding byte. Lenient: if the // read fails (e.g. EOF), just break rather than error. diff --git a/backend/riff/riff_test.go b/backend/riff/riff_test.go index 19295ff..3c9da6a 100644 --- a/backend/riff/riff_test.go +++ b/backend/riff/riff_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/binary" "errors" + "runtime" "testing" "yellowjacket/backend/riff" @@ -209,3 +210,45 @@ func TestParse_ReadsEveryChunkInOrder(t *testing.T) { t.Errorf("odd chunk data: got %q, want %q", chunks[1].Data, "INFOodd") } } + +// A chunk size is four bytes read off the file, so a truncated or +// malformed WAV is free to declare a chunk larger than the whole of +// itself. Parse must grow with what arrives rather than with what was +// claimed. +// +// This measures the allocation instead of the error because the error +// is the same either way: a build sizing its buffer from the header +// reports the truncation correctly, having asked the allocator for a +// gigabyte on the way. Deliberately not parallel — TotalAlloc is +// process-wide, and a test paused beside another one is measuring it +// too. +func TestParse_DoesNotAllocateWhatAChunkClaims(t *testing.T) { + // Large enough that a header-sized buffer is unmistakable, in a + // container of a few dozen bytes. + const declared = 1 << 30 + + var raw bytes.Buffer + + raw.WriteString("RIFF") + _ = binary.Write(&raw, binary.LittleEndian, uint32(declared+12)) + raw.WriteString("WAVE") + raw.WriteString("data") + _ = binary.Write(&raw, binary.LittleEndian, uint32(declared)) + raw.WriteString("and then the file ends") + + var before, after runtime.MemStats + + runtime.GC() + runtime.ReadMemStats(&before) + + if _, err := riff.Parse(bytes.NewReader(raw.Bytes())); err == nil { + t.Fatal("Parse: got nil error for a chunk larger than the file holding it") + } + + runtime.ReadMemStats(&after) + + if grew := after.TotalAlloc - before.TotalAlloc; grew > 1<<20 { + t.Errorf("Parse allocated %d bytes reading a %d-byte file whose chunk header claimed %d", + grew, raw.Len(), declared) + } +}