Merge pull request #233 from julienlau/julienlau/fix/stale-metadata-leaks

Fix stale metadata in legacy xl.meta headers and scanner responses.

Keep scanner response resets local and cover reused legacy headers and both scanner response variants.

Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/storage-rest-server.go

@@ -290,6 +290,9 @@ func (s *storageRESTServer) NSScannerHandler(ctx context.Context, params *nsScan
 		for update := range updates {
 			resp := storageNSScannerRPC.NewResponse()
 			resp.Update = &update
+			// Pooled responses may carry fields from a previous
+			// scan, always clear the fields not being set.
+			resp.Final = nil
 			out <- resp
 		}
 	}()
@@ -301,6 +304,7 @@ func (s *storageRESTServer) NSScannerHandler(ctx context.Context, params *nsScan
 	// Send final response.
 	resp := storageNSScannerRPC.NewResponse()
 	resp.Final = &ui
+	resp.Update = nil
 	out <- resp
 	return nil
 }

cmd/storage-rest-server_test.go

@@ -0,0 +1,94 @@
+// Copyright (c) 2026 Julien Laurenceau and Feng Ruohang
+//
+// This file is part of Silo Object Storage.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package cmd
+
+import (
+	"context"
+	"testing"
+
+	"github.com/minio/madmin-go/v3"
+	"github.com/minio/minio/internal/grid"
+)
+
+type scannerResponseTestDisk struct {
+	StorageAPI
+}
+
+func (scannerResponseTestDisk) NSScanner(_ context.Context, cache dataUsageCache, updates chan<- dataUsageEntry, _ madmin.HealScanMode, _ func() bool) (dataUsageCache, error) {
+	defer close(updates)
+	updates <- dataUsageEntry{Size: int64(cache.Info.NextCycle)}
+	cache.Info.NextCycle++
+	return cache, nil
+}
+
+func TestNSScannerHandlerClearsStaleResponses(t *testing.T) {
+	endpoint, err := NewEndpoint(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	globalLocalDrivesMu.Lock()
+	oldDrives, oldSets := globalLocalDrivesMap, globalLocalSetDrives
+	globalLocalDrivesMap = map[string]StorageAPI{endpoint.String(): scannerResponseTestDisk{}}
+	globalLocalSetDrives = nil
+	globalLocalDrivesMu.Unlock()
+	t.Cleanup(func() {
+		globalLocalDrivesMu.Lock()
+		globalLocalDrivesMap, globalLocalSetDrives = oldDrives, oldSets
+		globalLocalDrivesMu.Unlock()
+	})
+
+	oldRPC := storageNSScannerRPC
+	t.Cleanup(func() { storageNSScannerRPC = oldRPC })
+	// Every new allocation is dirty too: the check must not depend on
+	// sync.Pool returning a previously used object.
+	storageNSScannerRPC = grid.NewStream[*nsScannerOptions, grid.NoPayload, *nsScannerResp](grid.HandlerNSScanner,
+		func() *nsScannerOptions { return &nsScannerOptions{} }, nil,
+		func() *nsScannerResp {
+			return &nsScannerResp{Update: &dataUsageEntry{Size: -1}, Final: &dataUsageCache{}}
+		})
+	server := storageRESTServer{endpoint: endpoint}
+	for cycle := uint32(1); cycle <= 3; cycle++ {
+		out := make(chan *nsScannerResp, 2)
+		params := &nsScannerOptions{Cache: &dataUsageCache{Info: dataUsageCacheInfo{Name: "bucket", NextCycle: cycle}}}
+		if err := server.NSScannerHandler(t.Context(), params, out); err != nil {
+			t.Fatal(err)
+		}
+		if len(out) != 2 {
+			t.Fatalf("cycle %d: got %d responses, want an update and a final cache", cycle, len(out))
+		}
+		for i := 0; i < 2; i++ {
+			resp := <-out
+			wire, err := resp.MarshalMsg(nil)
+			storageNSScannerRPC.PutResponse(resp)
+			if err != nil {
+				t.Fatal(err)
+			}
+			var decoded nsScannerResp
+			if _, err := decoded.UnmarshalMsg(wire); err != nil {
+				t.Fatal(err)
+			}
+			if i == 0 {
+				if decoded.Update == nil || decoded.Final != nil || decoded.Update.Size != int64(cycle) {
+					t.Errorf("cycle %d: invalid update response: %+v", cycle, decoded)
+				}
+			} else if decoded.Update != nil || decoded.Final == nil || decoded.Final.Info.NextCycle != cycle+1 {
+				t.Errorf("cycle %d: invalid final response: %+v", cycle, decoded)
+			}
+		}
+	}
+}

cmd/xl-storage-format-v2-legacy.go

@@ -40,6 +40,10 @@ func (x *xlMetaV2VersionHeader) unmarshalV(v uint8, bts []byte) (o []byte, err e
 
 // unmarshalV1 decodes version 1, never released.
 func (x *xlMetaV2VersionHeader) unmarshalV1(bts []byte) (o []byte, err error) {
+	// Fields absent from the V1 wire format must be reset since the
+	// header may be reused across decodes.
+	x.Signature = [4]byte{}
+	x.EcN, x.EcM = 0, 0
 	var zb0001 uint32
 	zb0001, bts, err = msgp.ReadArrayHeaderBytes(bts)
 	if err != nil {
@@ -134,7 +138,7 @@ type xlMetaV2VersionHeaderV2 struct {
 
 // UnmarshalMsg implements msgp.Unmarshaler
 func (z *xlMetaV2VersionHeaderV2) UnmarshalMsg(bts []byte) (o []byte, err error) {
-	z.EcN, z.EcN = 0, 0
+	z.EcN, z.EcM = 0, 0
 	var zb0001 uint32
 	zb0001, bts, err = msgp.ReadArrayHeaderBytes(bts)
 	if err != nil {
@@ -184,7 +188,7 @@ func (z *xlMetaV2VersionHeaderV2) UnmarshalMsg(bts []byte) (o []byte, err error)
 
 // DecodeMsg implements msgp.Decodable
 func (z *xlMetaV2VersionHeaderV2) DecodeMsg(dc *msgp.Reader) (err error) {
-	z.EcN, z.EcN = 0, 0
+	z.EcN, z.EcM = 0, 0
 	var zb0001 uint32
 	zb0001, err = dc.ReadArrayHeader()
 	if err != nil {

cmd/xl-storage-format-v2_test.go

@@ -38,6 +38,7 @@ import (
 	"github.com/klauspost/compress/zstd"
 	"github.com/minio/minio/internal/bucket/lifecycle"
 	xhttp "github.com/minio/minio/internal/http"
+	"github.com/tinylib/msgp/msgp"
 )
 
 func TestReadXLMetaNoData(t *testing.T) {
@@ -1190,3 +1191,92 @@ func BenchmarkToFileInfoNoParts(b *testing.B) {
 func BenchmarkToFileInfoWithParts(b *testing.B) {
 	benchmarkManyPartsOptionally(b, true)
 }
+
+// TestXLMetaV2HeaderLegacyUnmarshalResetsEc checks that decoding legacy
+// version headers, which carry no erasure metadata fields, cannot leak
+// stale EcN/EcM values from a previously decoded object into the same
+// structure.
+func TestXLMetaV2HeaderLegacyUnmarshalResetsEc(t *testing.T) {
+	base := xlMetaV2VersionHeader{
+		VersionID: [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16},
+		ModTime:   time.Now().Unix(),
+		Signature: [4]byte{1, 2, 3, 4},
+		Type:      ObjectType,
+		Flags:     xlFlagInlineData,
+	}
+
+	// Legacy V2 header wire format: 5 fields, no EcN/EcM.
+	var legacy []byte
+	legacy = msgp.AppendArrayHeader(legacy, 5)
+	legacy = msgp.AppendBytes(legacy, base.VersionID[:])
+	legacy = msgp.AppendInt64(legacy, base.ModTime)
+	legacy = msgp.AppendBytes(legacy, base.Signature[:])
+	legacy = msgp.AppendUint8(legacy, uint8(base.Type))
+	legacy = msgp.AppendUint8(legacy, uint8(base.Flags))
+
+	for _, decoder := range []string{"UnmarshalMsg", "DecodeMsg", "loadIndexed"} {
+		t.Run(decoder, func(t *testing.T) {
+			header := base
+			header.EcN, header.EcM = 4, 8
+			x2 := xlMetaV2VersionHeaderV2{xlMetaV2VersionHeader: &header}
+			var err error
+			switch decoder {
+			case "UnmarshalMsg":
+				_, err = x2.UnmarshalMsg(legacy)
+			case "DecodeMsg":
+				err = x2.DecodeMsg(msgp.NewReader(bytes.NewReader(legacy)))
+			case "loadIndexed":
+				indexed := msgp.AppendUint(nil, 2)
+				indexed = msgp.AppendUint(indexed, xlMetaVersion)
+				indexed = msgp.AppendInt(indexed, 1)
+				indexed = msgp.AppendBytes(indexed, legacy)
+				indexed = msgp.AppendBytes(indexed, nil)
+				xl := xlMetaV2{versions: []xlMetaV2ShallowVersion{{header: header}}}
+				err = xl.loadIndexed(indexed, nil)
+				header = xl.versions[0].header
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			if header != base {
+				t.Fatalf("legacy header: got %+v, want %+v", header, base)
+			}
+		})
+	}
+
+	// V1 headers (4 fields, never released) must zero the fields too.
+	var v1 []byte
+	v1 = msgp.AppendArrayHeader(v1, 4)
+	v1 = msgp.AppendBytes(v1, base.VersionID[:])
+	v1 = msgp.AppendInt64(v1, base.ModTime)
+	v1 = msgp.AppendUint8(v1, uint8(base.Type))
+	v1 = msgp.AppendUint8(v1, uint8(base.Flags))
+
+	header := base
+	header.EcN, header.EcM = 4, 8
+	header.Signature = [4]byte{9, 9, 9, 9}
+	if _, err := header.unmarshalV1(v1); err != nil {
+		t.Fatalf("legacy V1 header unmarshal failed: %v", err)
+	}
+	if header.EcN != 0 || header.EcM != 0 {
+		t.Fatalf("stale erasure metadata leaked on V1 decode: EcN %d, EcM %d", header.EcN, header.EcM)
+	}
+	if header.Signature != ([4]byte{}) {
+		t.Fatalf("stale signature leaked on V1 decode: %v", header.Signature)
+	}
+
+	// Current headers must keep their erasure metadata.
+	current := base
+	current.EcN, current.EcM = 4, 8
+	wire, err := current.MarshalMsg(nil)
+	if err != nil {
+		t.Fatalf("header marshal failed: %v", err)
+	}
+	decoded := base
+	if _, err := decoded.UnmarshalMsg(wire); err != nil {
+		t.Fatalf("header unmarshal failed: %v", err)
+	}
+	if decoded.EcN != 4 || decoded.EcM != 8 {
+		t.Fatalf("erasure metadata lost on current decode: EcN %d, EcM %d", decoded.EcN, decoded.EcM)
+	}
+}