Chrome · DevTools
CVE-2025-1915
Logic Error in DevTools
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
TargetHandlerchrome/browser/devtools/protocol/target_handler.h |
modified | |
ifchrome/test/data/extensions/api_test/debugger_file_access/background.js |
modified |
Files Changed
AUTHORSchrome/browser/devtools/chrome_devtools_session.ccchrome/browser/devtools/protocol/target_handler.ccchrome/browser/devtools/protocol/target_handler.hchrome/test/data/extensions/api_test/debugger_file_access/background.js
Patch
From e166400fa4c632c5150d75bf587e8a34285fb783 Mon Sep 17 00:00:00 2001 From: Topi Lassila <[email protected]> Date: Mon, 03 Feb 2025 00:09:25 -0800 Subject: [PATCH] Check whether devtools clients should be allowed to target local files Bug: 391114799 Change-Id: I1ab514ebe7b4a00c740bb1943dcf3c3401d931b8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6218796 Reviewed-by: Alex Rudenko <[email protected]> Commit-Queue: Alex Rudenko <[email protected]> Reviewed-by: Simon Zünd <[email protected]> Cr-Commit-Position: refs/heads/main@{#1414781} --- diff --git a/AUTHORS b/AUTHORS index a3c3351..e96a3af 100644 --- a/AUTHORS +++ b/AUTHORS @@ -1467,6 +1467,7 @@ Tomas Popela <[email protected]> Tomasz Edward Posłuszny <[email protected]> Tony Shen <[email protected]> +Topi Lassila <[email protected]> Torsten Kurbad <[email protected]> Toshihito Kikuchi <[email protected]> Toshiaki Tanaka <[email protected]> diff --git a/chrome/browser/devtools/chrome_devtools_session.cc b/chrome/browser/devtools/chrome_devtools_session.cc index a5ae0dcfd..17d7ac9 100644 --- a/chrome/browser/devtools/chrome_devtools_session.cc +++ b/chrome/browser/devtools/chrome_devtools_session.cc @@ -100,7 +100,8 @@ if (IsDomainAvailableToUntrustedClient<TargetHandler>() || channel->GetClient()->IsTrusted()) { target_handler_ = std::make_unique<TargetHandler>( - &dispatcher_, channel->GetClient()->IsTrusted()); + &dispatcher_, channel->GetClient()->IsTrusted(), + channel->GetClient()->MayReadLocalFiles()); } if (IsDomainAvailableToUntrustedClient<BrowserHandler>() || channel->GetClient()->IsTrusted()) { diff --git a/chrome/browser/devtools/protocol/target_handler.cc b/chrome/browser/devtools/protocol/target_handler.cc index 8087bff4..363ee4f 100644 --- a/chrome/browser/devtools/protocol/target_handler.cc +++ b/chrome/browser/devtools/protocol/target_handler.cc @@ -48,8 +48,9 @@ } // namespace TargetHandler::TargetHandler(protocol::UberDispatcher* dispatcher, - bool is_trusted) - : is_trusted_(is_trusted) { + bool is_trusted, + bool may_read_local_files) + : is_trusted_(is_trusted), may_read_local_files_(may_read_local_files) { protocol::Target::Dispatcher::wire(dispatcher, this); } @@ -140,6 +141,11 @@ "Refusing to create a target with the specified URL"); } + if (!may_read_local_files_ && gurl.SchemeIsFile()) { + return protocol::Response::ServerError( + "Creating a target with a local URL is not allowed"); + } + create_new_window = !target_browser; const bool set_window_position = left || top || width || height; diff --git a/chrome/browser/devtools/protocol/target_handler.h b/chrome/browser/devtools/protocol/target_handler.h index 67cddb8..0e4b0b3 100644 --- a/chrome/browser/devtools/protocol/target_handler.h +++ b/chrome/browser/devtools/protocol/target_handler.h @@ -16,7 +16,9 @@ class TargetHandler : public protocol::Target::Backend { public: - TargetHandler(protocol::UberDispatcher* dispatcher, bool is_trusted); + TargetHandler(protocol::UberDispatcher* dispatcher, + bool is_trusted, + bool may_read_local_files); TargetHandler(const TargetHandler&) = delete; TargetHandler& operator=(const TargetHandler&) = delete; @@ -46,6 +48,7 @@ private: RemoteLocations remote_locations_; const bool is_trusted_; + const bool may_read_local_files_; }; #endif // CHROME_BROWSER_DEVTOOLS_PROTOCOL_TARGET_HANDLER_H_ diff --git a/chrome/test/data/extensions/api_test/debugger_file_access/background.js b/chrome/test/data/extensions/api_test/debugger_file_access/background.js index 36a064c3..38e3f00 100644 --- a/chrome/test/data/extensions/api_test/debugger_file_access/background.js +++ b/chrome/test/data/extensions/api_test/debugger_file_access/background.js @@ -118,6 +118,30 @@ }); }, + function testCreateTarget() { + const url = chrome.runtime.getURL('dummy.html'); + openTab(url).then((tab) => { + chrome.test.assertEq(url, tab.url); + const tabId = tab.id; + chrome.debugger.attach({tabId: tabId}, '1.1', function() { + chrome.test.assertNoLastError(); + chrome.debugger.sendCommand({tabId: tabId}, 'Target.createTarget', + {url: fileUrl}, function() { + if (expectFileAccess) { + chrome.test.assertNoLastError(); + } else { + chrome.test.assertLastError(JSON.stringify({ + code: -32000, + message: 'Creating a target with a local URL is not allowed' + })); + } + chrome.tabs.remove(tabId); + chrome.test.succeed(); + }); + }); + }); + }, + // https://crbug.com/866426 function setDownloadBehavior() { // We never allow to write local files.
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/test/data/extensions/api_test/debugger_file_access/background.js b/chrome/test/data/extensions/api_test/debugger_file_access/background.js
index 36a064c3..38e3f00 100644
--- a/chrome/test/data/extensions/api_test/debugger_file_access/background.js
+++ b/chrome/test/data/extensions/api_test/debugger_file_access/background.js
@@ -118,6 +118,30 @@
});
},
+ function testCreateTarget() {
+ const url = chrome.runtime.getURL('dummy.html');
+ openTab(url).then((tab) => {
+ chrome.test.assertEq(url, tab.url);
+ const tabId = tab.id;
+ chrome.debugger.attach({tabId: tabId}, '1.1', function() {
+ chrome.test.assertNoLastError();
+ chrome.debugger.sendCommand({tabId: tabId}, 'Target.createTarget',
+ {url: fileUrl}, function() {
+ if (expectFileAccess) {
+ chrome.test.assertNoLastError();
+ } else {
+ chrome.test.assertLastError(JSON.stringify({
+ code: -32000,
+ message: 'Creating a target with a local URL is not allowed'
+ }));
+ }
+ chrome.tabs.remove(tabId);
+ chrome.test.succeed();
+ });
+ });
+ });
+ },
+
// https://crbug.com/866426
function setDownloadBehavior() {
// We never allow to write local files.
Loading diff…
Original Bug Report
reported by [email protected]
Extensions without file URL access can open UNC paths through chrome.debugger
Steps to reproduce the problem
On a server that the target can access:
- Clone the following repository: https://github.com/lgandx/Responder
- Inside the repository, run sudo ./Responder.py -I <INTERFACE> -w, where INTERFACE is the public facing network interface of the server
On the target:
- Download the attached extension
- In background.js, replace SERVER_IP with the address of the server running responder.py
- Install the extension, and make sure to disable “Allow access to file URLs”
Note that the NTLM hash is leaked twice, once after the initial install with access to file URLs enabled (as expected), but also when the extension reloads after disabling that option.
Problem Description
DevTool’s TargetHandler::CreateTarget does not check if the current extension should be allowed to access local files, which allows extensions with the debugger permission to open arbitrary file URLs. On Windows, this can be exploited by opening a UNC path, which will then lead to the current user’s NTLM hash getting leaked, as in bugs such as https://issues.chromium.org/issues/40060207
In addition to the extension that can be used to reproduce this, I’ve also attached a suggested fix, which should disallow creating targets with file URLs if the appropriate permissions aren’t given.
Summary
Extensions without file URL access can open UNC paths through chrome.debugger
Additional Data
Category: Security
Chrome Channel: Stable
Regression: No
References
On This Page