From 30d636a452dfa0076ffdfe2826c358b3744bf902 Mon Sep 17 00:00:00 2001
From: Mike West <[email protected]>
Date: Tue, 14 Apr 2026 12:52:43 -0700
Subject: [PATCH] Prevent path traversal in ZipString.

This change uses base::SafeBaseName to ensure that the filename passed
to feedback_util::ZipString is a safe, single-component base name.
This prevents a potential path traversal vulnerability where a
compromised renderer could attempt to write files outside of the
temporary directory.

Bug: 502248774
Change-Id: I948b3ce274a2469ab30e0fb043473b6acfeffdfc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7758483
Reviewed-by: Xiangdong Kong <[email protected]>
Commit-Queue: Mike West <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1614676}
---

diff --git a/components/feedback/feedback_util.cc b/components/feedback/feedback_util.cc
index 767389e..5787f98 100644
--- a/components/feedback/feedback_util.cc
+++ b/components/feedback/feedback_util.cc
@@ -10,6 +10,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/files/file_util.h"
+#include "base/files/safe_base_name.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/functional/bind.h"
 #include "base/json/json_reader.h"
@@ -32,6 +33,12 @@
 
 std::optional<std::string> ZipString(const base::FilePath& filename,
                                      std::string_view data) {
+  std::optional<base::SafeBaseName> safe_name =
+      base::SafeBaseName::Create(filename);
+  if (!safe_name || safe_name->path() != filename) {
+    return std::nullopt;
+  }
+
   base::ScopedTempDir temp_dir;
   base::FilePath zip_file;
 
@@ -40,7 +47,7 @@
   if (!temp_dir.CreateUniqueTempDir()) {
     return std::nullopt;
   }
-  if (!base::WriteFile(temp_dir.GetPath().Append(filename), data)) {
+  if (!base::WriteFile(temp_dir.GetPath().Append(safe_name->path()), data)) {
     return std::nullopt;
   }
   if (!base::CreateTemporaryFile(&zip_file)) {
diff --git a/components/feedback/feedback_util_unittest.cc b/components/feedback/feedback_util_unittest.cc
index 95f30b2..5e066524 100644
--- a/components/feedback/feedback_util_unittest.cc
+++ b/components/feedback/feedback_util_unittest.cc
@@ -153,4 +153,43 @@
   EXPECT_EQ(autofill_data_str, expected_autofill_data_str);
 }
 
+TEST_F(FeedbackUtilTest, ZipStringTraversal) {
+  // Create a temp directory, and target a file within it:
+  base::ScopedTempDir root_dir;
+  ASSERT_TRUE(root_dir.CreateUniqueTempDir());
+  base::FilePath sensitive_file =
+      root_dir.GetPath().AppendASCII("sensitive.txt");
+
+  // Construct a traversal back to that file in a platform-dependent way:
+  std::string sensitive_path_str = sensitive_file.AsUTF8Unsafe();
+#if BUILDFLAG(IS_WIN)
+  // Remove "C:" if present
+  if (sensitive_path_str.size() >= 2 && sensitive_path_str[1] == ':') {
+    sensitive_path_str = sensitive_path_str.substr(2);
+  }
+  // Remove leading backslash
+  if (!sensitive_path_str.empty() && sensitive_path_str[0] == '\\') {
+    sensitive_path_str = sensitive_path_str.substr(1);
+  }
+  base::FilePath traversal(
+      FILE_PATH_LITERAL("..\\..\\..\\..\\..\\..\\..\\..\\"));
+#else
+  // Remove leading slash
+  if (!sensitive_path_str.empty() && sensitive_path_str[0] == '/') {
+    sensitive_path_str = sensitive_path_str.substr(1);
+  }
+  base::FilePath traversal(FILE_PATH_LITERAL("../../../../../../../../"));
+#endif
+
+  base::FilePath malicious_filename =
+      traversal.Append(base::FilePath::FromUTF8Unsafe(sensitive_path_str));
+
+  // Call ZipString.
+  std::optional<std::string> result =
+      feedback_util::ZipString(malicious_filename, "maliciousness");
+
+  EXPECT_FALSE(result.has_value());
+  EXPECT_FALSE(base::PathExists(sensitive_file));
+}
+
 }  // namespace feedback_util

diff --git a/components/feedback/feedback_util_unittest.cc b/components/feedback/feedback_util_unittest.cc
index 95f30b2..5e066524 100644
--- a/components/feedback/feedback_util_unittest.cc
+++ b/components/feedback/feedback_util_unittest.cc
@@ -153,4 +153,43 @@
   EXPECT_EQ(autofill_data_str, expected_autofill_data_str);
 }
 
+TEST_F(FeedbackUtilTest, ZipStringTraversal) {
+  // Create a temp directory, and target a file within it:
+  base::ScopedTempDir root_dir;
+  ASSERT_TRUE(root_dir.CreateUniqueTempDir());
+  base::FilePath sensitive_file =
+      root_dir.GetPath().AppendASCII("sensitive.txt");
+
+  // Construct a traversal back to that file in a platform-dependent way:
+  std::string sensitive_path_str = sensitive_file.AsUTF8Unsafe();
+#if BUILDFLAG(IS_WIN)
+  // Remove "C:" if present
+  if (sensitive_path_str.size() >= 2 && sensitive_path_str[1] == ':') {
+    sensitive_path_str = sensitive_path_str.substr(2);
+  }
+  // Remove leading backslash
+  if (!sensitive_path_str.empty() && sensitive_path_str[0] == '\\') {
+    sensitive_path_str = sensitive_path_str.substr(1);
+  }
+  base::FilePath traversal(
+      FILE_PATH_LITERAL("..\\..\\..\\..\\..\\..\\..\\..\\"));
+#else
+  // Remove leading slash
+  if (!sensitive_path_str.empty() && sensitive_path_str[0] == '/') {
+    sensitive_path_str = sensitive_path_str.substr(1);
+  }
+  base::FilePath traversal(FILE_PATH_LITERAL("../../../../../../../../"));
+#endif
+
+  base::FilePath malicious_filename =
+      traversal.Append(base::FilePath::FromUTF8Unsafe(sensitive_path_str));
+
+  // Call ZipString.
+  std::optional<std::string> result =
+      feedback_util::ZipString(malicious_filename, "maliciousness");
+
+  EXPECT_FALSE(result.has_value());
+  EXPECT_FALSE(base::PathExists(sensitive_file));
+}
+
 }  // namespace feedback_util