Fail writing files in protoc CLI if any file output path is relative.

A flag --unsafe_allow_out_dir_escape is added which disables the enforcement for any users who are intentionally writing files outside of the out_dir as set in protoc.

This was always the intended behavior but was not enforced before now.

This avoids risks of files being accidentally written which escape the intended output directories. Strictly speaking this is more restrictive than necessary, where a relative path of  `x/../y.java` could be technically ok since it doesn't escape while `x/../../y.java` does). But as we should should never have relative path components at this point in a working as intended flow so the simplest thing is to ban all relative paths when going to write out.

When investigating this issue, at least one case of someone intentionally escaping the output directory by (for example) setting `go_package = "../xyz";` Unfortunately, this change will break those rare usecases and the only fix will be to adjust the .proto file and protoc invocation context, but as a security hardening measure of an unintended behavior this is an intended change.

PiperOrigin-RevId: 893098701

Author: protobuf-github-bot

php/BUILD.bazel

@@ -2,7 +2,7 @@
 #
 # See also code generation logic under /src/google/protobuf/compiler/php.
 
-load("@rules_pkg//pkg:mappings.bzl", "pkg_filegroup", "pkg_files", "strip_prefix")
+load("@rules_pkg//pkg:mappings.bzl", "pkg_filegroup", "pkg_files")
 load("@rules_pkg//pkg:tar.bzl", "pkg_tar")
 load("//:protobuf_version.bzl", "PROTOBUF_PHP_VERSION", "PROTOC_VERSION")
 load("//build_defs:internal_shell.bzl", "inline_sh_binary")
@@ -175,7 +175,7 @@ genrule(
         "generated/ext/google/protobuf/wkt.inc",
     ],
     cmd = """
-        $(execpath //:protoc) --php_out=internal_generate_c_wkt:$(RULEDIR)/generated/src --proto_path=src $(locations //src/google/protobuf:well_known_type_protos);
+        $(execpath //:protoc) --php_out=internal_generate_c_wkt:$(RULEDIR)/generated/src --unsafe_allow_out_dir_escape --proto_path=src $(locations //src/google/protobuf:well_known_type_protos);
         $(execpath //:protoc) --php_out=internal:$(RULEDIR)/generated/src --proto_path=src $(location //src/google/protobuf:descriptor_proto_srcs);
     """ + _CHECK_GENCODE,
     tags = ["manual"],

php/generate_descriptor_protos.sh

@@ -24,6 +24,7 @@ fi
 pushd src
 $PROTOC --php_out=internal:../php/src google/protobuf/descriptor.proto
 $PROTOC --php_out=internal_generate_c_wkt:../php/src \
+  --unsafe_allow_out_dir_escape \
   google/protobuf/any.proto \
   google/protobuf/api.proto \
   google/protobuf/duration.proto \

src/google/protobuf/compiler/command_line_interface.cc

@@ -462,7 +462,7 @@ class CommandLineInterface::GeneratorContextImpl : public GeneratorContext {
 
   // Write all files in the directory to disk at the given output location,
   // which must end in a '/'.
-  bool WriteAllToDisk(const std::string& prefix);
+  bool WriteAllToDisk(const std::string& prefix, bool allow_escape = false);
 
   // Write the contents of this directory to a ZIP-format archive with the
   // given name.
@@ -562,7 +562,7 @@ CommandLineInterface::GeneratorContextImpl::GeneratorContextImpl(
     : parsed_files_(parsed_files), had_error_(false) {}
 
 bool CommandLineInterface::GeneratorContextImpl::WriteAllToDisk(
-    const std::string& prefix) {
+    const std::string& prefix, bool allow_escape) {
   if (had_error_) {
     return false;
   }
@@ -576,6 +576,15 @@ bool CommandLineInterface::GeneratorContextImpl::WriteAllToDisk(
     const char* data = pair.second.data();
     int size = pair.second.size();
 
+    if (!allow_escape && absl::StrContains(relative_filename, "..")) {
+      std::cerr << "Output file names must never have a relative path."
+                << " (" << relative_filename << "). "
+                << "Use --unsafe_allow_out_dir_escape to disable this error if "
+                   "intentional."
+                << std::endl;
+      return false;
+    }
+
     if (!TryCreateParentDirectory(prefix, relative_filename)) {
       return false;
     }
@@ -1481,7 +1490,7 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) {
     const std::string& location = pair.first;
     GeneratorContextImpl* directory = pair.second.get();
     if (absl::EndsWith(location, "/")) {
-      if (!directory->WriteAllToDisk(location)) {
+      if (!directory->WriteAllToDisk(location, unsafe_allow_out_dir_escape_)) {
         return 1;
       }
     } else {
@@ -2154,7 +2163,8 @@ bool CommandLineInterface::ParseArgument(const char* arg, std::string* name,
       *name == "--experimental_editions" ||
       *name == "--print_free_field_numbers" ||
       *name == "--experimental_allow_proto3_optional" ||
-      *name == "--deterministic_output" || *name == "--fatal_warnings") {
+      *name == "--deterministic_output" ||
+      *name == "--unsafe_allow_out_dir_escape" || *name == "--fatal_warnings") {
     // HACK:  These are the only flags that don't take a value.
     //   They probably should not be hard-coded like this but for now it's
     //   not worth doing better.
@@ -2390,6 +2400,8 @@ CommandLineInterface::InterpretArgument(const std::string& name,
 
   } else if (name == "--disallow_services") {
     disallow_services_ = true;
+  } else if (name == "--unsafe_allow_out_dir_escape") {
+    unsafe_allow_out_dir_escape_ = true;
   } else if (name == "--experimental_allow_proto3_optional") {
     // Flag is no longer observed, but we allow it for backward compat.
   } else if (name == "--encode" || name == "--decode" ||
@@ -2627,6 +2639,9 @@ Parse PROTO_FILES and generate output based on the options given:
                               deterministically ordered. Note that this order
                               is not canonical, and changes across builds or
                               releases of protoc.
+  --unsafe_allow_out_dir_escape
+                              Allow output files to use ".." to escape the
+                              output directory. Use with caution.
   --decode=MESSAGE_TYPE       Read a binary message of the given type from
                               standard input and write it in text format
                               to standard output.  The message type must

src/google/protobuf/compiler/command_line_interface.h

@@ -500,6 +500,9 @@ class PROTOC_EXPORT CommandLineInterface {
 
   // When using --encode, this will be passed to SetSerializationDeterministic.
   bool deterministic_output_ = false;
+  // Whether to allow files to be written to a path that is outside of the
+  // output directory.
+  bool unsafe_allow_out_dir_escape_ = false;
 
   bool opensource_runtime_ = google::protobuf::internal::IsOss();
 

src/google/protobuf/compiler/command_line_interface_unittest.cc

@@ -1175,6 +1175,58 @@ TEST_F(CommandLineInterfaceTest, CreateDirectory) {
   ExpectGenerated("test_plugin", "", "bar/baz/foo.proto", "Foo", "plugout");
 }
 
+TEST_F(CommandLineInterfaceTest, RejectDotDotInFilename) {
+  class DotDotGenerator : public CodeGenerator {
+   public:
+    bool Generate(const FileDescriptor* file, const std::string& parameter,
+                  GeneratorContext* context,
+                  std::string* error) const override {
+      std::unique_ptr<io::ZeroCopyOutputStream> output(
+          context->Open("abc/../../foo.proto.test"));
+      return true;
+    }
+  };
+
+  RegisterGenerator("--dotdot_out", std::make_unique<DotDotGenerator>(),
+                    "Test .. rejection.");
+
+  CreateTempFile("foo.proto",
+                 "syntax = \"proto2\";\n"
+                 "message Foo {}\n");
+
+  RunProtoc(
+      "protocol_compiler --dotdot_out=$tmpdir "
+      "--proto_path=$tmpdir foo.proto");
+
+  ExpectErrorSubstring("Output file names must never have a relative path.");
+}
+
+TEST_F(CommandLineInterfaceTest, AllowDotDotInFilenameWithFlag) {
+  class DotDotGenerator : public CodeGenerator {
+   public:
+    bool Generate(const FileDescriptor* file, const std::string& parameter,
+                  GeneratorContext* context,
+                  std::string* error) const override {
+      std::unique_ptr<io::ZeroCopyOutputStream> output(
+          context->Open("abc/../../foo.proto.test"));
+      return true;
+    }
+  };
+
+  RegisterGenerator("--dotdot_out", std::make_unique<DotDotGenerator>(),
+                    "Test .. allowance.");
+
+  CreateTempFile("foo.proto",
+                 "syntax = \"proto2\";\n"
+                 "message Foo {}\n");
+
+  RunProtoc(
+      "protocol_compiler --unsafe_allow_out_dir_escape --dotdot_out=$tmpdir "
+      "--proto_path=$tmpdir foo.proto");
+
+  ExpectNoErrors();
+}
+
 TEST_F(CommandLineInterfaceTest, GeneratorParameters) {
   // Test that generator parameters are correctly parsed from the command line.