Skip to content

Commit

Permalink
Add a test to verify delay load hooks work for chrome.dll.
Browse files Browse the repository at this point in the history
This test triggers the delay load hooks on child processes
by triggering the failed delay load of a DLL from within a
sandboxed utility process.

The DLL was chosen by examining delay load imports in
//build/config/win:delayloads and then picking an imported
function that was never called in a sandboxed process.

This adds the missing test from crrev.com/c/3626221.

BUG=1322217

Change-Id: Ia77521257b364c4a0323f4b20580e5449858eb32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3633247
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001079}
  • Loading branch information
Will Harris authored and Chromium LUCI CQ committed May 9, 2022
1 parent bd0488a commit 80db05b
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 5 deletions.
6 changes: 3 additions & 3 deletions chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,6 @@ base::LazyInstance<ChromeContentGpuClient>::DestructorAtExit
g_chrome_content_gpu_client = LAZY_INSTANCE_INITIALIZER;
base::LazyInstance<ChromeContentRendererClient>::DestructorAtExit
g_chrome_content_renderer_client = LAZY_INSTANCE_INITIALIZER;
base::LazyInstance<ChromeContentUtilityClient>::DestructorAtExit
g_chrome_content_utility_client = LAZY_INSTANCE_INITIALIZER;

extern int NaClMain(content::MainFunctionParams);

Expand Down Expand Up @@ -1324,7 +1322,9 @@ ChromeMainDelegate::CreateContentRendererClient() {

content::ContentUtilityClient*
ChromeMainDelegate::CreateContentUtilityClient() {
return g_chrome_content_utility_client.Pointer();
chrome_content_utility_client_ =
std::make_unique<ChromeContentUtilityClient>();
return chrome_content_utility_client_.get();
}

void ChromeMainDelegate::PreBrowserMain() {
Expand Down
2 changes: 2 additions & 0 deletions chrome/app/chrome_main_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class TracingSamplerProfiler;
}

class ChromeContentBrowserClient;
class ChromeContentUtilityClient;
class HeapProfilerController;

// Chrome implementation of ContentMainDelegate.
Expand Down Expand Up @@ -84,6 +85,7 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
ChromeContentClient chrome_content_client_;

std::unique_ptr<ChromeContentBrowserClient> chrome_content_browser_client_;
std::unique_ptr<ChromeContentUtilityClient> chrome_content_utility_client_;

std::unique_ptr<tracing::TracingSamplerProfiler> tracing_sampler_profiler_;

Expand Down
3 changes: 3 additions & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ static_library("test_support") {
"//services/data_decoder/public/cpp:test_support",
"//services/device/public/cpp:test_support",
"//services/device/public/cpp/geolocation",
"//services/test/echo:lib",
"//skia",
"//sql",
"//sql:test_support",
Expand Down Expand Up @@ -2431,6 +2432,7 @@ if (!is_android) {
"../browser/ui/views/settings_reset_prompt_dialog_browsertest.cc",
"../browser/ui/views/try_chrome_dialog_win/try_chrome_dialog_browsertest.cc",
"../browser/ui/views/uninstall_view_browsertest.cc",
"../test/delayload/delayload_hook_browsertest.cc",
]

configs += [ "//build/config/win:delayloads" ]
Expand All @@ -2448,6 +2450,7 @@ if (!is_android) {
"//components/chrome_cleaner/public/constants",
"//components/onc",
"//sandbox/win:sandbox",
"//services/test/echo/public/mojom",
"//third_party/wtl",
"//ui/resources",
]
Expand Down
30 changes: 30 additions & 0 deletions chrome/test/base/chrome_test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
#include "content/public/test/network_service_test_helper.h"
#include "content/public/test/test_launcher.h"
#include "content/public/test/test_utils.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/service_factory.h"
#include "services/test/echo/echo_service.h"
#include "ui/base/test/ui_controls.h"

#if BUILDFLAG(IS_MAC)
Expand Down Expand Up @@ -126,6 +129,14 @@ class ChromeTestLauncherDelegate::ScopedFirewallRules {

#endif // BUILDFLAG(IS_WIN)

namespace {

auto RunEchoService(mojo::PendingReceiver<echo::mojom::EchoService> receiver) {
return std::make_unique<echo::EchoService>(std::move(receiver));
}

} // namespace

ChromeTestLauncherDelegate::ChromeTestLauncherDelegate(
ChromeTestSuiteRunner* runner)
: runner_(runner) {}
Expand All @@ -152,13 +163,32 @@ class BrowserTestChromeContentBrowserClient
}
};

// A replacement ChromeContentUtilityClient that binds the
// echo::mojom::EchoService within the Utility process. For use with testing
// only.
class BrowserTestChromeContentUtilityClient
: public ChromeContentUtilityClient {
public:
void RegisterIOThreadServices(mojo::ServiceFactory& services) override {
ChromeContentUtilityClient::RegisterIOThreadServices(services);
services.Add(RunEchoService);
}
};

content::ContentBrowserClient*
ChromeTestChromeMainDelegate::CreateContentBrowserClient() {
chrome_content_browser_client_ =
std::make_unique<BrowserTestChromeContentBrowserClient>();
return chrome_content_browser_client_.get();
}

content::ContentUtilityClient*
ChromeTestChromeMainDelegate::CreateContentUtilityClient() {
chrome_content_utility_client_ =
std::make_unique<BrowserTestChromeContentUtilityClient>();
return chrome_content_utility_client_.get();
}

#if BUILDFLAG(IS_WIN)
bool ChromeTestChromeMainDelegate::ShouldHandleConsoleControlEvents() {
// Allow Ctrl-C and friends to terminate the test processes forthwith.
Expand Down
1 change: 1 addition & 0 deletions chrome/test/base/chrome_test_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class ChromeTestChromeMainDelegate

// ChromeMainDelegateOverrides.
content::ContentBrowserClient* CreateContentBrowserClient() override;
content::ContentUtilityClient* CreateContentUtilityClient() override;
#if BUILDFLAG(IS_WIN)
bool ShouldHandleConsoleControlEvents() override;
#endif
Expand Down
79 changes: 79 additions & 0 deletions chrome/test/delayload/delayload_hook_browsertest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <windows.h>

#include "base/run_loop.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "content/public/browser/browser_child_process_observer.h"
#include "content/public/browser/child_process_data.h"
#include "content/public/browser/child_process_termination_info.h"
#include "content/public/browser/service_process_host.h"
#include "content/public/browser/service_process_info.h"
#include "content/public/test/browser_test.h"
#include "services/test/echo/public/mojom/echo.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace chrome {

using ChromeDelayLoadHookTest = InProcessBrowserTest;

class EchoServiceProcessObserver : public content::ServiceProcessHost::Observer,
public content::BrowserChildProcessObserver {
public:
EchoServiceProcessObserver() {
content::ServiceProcessHost::AddObserver(this);
content::BrowserChildProcessObserver::Add(this);
}

EchoServiceProcessObserver(const EchoServiceProcessObserver&) = delete;
EchoServiceProcessObserver& operator=(const EchoServiceProcessObserver&) =
delete;

~EchoServiceProcessObserver() override {
content::BrowserChildProcessObserver::Remove(this);
content::ServiceProcessHost::RemoveObserver(this);
}

void WaitForLaunch() { launch_loop_.Run(); }

int WaitForCrash() {
crash_loop_.Run();
return exit_code_;
}

private:
// content::ServiceProcessHost::Observer:
void OnServiceProcessLaunched(
const content::ServiceProcessInfo& info) override {
if (info.IsService<echo::mojom::EchoService>())
launch_loop_.Quit();
}

// content::BrowserChildProcessObserver:
void BrowserChildProcessCrashed(
const content::ChildProcessData& data,
const content::ChildProcessTerminationInfo& info) override {
if (data.metrics_name == echo::mojom::EchoService::Name_) {
exit_code_ = info.exit_code;
crash_loop_.Quit();
}
}

int exit_code_;
base::RunLoop launch_loop_;
base::RunLoop crash_loop_;
};

IN_PROC_BROWSER_TEST_F(ChromeDelayLoadHookTest, ObserveDelayLoadFailure) {
EchoServiceProcessObserver observer;
auto echo_service =
content::ServiceProcessHost::Launch<echo::mojom::EchoService>();
observer.WaitForLaunch();
echo_service->DelayLoad();
int exit_code = observer.WaitForCrash();
EXPECT_EQ(EXCEPTION_BREAKPOINT, static_cast<DWORD>(exit_code));
}

} // namespace chrome
19 changes: 17 additions & 2 deletions services/test/echo/echo_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@

#include "services/test/echo/echo_service.h"

#include <string.h>

#include "base/immediate_crash.h"
#include "base/memory/shared_memory_mapping.h"
#include "build/build_config.h"

#if BUILDFLAG(IS_WIN)
#include <windows.h>
#include <winevt.h>
#endif

#include <string>

namespace echo {

Expand Down Expand Up @@ -36,4 +42,13 @@ void EchoService::Crash() {
IMMEDIATE_CRASH();
}

#if BUILDFLAG(IS_WIN)
void EchoService::DelayLoad() {
// This causes wevtapi.dll to be delay loaded. It should not work from inside
// a sandboxed process.
EVT_HANDLE handle = ::EvtCreateRenderContext(0, nullptr, 0);
::EvtClose(handle);
}
#endif // BUILDFLAG(IS_WIN)

} // namespace echo
4 changes: 4 additions & 0 deletions services/test/echo/echo_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef SERVICES_TEST_ECHO_ECHO_SERVICE_H_
#define SERVICES_TEST_ECHO_ECHO_SERVICE_H_

#include "build/build_config.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "services/test/echo/public/mojom/echo.mojom.h"
Expand All @@ -28,6 +29,9 @@ class EchoService : public mojom::EchoService {
base::UnsafeSharedMemoryRegion region) override;
void Quit() override;
void Crash() override;
#if BUILDFLAG(IS_WIN)
void DelayLoad() override;
#endif

mojo::Receiver<mojom::EchoService> receiver_;
};
Expand Down
4 changes: 4 additions & 0 deletions services/test/echo/public/mojom/echo.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@ interface EchoService {

// Forcibly crashes the service.
Crash();

// Delay Load a DLL.
[EnableIf=is_win]
DelayLoad();
};

0 comments on commit 80db05b

Please sign in to comment.