volume: validate the file extension in CopyFile and ReceiveFile (#10644)

* volume: validate the file extension in CopyFile and ReceiveFile

CopyFile and ReceiveFile build an on-disk path from the client-supplied
Ext. Both are intentionally ungated for cluster-internal peers, so a
value like "/../../x" is joined onto the volume directory and, once
path-cleaned, resolves outside it -- an EC-shard receive can then write,
and CopyFile read, anywhere the process can reach.

Constrain Ext to a real suffix (a leading dot followed by alphanumerics)
before it is used to build any path, so it can no longer carry a
separator or a parent reference.

* test: use an alphanumeric missing-file extension in the copy variants

The not-found and stop-offset-zero cases used ".definitely-missing" as a
deliberately absent source. The extension is now validated, and the hyphen
makes it invalid, so switch to ".missing" -- still a nonexistent file, but a
real extension shape.

* volume: validate the collection in CopyFile and ReceiveFile

The client-supplied Collection is folded into the on-disk path as
"<collection>_<vid>" by VolumeFileName and EcShardBaseFileName, both joined
with path.Join / util.Join. A Collection carrying a separator, e.g.
"../../x", therefore path-cleans to a target outside the volume directory,
the same escape the extension check just closed. Reject a collection that is
a bare parent reference or holds a separator; ordinary names ('.', '-' and
all) still pass.

test/volume_server/grpc/copy_receive_variants_test.go

@@ -90,7 +90,7 @@ func TestCopyFileIgnoreNotFoundAndStopOffsetZeroPaths(t *testing.T) {
 
 	missingNoIgnore, err := grpcClient.CopyFile(ctx, &volume_server_pb.CopyFileRequest{
 		VolumeId:                 volumeID,
-		Ext:                      ".definitely-missing",
+		Ext:                      ".missing",
 		CompactionRevision:       math.MaxUint32,
 		StopOffset:               1,
 		IgnoreSourceFileNotFound: false,
@@ -104,7 +104,7 @@ func TestCopyFileIgnoreNotFoundAndStopOffsetZeroPaths(t *testing.T) {
 
 	missingIgnored, err := grpcClient.CopyFile(ctx, &volume_server_pb.CopyFileRequest{
 		VolumeId:                 volumeID,
-		Ext:                      ".definitely-missing",
+		Ext:                      ".missing",
 		CompactionRevision:       math.MaxUint32,
 		StopOffset:               1,
 		IgnoreSourceFileNotFound: true,
@@ -119,7 +119,7 @@ func TestCopyFileIgnoreNotFoundAndStopOffsetZeroPaths(t *testing.T) {
 
 	stopZeroStream, err := grpcClient.CopyFile(ctx, &volume_server_pb.CopyFileRequest{
 		VolumeId:                 volumeID,
-		Ext:                      ".definitely-missing",
+		Ext:                      ".missing",
 		CompactionRevision:       math.MaxUint32,
 		StopOffset:               0,
 		IgnoreSourceFileNotFound: false,

weed/server/volume_grpc_copy.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"math"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/seaweedfs/seaweedfs/weed/pb/master_pb"
@@ -480,11 +481,52 @@ func (vs *VolumeServer) ReadVolumeFileStatus(ctx context.Context, req *volume_se
 	return resp, nil
 }
 
+// checkVolumeFileExtension guards the client-supplied Ext that CopyFile and
+// ReceiveFile turn into an on-disk path. Both RPCs are intentionally ungated
+// for cluster-internal peers (see volume_grpc_admin_auth_coverage_test.go), so
+// this is the only check standing between a peer request and the os.Open /
+// os.Create target: without it an Ext like "/../../x" is joined onto the volume
+// directory and, once path-cleaned, resolves outside it. A genuine extension is
+// a leading dot followed by alphanumerics -- ".dat", ".idx", ".vif", ".ecx",
+// ".ecj", ".ecsum", ".ec00".. -- and never contains a separator or "..".
+func checkVolumeFileExtension(ext string) error {
+	if len(ext) < 2 || ext[0] != '.' {
+		return fmt.Errorf("invalid file extension %q", ext)
+	}
+	for _, r := range ext[1:] {
+		if r < '0' || (r > '9' && r < 'A') || (r > 'Z' && r < 'a') || r > 'z' {
+			return fmt.Errorf("invalid file extension %q", ext)
+		}
+	}
+	return nil
+}
+
+// checkVolumeCollection guards the client-supplied Collection, which CopyFile
+// and ReceiveFile fold into a path component ("<collection>_<vid>"). An empty
+// collection is the default; any other value must be a single path element so a
+// collection like "../../x" cannot climb out of the volume directory once
+// path-cleaned. Collection names are user-facing and may hold '.' or '-', so
+// this rejects only separators and bare parent references rather than the
+// stricter alphanumeric rule used for extensions.
+func checkVolumeCollection(collection string) error {
+	if collection == "." || collection == ".." || strings.ContainsAny(collection, `/\`) {
+		return fmt.Errorf("invalid collection %q", collection)
+	}
+	return nil
+}
+
 // CopyFile client pulls the volume related file from the source server.
 // if req.CompactionRevision != math.MaxUint32, it ensures the compact revision is as expected
 // The copying still stop at req.StopOffset, but you can set it to math.MaxUint64 in order to read all data.
 func (vs *VolumeServer) CopyFile(req *volume_server_pb.CopyFileRequest, stream volume_server_pb.VolumeServer_CopyFileServer) error {
 
+	if err := checkVolumeFileExtension(req.Ext); err != nil {
+		return err
+	}
+	if err := checkVolumeCollection(req.Collection); err != nil {
+		return err
+	}
+
 	var fileName string
 	if !req.IsEcVolume {
 		v := vs.store.GetVolume(needle.VolumeId(req.VolumeId))
@@ -652,6 +694,19 @@ func (vs *VolumeServer) ReceiveFile(stream volume_server_pb.VolumeServer_Receive
 			glog.V(1).Infof("ReceiveFile: volume %d, ext %s, collection %s, shard %d, size %d",
 				fileInfo.VolumeId, fileInfo.Ext, fileInfo.Collection, fileInfo.ShardId, fileInfo.FileSize)
 
+			if err := checkVolumeFileExtension(fileInfo.Ext); err != nil {
+				glog.Errorf("ReceiveFile: %v", err)
+				return stream.SendAndClose(&volume_server_pb.ReceiveFileResponse{
+					Error: err.Error(),
+				})
+			}
+			if err := checkVolumeCollection(fileInfo.Collection); err != nil {
+				glog.Errorf("ReceiveFile: %v", err)
+				return stream.SendAndClose(&volume_server_pb.ReceiveFileResponse{
+					Error: err.Error(),
+				})
+			}
+
 			if fileInfo.IsEcVolume {
 				// os.Create below truncates in place; a mounted EcVolume
 				// holds fds on the same inodes, so overwriting corrupts

weed/server/volume_grpc_copy_traversal_test.go

@@ -0,0 +1,212 @@
+package weed_server
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb"
+	"github.com/seaweedfs/seaweedfs/weed/stats"
+	"github.com/seaweedfs/seaweedfs/weed/storage"
+	"github.com/seaweedfs/seaweedfs/weed/storage/types"
+	"github.com/seaweedfs/seaweedfs/weed/util"
+)
+
+// newTraversalTestStore builds a single-location store rooted at dir.
+func newTraversalTestStore(dir string) *storage.Store {
+	return storage.NewStore(
+		grpc.WithTransportCredentials(insecure.NewCredentials()),
+		"127.0.0.1", 0, 0, "", "test-store",
+		[]string{dir}, []int32{10}, []util.MinFreeSpace{{}},
+		dir, storage.NeedleMapInMemory,
+		[]types.DiskType{types.HardDriveType}, [][]string{nil},
+		0, stats.DiskIOProbeConfig{},
+	)
+}
+
+// fakeReceiveFileStream scripts a ReceiveFile request sequence and records the
+// final response returned via SendAndClose.
+type fakeReceiveFileStream struct {
+	grpc.ServerStream
+	reqs  []*volume_server_pb.ReceiveFileRequest
+	index int
+	resp  *volume_server_pb.ReceiveFileResponse
+}
+
+func (s *fakeReceiveFileStream) Recv() (*volume_server_pb.ReceiveFileRequest, error) {
+	if s.index >= len(s.reqs) {
+		return nil, io.EOF
+	}
+	r := s.reqs[s.index]
+	s.index++
+	return r, nil
+}
+
+func (s *fakeReceiveFileStream) SendAndClose(resp *volume_server_pb.ReceiveFileResponse) error {
+	s.resp = resp
+	return nil
+}
+
+func infoReq(info *volume_server_pb.ReceiveFileInfo) *volume_server_pb.ReceiveFileRequest {
+	return &volume_server_pb.ReceiveFileRequest{Data: &volume_server_pb.ReceiveFileRequest_Info{Info: info}}
+}
+
+func contentReq(b []byte) *volume_server_pb.ReceiveFileRequest {
+	return &volume_server_pb.ReceiveFileRequest{Data: &volume_server_pb.ReceiveFileRequest_FileContent{FileContent: b}}
+}
+
+// TestReceiveFile_RejectsTraversalExt ensures a client-supplied Ext with parent
+// references cannot steer the EC-shard write outside the volume directory. The
+// EC branch joins the Ext with util.Join, which path-cleans the ".." away
+// before os.Create, so a bare concatenation check would miss it.
+func TestReceiveFile_RejectsTraversalExt(t *testing.T) {
+	root := t.TempDir()
+	storeDir := filepath.Join(root, "a", "b", "store")
+	if err := os.MkdirAll(storeDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	vs := &VolumeServer{store: newTraversalTestStore(storeDir)}
+
+	stream := &fakeReceiveFileStream{reqs: []*volume_server_pb.ReceiveFileRequest{
+		infoReq(&volume_server_pb.ReceiveFileInfo{
+			VolumeId:   4,
+			Ext:        "/../../pwned.ec00",
+			IsEcVolume: true,
+			FileSize:   5,
+		}),
+		contentReq([]byte("pwned")),
+	}}
+
+	if err := vs.ReceiveFile(stream); err != nil {
+		t.Fatalf("ReceiveFile returned transport error: %v", err)
+	}
+	if stream.resp == nil || stream.resp.Error == "" {
+		t.Errorf("traversal ext was accepted; response = %+v", stream.resp)
+	}
+
+	assertNoEscapedFile(t, root, storeDir, "pwned")
+}
+
+// TestReceiveFile_RejectsTraversalCollection covers the sibling vector: the
+// Collection is folded into the path as "<collection>_<vid>", so a value with
+// a separator escapes the volume directory the same way a traversal ext does.
+func TestReceiveFile_RejectsTraversalCollection(t *testing.T) {
+	root := t.TempDir()
+	storeDir := filepath.Join(root, "a", "b", "store")
+	if err := os.MkdirAll(storeDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	vs := &VolumeServer{store: newTraversalTestStore(storeDir)}
+
+	stream := &fakeReceiveFileStream{reqs: []*volume_server_pb.ReceiveFileRequest{
+		infoReq(&volume_server_pb.ReceiveFileInfo{
+			VolumeId:   4,
+			Ext:        ".ec00",
+			Collection: "../../pwned",
+			IsEcVolume: true,
+			FileSize:   5,
+		}),
+		contentReq([]byte("pwned")),
+	}}
+
+	if err := vs.ReceiveFile(stream); err != nil {
+		t.Fatalf("ReceiveFile returned transport error: %v", err)
+	}
+	if stream.resp == nil || stream.resp.Error == "" {
+		t.Errorf("traversal collection was accepted; response = %+v", stream.resp)
+	}
+	assertNoEscapedFile(t, root, storeDir, "pwned")
+}
+
+// TestReceiveFile_AcceptsNormalExt is the positive control: a legitimate EC
+// shard extension still lands inside the volume directory.
+func TestReceiveFile_AcceptsNormalExt(t *testing.T) {
+	storeDir := t.TempDir()
+	vs := &VolumeServer{store: newTraversalTestStore(storeDir)}
+
+	// A realistic collection name holding '.' and '-' must still be accepted.
+	stream := &fakeReceiveFileStream{reqs: []*volume_server_pb.ReceiveFileRequest{
+		infoReq(&volume_server_pb.ReceiveFileInfo{
+			VolumeId:   4,
+			Ext:        ".ec00",
+			Collection: "my.bucket-1",
+			IsEcVolume: true,
+			FileSize:   5,
+		}),
+		contentReq([]byte("shard")),
+	}}
+
+	if err := vs.ReceiveFile(stream); err != nil {
+		t.Fatalf("ReceiveFile: %v", err)
+	}
+	if stream.resp == nil || stream.resp.Error != "" {
+		t.Fatalf("normal ext rejected: %+v", stream.resp)
+	}
+	if got, err := os.ReadFile(filepath.Join(storeDir, "my.bucket-1_4.ec00")); err != nil || string(got) != "shard" {
+		t.Fatalf("expected shard written inside volume dir, got %q err %v", got, err)
+	}
+}
+
+// fakeCopyFileServer records streamed content for CopyFile.
+type fakeCopyFileServer struct {
+	grpc.ServerStream
+	sent []byte
+}
+
+func (s *fakeCopyFileServer) Send(resp *volume_server_pb.CopyFileResponse) error {
+	s.sent = append(s.sent, resp.FileContent...)
+	return nil
+}
+
+// TestCopyFile_RejectsTraversalExt ensures the read side cannot be steered to an
+// arbitrary file with a traversal Ext.
+func TestCopyFile_RejectsTraversalExt(t *testing.T) {
+	root := t.TempDir()
+	storeDir := filepath.Join(root, "a", "b", "store")
+	if err := os.MkdirAll(storeDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	secret := filepath.Join(root, "a", "b", "secret.txt")
+	if err := os.WriteFile(secret, []byte("top secret"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	vs := &VolumeServer{store: newTraversalTestStore(storeDir)}
+
+	// util.Join(storeDir, "4"+"/../../secret.txt") path-cleans to the secret.
+	req := &volume_server_pb.CopyFileRequest{
+		VolumeId:   4,
+		Ext:        "/../../secret.txt",
+		IsEcVolume: true,
+		StopOffset: 1 << 20,
+	}
+	server := &fakeCopyFileServer{}
+	err := vs.CopyFile(req, server)
+	if err == nil {
+		t.Errorf("CopyFile accepted a traversal ext")
+	}
+	if len(server.sent) != 0 {
+		t.Errorf("CopyFile leaked %d bytes for a traversal ext: %q", len(server.sent), server.sent)
+	}
+}
+
+// assertNoEscapedFile fails if any file whose name contains needle exists under
+// root but outside storeDir.
+func assertNoEscapedFile(t *testing.T, root, storeDir, needle string) {
+	t.Helper()
+	_ = filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
+		if err != nil || d.IsDir() {
+			return nil
+		}
+		rel, _ := filepath.Rel(storeDir, path)
+		outside := rel == "" || rel[0] == '.'
+		if outside && strings.Contains(d.Name(), needle) {
+			t.Errorf("path traversal: file written outside volume dir at %s", path)
+		}
+		return nil
+	})
+}