CVE-2026-13917
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
TEST_Fios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm |
modified |
Files Changed
ios/chrome/browser/download/coordinator/download_manager_coordinator.mmios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm
Patch
From b1e96f29005bf87be6677fd4d38d46a9f19741f7 Mon Sep 17 00:00:00 2001 From: Quentin Pubert <[email protected]> Date: Thu, 07 May 2026 03:25:03 -0700 Subject: [PATCH] [iOS][Download] Fix path traversal in DownloadManagerCoordinator This CL resolves a local path traversal vulnerability in iOS Download Manager. When opening downloaded files, GURLs were constructed insecurely using base::StringPrintf with the raw path, allowing attackers to bypass sanitization using double-URL-encoded traversals (like %2e%2e%2f) in the filename. This fix replaces base::StringPrintf with net::FilePathToFileURL(path), which safely percent-escapes path components, preventing WKWebView/NSURL from decoding them into traversal boundaries (../). This CL also adds a unit test (OpenDownloadedFile) to verify secure URL construction and safety differences. Bug: 508286935 Change-Id: Ia2842ea43629391ca4ba83b3f9f0dc5a295fc5e4 Fixed: 508286935 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7818061 Commit-Queue: Quentin Pubert <[email protected]> Reviewed-by: Olivier Robin <[email protected]> Commit-Queue: Olivier Robin <[email protected]> Auto-Submit: Quentin Pubert <[email protected]> Cr-Commit-Position: refs/heads/main@{#1626822} --- diff --git a/ios/chrome/browser/download/coordinator/download_manager_coordinator.mm b/ios/chrome/browser/download/coordinator/download_manager_coordinator.mm index 638e7a8c..a2830d4 100644 --- a/ios/chrome/browser/download/coordinator/download_manager_coordinator.mm +++ b/ios/chrome/browser/download/coordinator/download_manager_coordinator.mm @@ -77,6 +77,7 @@ #import "ios/web/public/download/download_task.h" #import "ios/web/public/web_client.h" #import "net/base/apple/url_conversions.h" +#import "net/base/filename_util.h" #import "net/base/net_errors.h" #import "ui/base/l10n/l10n_util_mac.h" @@ -504,8 +505,7 @@ (UIViewController*)controller { base::RecordAction(base::UserMetricsAction("IOSDownloadOpen")); base::FilePath path = _mediator.GetDownloadPath(); - GURL filePathURL = - GURL(base::StringPrintf("%s://%s", "file", path.value().c_str())); + GURL filePathURL = net::FilePathToFileURL(path); GURL virtualFilePathURL = GURL( base::StringPrintf("%s://%s/%s", kChromeUIScheme, kChromeUIDownloadsHost, filePathURL.ExtractFileName().c_str())); diff --git a/ios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm b/ios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm index 3f20560a..e720cbcd 100644 --- a/ios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm +++ b/ios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm @@ -40,6 +40,8 @@ #import "ios/chrome/browser/shared/public/commands/browser_coordinator_commands.h" #import "ios/chrome/browser/shared/public/commands/command_dispatcher.h" #import "ios/chrome/browser/shared/public/commands/download_list_commands.h" +#import "ios/chrome/browser/shared/public/commands/open_new_tab_command.h" +#import "ios/chrome/browser/shared/public/commands/scene_commands.h" #import "ios/chrome/browser/shared/public/features/features.h" #import "ios/chrome/browser/shared/ui/util/file_size_util.h" #import "ios/chrome/browser/signin/model/authentication_service_factory.h" @@ -51,6 +53,8 @@ #import "ios/web/public/test/fakes/fake_download_task.h" #import "ios/web/public/test/fakes/fake_web_state.h" #import "ios/web/public/test/web_task_environment.h" +#import "net/base/apple/url_conversions.h" +#import "net/base/filename_util.h" #import "net/base/net_errors.h" #import "testing/gtest_mac.h" #import "testing/platform_test.h" @@ -965,3 +969,95 @@ } EXPECT_FALSE(coordinator_.viewController); } + +// Tests opening the downloaded file. Verifies that the file URL is safely +// constructed (escaped) and dispatched via `OpenNewTabCommand`. +TEST_F(DownloadManagerCoordinatorTest, OpenDownloadedFile) { + std::unique_ptr<web::FakeDownloadTask> task = CreateTestTask(); + web::FakeDownloadTask* task_ptr = task.get(); + + // Configure the task with an unsafe filename. + task_ptr->SetGeneratedFileName(base::FilePath("file name %2e%2e%2f.zip")); + + tab_helper()->SetCurrentDownload(std::move(task)); + coordinator_.downloadTask = task_ptr->GetWeakPtr(); + [coordinator_ start]; + + DownloadManagerViewController* viewController = + base_view_controller_.childViewControllers.firstObject; + ASSERT_EQ([DownloadManagerViewController class], [viewController class]); + + id<SceneCommands> scene_dispatcher_mock = + OCMProtocolMock(@protocol(SceneCommands)); + [browser_->GetCommandDispatcher() + startDispatchingToTarget:scene_dispatcher_mock + forProtocol:@protocol(SceneCommands)]; + + // Start the download from the coordinator. + @autoreleasepool { + [viewController.delegate + downloadManagerViewControllerDidStartDownload:viewController]; + } + + // Complete the download. + task_ptr->SetDone(true); + + // Wait for the final file path to be resolved. + ASSERT_TRUE(WaitUntilConditionOrTimeout( + base::test::ios::kWaitForDownloadTimeout, true, ^{ + return !tab_helper()->GetDownloadTaskFinalFilePath().empty(); + })); + + base::FilePath final_path = tab_helper()->GetDownloadTaskFinalFilePath(); + + // Expect `OpenNewTabCommand` with properly escaped URLs. + __block BOOL command_verified = NO; + OCMStub([scene_dispatcher_mock + openURLInNewTab:[OCMArg checkWithBlock:^(id object) { + EXPECT_EQ([OpenNewTabCommand class], [object class]); + OpenNewTabCommand* command = + base::apple::ObjCCastStrict<OpenNewTabCommand>(object); + + // The file URL should have escaped characters: + // space -> %20, % -> %25 + // So "file name %2e%2e%2f.zip" -> "file%20name%20%252e%252e%252f.zip" + std::string expected_url_spec = + net::FilePathToFileURL(final_path).spec(); + EXPECT_EQ(expected_url_spec, command.URL.spec()); + + // The virtual URL should be + // chrome://downloads/file%20name%20%252e%252e%252f.zip + std::string expected_virtual_url_spec = + "chrome://downloads/file%20name%20%252e%252e%252f.zip"; + EXPECT_EQ(expected_virtual_url_spec, command.virtualURL.spec()); + + command_verified = YES; + return YES; + }]]); + + // Open the downloaded file. + @autoreleasepool { + [viewController.delegate + openDownloadedFileForDownloadManagerViewController:viewController]; + } + + EXPECT_TRUE(command_verified); + + // Verify the path traversal safety difference when decoded once (as the OS + // loader does): + // 1. Unsafe URL decodes once to literal ".." traversal components: + GURL unsafeGURL("file:///Documents/file%20name%20%2e%2e%2f.zip"); + NSString* unsafeDecoded = [base::SysUTF8ToNSString(unsafeGURL.spec()) + stringByRemovingPercentEncoding]; + EXPECT_TRUE([unsafeDecoded containsString:@"/.."] || + [unsafeDecoded containsString:@".."]); + + // 2. Safe URL decodes once to "%2e%2e%2f", NOT containing literal traversal + // components: + GURL safeGURL("file:///Documents/file%20name%20%252e%252e%252f.zip"); + NSString* safeDecoded = [base::SysUTF8ToNSString(safeGURL.spec()) + stringByRemovingPercentEncoding]; + EXPECT_FALSE([safeDecoded containsString:@"/.."] || + [safeDecoded containsString:@".."]); + EXPECT_TRUE([safeDecoded containsString:@"%2e%2e%2f"]); +}
Regression Test / PoC
diff --git a/ios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm b/ios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm
index 3f20560a..e720cbcd 100644
--- a/ios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm
+++ b/ios/chrome/browser/download/coordinator/download_manager_coordinator_unittest.mm
@@ -40,6 +40,8 @@
#import "ios/chrome/browser/shared/public/commands/browser_coordinator_commands.h"
#import "ios/chrome/browser/shared/public/commands/command_dispatcher.h"
#import "ios/chrome/browser/shared/public/commands/download_list_commands.h"
+#import "ios/chrome/browser/shared/public/commands/open_new_tab_command.h"
+#import "ios/chrome/browser/shared/public/commands/scene_commands.h"
#import "ios/chrome/browser/shared/public/features/features.h"
#import "ios/chrome/browser/shared/ui/util/file_size_util.h"
#import "ios/chrome/browser/signin/model/authentication_service_factory.h"
@@ -51,6 +53,8 @@
#import "ios/web/public/test/fakes/fake_download_task.h"
#import "ios/web/public/test/fakes/fake_web_state.h"
#import "ios/web/public/test/web_task_environment.h"
+#import "net/base/apple/url_conversions.h"
+#import "net/base/filename_util.h"
#import "net/base/net_errors.h"
#import "testing/gtest_mac.h"
#import "testing/platform_test.h"
@@ -965,3 +969,95 @@
}
EXPECT_FALSE(coordinator_.viewController);
}
+
+// Tests opening the downloaded file. Verifies that the file URL is safely
+// constructed (escaped) and dispatched via `OpenNewTabCommand`.
+TEST_F(DownloadManagerCoordinatorTest, OpenDownloadedFile) {
+ std::unique_ptr<web::FakeDownloadTask> task = CreateTestTask();
+ web::FakeDownloadTask* task_ptr = task.get();
+
+ // Configure the task with an unsafe filename.
+ task_ptr->SetGeneratedFileName(base::FilePath("file name %2e%2e%2f.zip"));
+
+ tab_helper()->SetCurrentDownload(std::move(task));
+ coordinator_.downloadTask = task_ptr->GetWeakPtr();
+ [coordinator_ start];
+
+ DownloadManagerViewController* viewController =
+ base_view_controller_.childViewControllers.firstObject;
+ ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
+
+ id<SceneCommands> scene_dispatcher_mock =
+ OCMProtocolMock(@protocol(SceneCommands));
+ [browser_->GetCommandDispatcher()
+ startDispatchingToTarget:scene_dispatcher_mock
+ forProtocol:@protocol(SceneCommands)];
+
+ // Start the download from the coordinator.
+ @autoreleasepool {
+ [viewController.delegate
+ downloadManagerViewControllerDidStartDownload:viewController];
+ }
+
+ // Complete the download.
+ task_ptr->SetDone(true);
+
+ // Wait for the final file path to be resolved.
+ ASSERT_TRUE(WaitUntilConditionOrTimeout(
+ base::test::ios::kWaitForDownloadTimeout, true, ^{
+ return !tab_helper()->GetDownloadTaskFinalFilePath().empty();
+ }));
+
+ base::FilePath final_path = tab_helper()->GetDownloadTaskFinalFilePath();
+
+ // Expect `OpenNewTabCommand` with properly escaped URLs.
+ __block BOOL command_verified = NO;
+ OCMStub([scene_dispatcher_mock
+ openURLInNewTab:[OCMArg checkWithBlock:^(id object) {
+ EXPECT_EQ([OpenNewTabCommand class], [object class]);
+ OpenNewTabCommand* command =
+ base::apple::ObjCCastStrict<OpenNewTabCommand>(object);
+
+ // The file URL should have escaped characters:
+ // space -> %20, % -> %25
+ // So "file name %2e%2e%2f.zip" -> "file%20name%20%252e%252e%252f.zip"
+ std::string expected_url_spec =
+ net::FilePathToFileURL(final_path).spec();
+ EXPECT_EQ(expected_url_spec, command.URL.spec());
+
+ // The virtual URL should be
+ // chrome://downloads/file%20name%20%252e%252e%252f.zip
+ std::string expected_virtual_url_spec =
+ "chrome://downloads/file%20name%20%252e%252e%252f.zip";
+ EXPECT_EQ(expected_virtual_url_spec, command.virtualURL.spec());
+
+ command_verified = YES;
+ return YES;
+ }]]);
+
+ // Open the downloaded file.
+ @autoreleasepool {
+ [viewController.delegate
+ openDownloadedFileForDownloadManagerViewController:viewController];
+ }
+
+ EXPECT_TRUE(command_verified);
+
+ // Verify the path traversal safety difference when decoded once (as the OS
+ // loader does):
+ // 1. Unsafe URL decodes once to literal ".." traversal components:
+ GURL unsafeGURL("file:///Documents/file%20name%20%2e%2e%2f.zip");
+ NSString* unsafeDecoded = [base::SysUTF8ToNSString(unsafeGURL.spec())
+ stringByRemovingPercentEncoding];
+ EXPECT_TRUE([unsafeDecoded containsString:@"/.."] ||
+ [unsafeDecoded containsString:@".."]);
+
+ // 2. Safe URL decodes once to "%2e%2e%2f", NOT containing literal traversal
+ // components:
+ GURL safeGURL("file:///Documents/file%20name%20%252e%252e%252f.zip");
+ NSString* safeDecoded = [base::SysUTF8ToNSString(safeGURL.spec())
+ stringByRemovingPercentEncoding];
+ EXPECT_FALSE([safeDecoded containsString:@"/.."] ||
+ [safeDecoded containsString:@".."]);
+ EXPECT_TRUE([safeDecoded containsString:@"%2e%2e%2f"]);
+}
Original Bug Report
Potential Local Path Traversal in iOS Chrome Download Manager
Flapjack, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the Chrome Security team. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: iOS Chrome’s download manager insecurely constructs file:// URLs from downloaded file paths without proper escaping. An attacker can provide a double-URL-encoded filename to bypass initial sanitization and trigger a path traversal when the URL is decoded by WKWebView. This could potentially allow an attacker to trick a user into viewing arbitrary internal files from the application sandbox.
Affected files:
ios/chrome/browser/download/coordinator/download_manager_coordinator.mm
Estimated timestamp from git blame: Unknown (Google3 checkout)
Description
A potential vulnerability exists in iOS Chrome’s DownloadManagerCoordinator where it constructs URLs for downloaded files. When a user chooses to open a downloaded file, the application creates a GURL by concatenating the file:// scheme directly with the raw file path string using base::StringPrintf.
Because the raw path string is not properly percent-escaped, an attacker can embed URL-encoded traversal sequences (like %2e%2e%2f) into the filename. When this GURL is later converted to an NSURL and loaded by WKWebView, the underlying iOS frameworks automatically percent-decode the path. This resolves the traversal sequences, allowing the web view to escape the Downloads directory and load arbitrary files from the application’s sandbox (e.g., Library/Preferences/com.google.chrome.ios.plist).
Suggested Attacker Steps
Note: These are suggested/potential steps based on static code analysis. Our tooling agent does not currently have the ability to run code or verify the exploit end-to-end.
- Serve Malicious Download: An attacker hosts a file with a double-URL-encoded
Content-Dispositionheader, such as:Content-Disposition: attachment; filename="%252e%252e%252fLibrary%252fPreferences%252fcom.google.chrome.ios.plist" - Sanitization Bypass: During download,
net::GenerateFileNameunescapes%25to%, resulting in%2e%2e%2f.... Because the iOS Chrome filesystem sanitization functions only check for literal slashes (/) and dots (.), this percent-encoded string is considered valid and the file is saved literally as%2e%2e%2f...in theDownloadsdirectory. - User Interaction: The user taps “Open” in the iOS Chrome download UI.
- Insecure URL Construction: In
ios/chrome/browser/download/coordinator/download_manager_coordinator.mm, theopenDownloadedFileForDownloadManagerViewController:method constructs aGURLdirectly from the raw string without escaping it:GURL filePathURL = GURL(base::StringPrintf("%s://%s", "file", path.value().c_str())); - Security Check Bypass: A
virtualFilePathURLis created using thechrome://downloads/scheme. WhenCRWWebRequestControllerintercepts the navigation, it allows it becauseweb::GetWebClient()->IsAppSpecificURL(virtualURL)returns true forchrome://URLs. - Path Traversal Execution:
CRWWebRequestControllerconverts theGURLto anNSURLand calls[-WKWebView loadFileRequest:allowingReadAccessToURL:], passing the traversal URL for both arguments.NSURLpercent-decodes%2fto/and%2eto.. The OS resolves the..sequences, escaping theDownloadsfolder, and exposes the target internal file to the user in a new tab.
Impact
This is a localized sandbox logic bypass resulting in information disclosure. If an attacker can convince a user to download and open a file, they can force the browser to display sensitive internal application files (like local preferences, cookies, or databases) to the user.
Suggested Fix
Update openDownloadedFileForDownloadManagerViewController: to construct the GURL using net::FilePathToFileURL(path) instead of base::StringPrintf.
net::FilePathToFileURL safely percent-escapes characters like % to %25. This ensures that when NSURL later decodes the path, it decodes %25 back to % rather than interpreting %2e%2e%2f as a directory traversal sequence.
Evaluated with Chrome root at commit: cc901875d53bf4e4fe0e01f02843871da4106e70
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.