From 34646ec99c08124a4a01b80c76535d1b4659ae0e Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Tue, 28 Jul 2026 19:17:38 +0200 Subject: [PATCH 1/2] fix(puffin): reject trailing data in footer payloads Signed-off-by: Minh Vu --- puffin/puffin_reader.go | 10 +++++++++- puffin/puffin_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/puffin/puffin_reader.go b/puffin/puffin_reader.go index 9276747b5..7ce38cfee 100644 --- a/puffin/puffin_reader.go +++ b/puffin/puffin_reader.go @@ -224,11 +224,19 @@ func (r *Reader) readFooter() error { } payloadReader := io.NewSectionReader(r.r, footerStart+MagicSize, payloadSize) + decoder := json.NewDecoder(payloadReader) var footer Footer - if err := json.NewDecoder(payloadReader).Decode(&footer); err != nil { + if err := decoder.Decode(&footer); err != nil { return fmt.Errorf("puffin: decode footer JSON: %w", err) } + var extra json.RawMessage + if err := decoder.Decode(&extra); err == nil { + return errors.New("puffin: footer contains multiple JSON values") + } else if !errors.Is(err, io.EOF) { + return fmt.Errorf("puffin: trailing data after footer JSON: %w", err) + } + // Validate blob metadata if err := r.validateBlobs(footer.Blobs, footerStart); err != nil { return err diff --git a/puffin/puffin_test.go b/puffin/puffin_test.go index 4c186ebb4..76759547a 100644 --- a/puffin/puffin_test.go +++ b/puffin/puffin_test.go @@ -19,6 +19,7 @@ package puffin_test import ( "bytes" + "encoding/binary" "errors" "math" "os" @@ -149,6 +150,15 @@ func validFileWithBlob() []byte { return buf.Bytes() } +func fileWithFooterPayload(payload []byte) []byte { + data := append([]byte("PFA1PFA1"), payload...) + trailer := make([]byte, 12) + binary.LittleEndian.PutUint32(trailer[:4], uint32(len(payload))) + copy(trailer[8:], "PFA1") + + return append(data, trailer...) +} + // --- Tests --- // TestRoundTrip verifies that data written by Writer can be read back by Reader. @@ -580,6 +590,32 @@ func TestReaderInvalidFile(t *testing.T) { }) } +func TestReaderRejectsTrailingFooterData(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + payload string + wantErr string + }{ + {name: "single object", payload: `{"blobs":[]}`}, + {name: "trailing whitespace", payload: "{\"blobs\":[]} \n\t"}, + {name: "second JSON value", payload: `{"blobs":[]}{"blobs":[]}`, wantErr: "multiple JSON values"}, + {name: "trailing garbage", payload: `{"blobs":[]}garbage`, wantErr: "trailing data"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := puffin.NewReader(bytes.NewReader(fileWithFooterPayload([]byte(test.payload)))) + if test.wantErr == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, test.wantErr) + } + }) + } +} + // TestReaderBlobAccess verifies blob access methods work correctly. // Tests the primary API for retrieving blob data from puffin files. func TestReaderBlobAccess(t *testing.T) { From 02e5987cfa635efb9a2cc21f0aa64f11f5502eac Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Tue, 28 Jul 2026 19:21:14 +0200 Subject: [PATCH 2/2] refactor(puffin): detect trailing values without copying Signed-off-by: Minh Vu --- puffin/puffin_reader.go | 3 +-- puffin/puffin_test.go | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/puffin/puffin_reader.go b/puffin/puffin_reader.go index 7ce38cfee..8aed3b23d 100644 --- a/puffin/puffin_reader.go +++ b/puffin/puffin_reader.go @@ -230,8 +230,7 @@ func (r *Reader) readFooter() error { return fmt.Errorf("puffin: decode footer JSON: %w", err) } - var extra json.RawMessage - if err := decoder.Decode(&extra); err == nil { + if _, err := decoder.Token(); err == nil { return errors.New("puffin: footer contains multiple JSON values") } else if !errors.Is(err, io.EOF) { return fmt.Errorf("puffin: trailing data after footer JSON: %w", err) diff --git a/puffin/puffin_test.go b/puffin/puffin_test.go index 76759547a..0bb69bbaa 100644 --- a/puffin/puffin_test.go +++ b/puffin/puffin_test.go @@ -601,6 +601,7 @@ func TestReaderRejectsTrailingFooterData(t *testing.T) { {name: "single object", payload: `{"blobs":[]}`}, {name: "trailing whitespace", payload: "{\"blobs\":[]} \n\t"}, {name: "second JSON value", payload: `{"blobs":[]}{"blobs":[]}`, wantErr: "multiple JSON values"}, + {name: "second scalar value", payload: `{"blobs":[]}42`, wantErr: "multiple JSON values"}, {name: "trailing garbage", payload: `{"blobs":[]}garbage`, wantErr: "trailing data"}, }