From 6be36ba6322ed8f3c4a7e2e17b424b0f13ed97f7 Mon Sep 17 00:00:00 2001 From: Bram Date: Mon, 27 Jul 2026 15:30:28 -0400 Subject: [PATCH] Fix path traversal attack in `-proto-out-dir` (#573) * Fix path traversal in `-proto-out-dir` A malicious file descriptor name (e.g. from an untrusted protoset or server reflection) could escape the output directory through ../ sequences, writing attacker-controlled content to arbitrary paths. Confine all writes to an `os.Root` opened on the output directory. * assert * refactor --------- Co-authored-by: Scott Blum --- desc_source.go | 27 ++++++++++++++++++------- desc_source_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/desc_source.go b/desc_source.go index 258c346..b879cf2 100644 --- a/desc_source.go +++ b/desc_source.go @@ -309,24 +309,37 @@ func WriteProtoFiles(outProtoDirPath string, descSource DescriptorSource, symbol for _, filename := range filenames { allFileDescriptors = addFilesToFileDescriptorList(allFileDescriptors, expandedFiles, fds[filename]) } + return writeProtoFiles(outProtoDirPath, allFileDescriptors) +} + +func writeProtoFiles(outProtoDirPath string, allFileDescriptors []*desc.FileDescriptor) error { + if err := os.MkdirAll(outProtoDirPath, 0755); err != nil { + return fmt.Errorf("failed to create output directory %q: %w", outProtoDirPath, err) + } + root, err := os.OpenRoot(outProtoDirPath) + if err != nil { + return fmt.Errorf("failed to open output directory %q: %w", outProtoDirPath, err) + } + defer root.Close() pr := protoprint.Printer{} // now we can serialize to files for i := range allFileDescriptors { - if err := writeProtoFile(outProtoDirPath, allFileDescriptors[i], &pr); err != nil { + if err := writeProtoFile(root, allFileDescriptors[i], &pr); err != nil { return err } } return nil } -func writeProtoFile(outProtoDirPath string, fd *desc.FileDescriptor, pr *protoprint.Printer) error { - outFile := filepath.Join(outProtoDirPath, fd.GetFullyQualifiedName()) - outDir := filepath.Dir(outFile) - if err := os.MkdirAll(outDir, 0777); err != nil { - return fmt.Errorf("failed to create directory %q: %w", outDir, err) +func writeProtoFile(root *os.Root, fd *desc.FileDescriptor, pr *protoprint.Printer) error { + // root confines all path operations to the output directory, so a + // malicious file name (e.g. "../escape.proto") cannot write outside it + outFile := fd.GetFullyQualifiedName() + if err := root.MkdirAll(filepath.Dir(outFile), 0755); err != nil { + return fmt.Errorf("failed to create directory for %q: %w", outFile, err) } - f, err := os.Create(outFile) + f, err := root.Create(outFile) if err != nil { return fmt.Errorf("failed to create proto file %q: %w", outFile, err) } diff --git a/desc_source_test.go b/desc_source_test.go index b043b6d..d5e318a 100644 --- a/desc_source_test.go +++ b/desc_source_test.go @@ -3,9 +3,11 @@ package grpcurl import ( "bytes" "os" + "path/filepath" "testing" - "github.com/golang/protobuf/proto" //lint:ignore SA1019 we have to import this because it appears in exported API + "github.com/golang/protobuf/proto" + "github.com/jhump/protoreflect/desc" "google.golang.org/protobuf/types/descriptorpb" ) @@ -60,3 +62,47 @@ func checkWriteProtoset(t *testing.T, descSrc DescriptorSource, protoset *descri t.Fatalf("written protoset not equal to input:\nExpecting: %s\nActual: %s", protoset, &result) } } + +func writeProtoFileForTest(t *testing.T, dir, fdName string) error { + t.Helper() + fd, err := desc.CreateFileDescriptor(&descriptorpb.FileDescriptorProto{ + Name: proto.String(fdName), + Syntax: proto.String("proto3"), + }) + if err != nil { + t.Fatalf("failed to create file descriptor: %v", err) + } + return writeProtoFiles(dir, []*desc.FileDescriptor{fd}) +} + +func TestWriteProtoFile_NormalPath(t *testing.T) { + dir := t.TempDir() + if err := writeProtoFileForTest(t, dir, "foo/bar.proto"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "foo", "bar.proto")); err != nil { + t.Fatalf("expected output file not created: %v", err) + } +} + +func TestWriteProtoFile_RejectsPathTraversal(t *testing.T) { + dir := t.TempDir() + if err := writeProtoFileForTest(t, dir, "../escape.proto"); err == nil { + t.Fatal("expected error for path-traversing descriptor name, got nil") + } + escapePath := filepath.Join(dir, "..", "escape.proto") + if _, err := os.Stat(escapePath); err == nil { + t.Fatalf("file was created outside output directory at %q", escapePath) + } +} + +func TestWriteProtoFile_RejectsDeepPathTraversal(t *testing.T) { + dir := t.TempDir() + if err := writeProtoFileForTest(t, dir, "foo/../../../escape.proto"); err == nil { + t.Fatal("expected error for path-traversing descriptor name, got nil") + } + escapePath := filepath.Join(dir, "foo", "..", "..", "..", "escape.proto") + if _, err := os.Stat(escapePath); err == nil { + t.Fatalf("file was created outside output directory at %q", escapePath) + } +}