[Windows] Fix and remove sandbox tests.
This CL fixes or removes some tests from the sandbox. This includes the
following:
* Removed tests checking for Windows behaviors, only check mitigations.
* Reworked ConvertToLongPath to not need admin access.
* Changed desired file access to what's needed rather than everything
which can fail depending on the permissions on the temporary directory.
Bug: 1270309
Bug: 1278912
Change-Id: Ia53c2c5ba24db1b487be14243cde95b62c3bbd4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3329667
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: James Forshaw <forshaw@chromium.org>
Cr-Commit-Position: refs/heads/main@{#950988}
NOKEYCHECK=True
GitOrigin-RevId: 79b6564b99e48640bef3828678ef0b86b792f1b7
diff --git a/win/BUILD.gn b/win/BUILD.gn
index 681a894..1eecd58 100644
--- a/win/BUILD.gn
+++ b/win/BUILD.gn
@@ -184,7 +184,6 @@
"src/policy_target_test.cc",
"src/process_mitigations_dyncode_unittest.cc",
"src/process_mitigations_extensionpoints_unittest.cc",
- "src/process_mitigations_imageload_unittest.cc",
"src/process_mitigations_unittest.cc",
"src/process_mitigations_win32k_unittest.cc",
"src/process_policy_test.cc",
@@ -207,8 +206,6 @@
]
data_deps = [
- ":sbox_integration_test_hijack_dll",
- ":sbox_integration_test_hijack_shim_dll",
":sbox_integration_test_hooking_dll",
":sbox_integration_test_win_proc",
]
@@ -219,15 +216,6 @@
]
}
-shared_library("sbox_integration_test_hijack_dll") {
- sources = [
- "tests/integration_tests/hijack_dll.cc",
- "tests/integration_tests/hijack_dll.def",
- ]
-
- deps = [ ":maybe_set_appcontainer_acls" ]
-}
-
group("maybe_set_appcontainer_acls") {
# NACL on 32-bit builds this target twice, once for 64-bit and once for 32-bit
# so avoid this dep from running twice with the same output in that case.
@@ -251,20 +239,6 @@
}
}
-shared_library("sbox_integration_test_hijack_shim_dll") {
- sources = [
- "tests/integration_tests/hijack_shim_dll.cc",
- "tests/integration_tests/hijack_shim_dll.def",
- ]
-
- # Implicitly linking hijack_dll as loader import resolution required.
- deps = [
- ":common",
- ":sbox_integration_test_hijack_dll",
- "//base",
- ]
-}
-
shared_library("sbox_integration_test_hooking_dll") {
sources = [ "tests/integration_tests/hooking_dll.cc" ]
@@ -344,7 +318,6 @@
"src/sandbox_types.h",
"src/security_level.h",
"tests/common/controller.h",
- "tests/integration_tests/hijack_shim_dll.h",
]
deps = [ "//base" ]
diff --git a/win/src/app_container_test.cc b/win/src/app_container_test.cc
index d942eab..5eef889 100644
--- a/win/src/app_container_test.cc
+++ b/win/src/app_container_test.cc
@@ -631,10 +631,10 @@
}
SBOX_TESTS_COMMAND int LoadDLL(int argc, wchar_t** argv) {
- // DLL here doesn't matter as long as it's in the output directory: re-use one
- // from another sbox test.
- base::ScopedNativeLibrary test_dll(base::FilePath(
- FILE_PATH_LITERAL("sbox_integration_test_hijack_dll.dll")));
+ // Library here doesn't matter as long as it's in the output directory: re-use
+ // one from another sbox test.
+ base::ScopedNativeLibrary test_dll(
+ base::FilePath(FILE_PATH_LITERAL("sbox_integration_test_win_proc.exe")));
if (test_dll.is_valid())
return SBOX_TEST_SUCCEEDED;
return SBOX_TEST_FAILED;
diff --git a/win/src/file_policy_test.cc b/win/src/file_policy_test.cc
index b061c04..feef97e 100644
--- a/win/src/file_policy_test.cc
+++ b/win/src/file_policy_test.cc
@@ -48,8 +48,9 @@
return file1.IsValid() ? SBOX_TEST_FIRST_ERROR : SBOX_TEST_SECOND_ERROR;
} else if (operation == L"Write") {
- base::win::ScopedHandle file1(CreateFile(
- argv[1], GENERIC_ALL, kSharing, nullptr, OPEN_EXISTING, 0, nullptr));
+ base::win::ScopedHandle file1(
+ CreateFile(argv[1], GENERIC_READ | GENERIC_WRITE | GENERIC_EXECUTE,
+ kSharing, nullptr, OPEN_EXISTING, 0, nullptr));
base::win::ScopedHandle file2(
CreateFile(argv[1], GENERIC_READ | FILE_WRITE_DATA, kSharing, nullptr,
OPEN_EXISTING, 0, nullptr));
@@ -593,7 +594,7 @@
std::wstring temp_file_title = subfolder.substr(subfolder.rfind(L"\\") + 1);
std::wstring temp_file = subfolder + L"\\file_" + temp_file_title;
- HANDLE file = ::CreateFile(temp_file.c_str(), FILE_ALL_ACCESS,
+ HANDLE file = ::CreateFile(temp_file.c_str(), FILE_WRITE_DATA,
FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr,
CREATE_ALWAYS, 0, nullptr);
ASSERT_TRUE(INVALID_HANDLE_VALUE != file);
@@ -602,7 +603,7 @@
// Create a temporary file in the temp directory.
std::wstring temp_dir = temp_directory;
std::wstring temp_file_in_temp = temp_dir + L"file_" + temp_file_title;
- file = ::CreateFile(temp_file_in_temp.c_str(), FILE_ALL_ACCESS,
+ file = ::CreateFile(temp_file_in_temp.c_str(), FILE_WRITE_DATA,
FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr,
CREATE_ALWAYS, 0, nullptr);
ASSERT_TRUE(INVALID_HANDLE_VALUE != file);
@@ -624,7 +625,7 @@
// Replace the subfolder by a reparse point to %temp%.
::DeleteFile(temp_file.c_str());
- HANDLE dir = ::CreateFile(subfolder.c_str(), FILE_ALL_ACCESS,
+ HANDLE dir = ::CreateFile(subfolder.c_str(), FILE_WRITE_DATA,
FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr);
EXPECT_TRUE(INVALID_HANDLE_VALUE != dir);
@@ -639,7 +640,7 @@
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command_write.c_str()));
// Remove the reparse point.
- dir = ::CreateFile(subfolder.c_str(), FILE_ALL_ACCESS,
+ dir = ::CreateFile(subfolder.c_str(), FILE_WRITE_DATA,
FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING,
FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,
nullptr);
diff --git a/win/src/process_mitigations_imageload_unittest.cc b/win/src/process_mitigations_imageload_unittest.cc
deleted file mode 100644
index f509466..0000000
--- a/win/src/process_mitigations_imageload_unittest.cc
+++ /dev/null
@@ -1,462 +0,0 @@
-// Copyright 2017 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 "sandbox/win/src/process_mitigations.h"
-
-#include <windows.h>
-
-#include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
-#include "base/path_service.h"
-#include "base/scoped_native_library.h"
-#include "base/strings/stringprintf.h"
-#include "base/test/test_timeouts.h"
-#include "base/win/windows_version.h"
-#include "sandbox/win/src/sandbox.h"
-#include "sandbox/win/src/target_services.h"
-#include "sandbox/win/tests/common/controller.h"
-#include "sandbox/win/tests/integration_tests/hijack_shim_dll.h"
-#include "sandbox/win/tests/integration_tests/integration_tests_common.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace {
-
-//------------------------------------------------------------------------------
-// Internal Defines & Functions
-//------------------------------------------------------------------------------
-constexpr wchar_t g_hijack_dll_file[] = L"sbox_integration_test_hijack_dll.dll";
-constexpr wchar_t g_hijack_shim_dll_file[] =
- L"sbox_integration_test_hijack_shim_dll.dll";
-
-// System mutex to prevent conflicting tests from running at the same time.
-// This particular mutex is related to the use of the hijack dlls.
-constexpr wchar_t g_hijack_dlls_mutex[] = L"ChromeTestHijackDllsMutex";
-
-// API defined in hijack_shim_dll.h.
-using CheckHijackResultFunction = decltype(&CheckHijackResult);
-
-//------------------------------------------------------------------------------
-// ImageLoadRemote test helper function.
-//
-// Trigger test child process (with or without mitigation enabled).
-//------------------------------------------------------------------------------
-void TestWin10ImageLoadRemote(bool is_success_test) {
- // ***Insert a manual testing share UNC path here!
- // E.g.: \\\\hostname\\sharename\\calc.exe
- std::wstring unc = L"\"\\\\hostname\\sharename\\calc.exe\"";
-
- sandbox::TestRunner runner;
- sandbox::TargetPolicy* policy = runner.GetPolicy();
-
- // Set a policy that would normally allow for process creation.
- policy->SetJobLevel(sandbox::JOB_NONE, 0);
- policy->SetTokenLevel(sandbox::USER_UNPROTECTED, sandbox::USER_UNPROTECTED);
- runner.SetDisableCsrss(false);
-
- if (!is_success_test) {
- // Enable the NoRemote mitigation.
- EXPECT_EQ(policy->SetDelayedProcessMitigations(
- sandbox::MITIGATION_IMAGE_LOAD_NO_REMOTE),
- sandbox::SBOX_ALL_OK);
- }
-
- std::wstring test = L"TestChildProcess ";
- test += unc.c_str();
- EXPECT_EQ((is_success_test ? sandbox::SBOX_TEST_SUCCEEDED
- : sandbox::SBOX_TEST_FAILED),
- runner.RunTest(test.c_str()));
-}
-
-//------------------------------------------------------------------------------
-// ImageLoadLow test helper function.
-//
-// 1. Set up a copy of calc, using icacls to make it low integrity.
-// 2. Trigger test child process (with or without mitigation enabled).
-//------------------------------------------------------------------------------
-void TestWin10ImageLoadLowLabel(bool is_success_test) {
- // Setup a mandatory low executable for this test (calc.exe).
- // If anything fails during setup, ASSERT to end test.
- base::FilePath orig_path;
- ASSERT_TRUE(base::PathService::Get(base::DIR_SYSTEM, &orig_path));
- orig_path = orig_path.Append(L"calc.exe");
-
- base::ScopedTempDir temp_dir;
- ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- base::FilePath new_path = temp_dir.GetPath();
- new_path = new_path.Append(L"lowIL_calc.exe");
-
- // Test file will be cleaned up by the ScopedTempDir.
- ASSERT_TRUE(base::CopyFileW(orig_path, new_path));
-
- std::wstring cmd_line = L"icacls \"";
- cmd_line += new_path.value().c_str();
- cmd_line += L"\" /setintegritylevel Low";
-
- base::LaunchOptions options = base::LaunchOptionsForTest();
- base::Process setup_proc = base::LaunchProcess(cmd_line.c_str(), options);
- ASSERT_TRUE(setup_proc.IsValid());
-
- int exit_code = 1;
- if (!setup_proc.WaitForExitWithTimeout(TestTimeouts::action_max_timeout(),
- &exit_code)) {
- // Might have timed out, or might have failed.
- // Terminate to make sure we clean up any mess.
- setup_proc.Terminate(0, false);
- ASSERT_TRUE(false);
- }
- // Make sure icacls was successful.
- ASSERT_EQ(0, exit_code);
-
- sandbox::TestRunner runner;
- sandbox::TargetPolicy* policy = runner.GetPolicy();
-
- // Set a policy that would normally allow for process creation.
- policy->SetJobLevel(sandbox::JOB_NONE, 0);
- policy->SetTokenLevel(sandbox::USER_UNPROTECTED, sandbox::USER_UNPROTECTED);
- runner.SetDisableCsrss(false);
-
- if (!is_success_test) {
- // Enable the NoLowLabel mitigation.
- EXPECT_EQ(policy->SetDelayedProcessMitigations(
- sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL),
- sandbox::SBOX_ALL_OK);
- }
-
- std::wstring test = L"TestChildProcess \"";
- test += new_path.value().c_str();
- test += L"\" false";
-
- EXPECT_EQ((is_success_test ? sandbox::SBOX_TEST_SUCCEEDED
- : sandbox::SBOX_TEST_FAILED),
- runner.RunTest(test.c_str()));
-}
-
-//------------------------------------------------------------------------------
-// ImageLoadPreferSystem32 test helper function.
-//
-// - Acquire the global g_hijack_dlls_mutex mutex before calling
-// (as we meddle with a shared system resource).
-// - Note: Do not use ASSERTs in this function, as a global mutex is held.
-// - If |baseline_test|, there is no attempted hijacking. Just test the
-// implicit import in the local directory.
-//
-// 1. Put a copy of the hijack DLL into system32.
-// 2. Trigger test child process (with or without mitigation enabled). When
-// the OS resolves the import table for the child process, it will either
-// choose the version in the local app directory, or the copy in system32.
-//------------------------------------------------------------------------------
-void TestWin10ImageLoadPreferSys32(bool baseline_test, bool expect_sys32_path) {
- base::FilePath app_path;
- EXPECT_TRUE(base::PathService::Get(base::DIR_EXE, &app_path));
-
- // Put a copy of the hijack dll into system32. So there's one in the
- // local dir, and one in system32.
- base::FilePath old_dll_path = app_path.Append(g_hijack_dll_file);
-
- base::FilePath sys_path;
- EXPECT_TRUE(base::PathService::Get(base::DIR_SYSTEM, &sys_path));
- base::FilePath new_dll_path = sys_path.Append(g_hijack_dll_file);
-
- // Note: test requires admin to copy/delete files in system32.
- EXPECT_TRUE(base::CopyFileW(old_dll_path, new_dll_path));
-
- // Get path for the test shim DLL to pass along to test child proc.
- base::FilePath shim_dll_path = app_path.Append(g_hijack_shim_dll_file);
-
- sandbox::TestRunner runner;
- sandbox::TargetPolicy* policy = runner.GetPolicy();
-
- // ACCESS_DENIED errors loading DLLs without a higher token - AddFsRule
- // alone doesn't cut it.
- policy->SetTokenLevel(sandbox::TokenLevel::USER_RESTRICTED_SAME_ACCESS,
- sandbox::TokenLevel::USER_LIMITED);
- runner.AddFsRule(sandbox::TargetPolicy::FILES_ALLOW_READONLY,
- shim_dll_path.value().c_str());
- runner.AddFsRule(sandbox::TargetPolicy::FILES_ALLOW_READONLY,
- old_dll_path.value().c_str());
- runner.AddFsRule(sandbox::TargetPolicy::FILES_ALLOW_READONLY,
- new_dll_path.value().c_str());
-
- if (!baseline_test && expect_sys32_path) {
- // Enable the PreferSystem32 mitigation.
- EXPECT_EQ(policy->SetDelayedProcessMitigations(
- sandbox::MITIGATION_IMAGE_LOAD_PREFER_SYS32),
- sandbox::SBOX_ALL_OK);
- }
-
- // For the baseline test, disable sandbox completely. Just trying to ensure
- // that the implicit link & import succeeds from local dir.
- if (baseline_test)
- runner.SetUnsandboxed(true);
-
- // If this is the "success" test, expect a return value of "false" - as the
- // hijack was successful so the hijack_dll is NOT in system32.
- // The failure case has the mitigation enabled, so expect the hijack_dll to be
- // in system32.
- std::wstring test = base::StringPrintf(
- L"%ls %ls \"%ls\"", L"TestImageLoadHijack",
- (!expect_sys32_path) ? L"true" : L"false", shim_dll_path.value().c_str());
-
- // NOTE: 1706098690 == SBOX_TEST_NOT_FOUND == LoadLibrary on the shim dll
- // failed, or the secondary import load of hijack.dll failed.
- EXPECT_EQ((expect_sys32_path ? sandbox::SBOX_TEST_SUCCEEDED
- : sandbox::SBOX_TEST_FAILED),
- runner.RunTest(test.c_str()));
-
- EXPECT_TRUE(base::DeleteFile(new_dll_path));
-}
-
-} // namespace
-
-namespace sandbox {
-
-//------------------------------------------------------------------------------
-// Exported functions called by child test processes.
-//------------------------------------------------------------------------------
-
-// This test loading and using the shim DLL is required.
-// The waterfall bots (test environment/harness) have unique resource
-// access failures if the main sbox_integration_tests executable
-// implicitely links against the hijack DLL (and implicit linking is required
-// to test this mitigation) - regardless of whether this test runs or is
-// disabled.
-//
-// - Arg1: "true" or "false", if the DLL path should be in system32.
-// - Arg2: the full path to the test shim DLL to load.
-SBOX_TESTS_COMMAND int TestImageLoadHijack(int argc, wchar_t** argv) {
- if (argc < 2 || !argv[0] || !argv[1])
- return SBOX_TEST_INVALID_PARAMETER;
-
- bool expect_system = false;
- if (::wcsicmp(argv[0], L"true") == 0)
- expect_system = true;
-
- // Ensure implicitely linked, secondary hijack DLL is not already
- // loaded in process.
- HMODULE check = ::GetModuleHandleW(g_hijack_dll_file);
- if (check)
- return SBOX_TEST_FAILED;
-
- // Load the shim DLL for this test.
- base::ScopedNativeLibrary shim_dll((base::FilePath(argv[1])));
- if (!shim_dll.is_valid())
- return SBOX_TEST_NOT_FOUND;
-
- CheckHijackResultFunction check_hijack_result =
- reinterpret_cast<CheckHijackResultFunction>(
- shim_dll.GetFunctionPointer(g_hijack_shim_func));
-
- if (!check_hijack_result)
- return SBOX_TEST_FAILED_TO_RUN_TEST;
-
- return check_hijack_result(expect_system);
-}
-
-//------------------------------------------------------------------------------
-// Exported Image Load Tests
-//------------------------------------------------------------------------------
-
-//------------------------------------------------------------------------------
-// Disable image load from remote devices (MITIGATION_IMAGE_LOAD_NO_REMOTE).
-// >= Win10_TH2
-//------------------------------------------------------------------------------
-
-// This test validates that setting the MITIGATION_IMAGE_LOAD_NO_REMOTE
-// mitigation enables the setting on a process.
-TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoRemotePolicySuccess) {
- if (base::win::GetVersion() < base::win::Version::WIN10_TH2)
- return;
-
- std::wstring test_command = L"CheckPolicy ";
- test_command += std::to_wstring(TESTPOLICY_LOADNOREMOTE);
-
- //---------------------------------
- // 1) Test setting pre-startup.
- //---------------------------------
- TestRunner runner;
- sandbox::TargetPolicy* policy = runner.GetPolicy();
-
- EXPECT_EQ(policy->SetProcessMitigations(MITIGATION_IMAGE_LOAD_NO_REMOTE),
- SBOX_ALL_OK);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(test_command.c_str()));
-
- //---------------------------------
- // 2) Test setting post-startup.
- //---------------------------------
- TestRunner runner2;
- sandbox::TargetPolicy* policy2 = runner2.GetPolicy();
-
- EXPECT_EQ(
- policy2->SetDelayedProcessMitigations(MITIGATION_IMAGE_LOAD_NO_REMOTE),
- SBOX_ALL_OK);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str()));
-}
-
-// This test validates that we CAN create a new process from
-// a remote UNC device, if the MITIGATION_IMAGE_LOAD_NO_REMOTE
-// mitigation is NOT set.
-//
-// MANUAL testing only.
-TEST(ProcessMitigationsTest, DISABLED_CheckWin10ImageLoadNoRemoteSuccess) {
- if (base::win::GetVersion() < base::win::Version::WIN10_TH2)
- return;
-
- TestWin10ImageLoadRemote(true /* is_success_test */);
-}
-
-// This test validates that setting the MITIGATION_IMAGE_LOAD_NO_REMOTE
-// mitigation prevents creating a new process from a remote
-// UNC device.
-//
-// MANUAL testing only.
-TEST(ProcessMitigationsTest, DISABLED_CheckWin10ImageLoadNoRemoteFailure) {
- if (base::win::GetVersion() < base::win::Version::WIN10_TH2)
- return;
-
- TestWin10ImageLoadRemote(false /* is_success_test */);
-}
-
-//------------------------------------------------------------------------------
-// Disable image load when "mandatory low label" (integrity level).
-// (MITIGATION_IMAGE_LOAD_NO_LOW_LABEL)
-// >= Win10_TH2
-//------------------------------------------------------------------------------
-
-// This test validates that setting the MITIGATION_IMAGE_LOAD_NO_LOW_LABEL
-// mitigation enables the setting on a process.
-TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoLowLabelPolicySuccess) {
- if (base::win::GetVersion() < base::win::Version::WIN10_TH2)
- return;
-
- std::wstring test_command = L"CheckPolicy ";
- test_command += std::to_wstring(TESTPOLICY_LOADNOLOW);
-
- //---------------------------------
- // 1) Test setting pre-startup.
- //---------------------------------
- TestRunner runner;
- sandbox::TargetPolicy* policy = runner.GetPolicy();
-
- EXPECT_EQ(policy->SetProcessMitigations(MITIGATION_IMAGE_LOAD_NO_LOW_LABEL),
- SBOX_ALL_OK);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(test_command.c_str()));
-
- //---------------------------------
- // 2) Test setting post-startup.
- //---------------------------------
- TestRunner runner2;
- sandbox::TargetPolicy* policy2 = runner2.GetPolicy();
-
- EXPECT_EQ(
- policy2->SetDelayedProcessMitigations(MITIGATION_IMAGE_LOAD_NO_LOW_LABEL),
- SBOX_ALL_OK);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str()));
-}
-
-// This test validates that we CAN create a new process with
-// low mandatory label (IL), if the MITIGATION_IMAGE_LOAD_NO_LOW_LABEL
-// mitigation is NOT set.
-TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoLowLabelSuccess) {
- if (base::win::GetVersion() < base::win::Version::WIN10_TH2)
- return;
-
- TestWin10ImageLoadLowLabel(true /* is_success_test */);
-}
-
-// This test validates that setting the MITIGATION_IMAGE_LOAD_NO_LOW_LABEL
-// mitigation prevents creating a new process with low mandatory label (IL).
-TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoLowLabelFailure) {
- if (base::win::GetVersion() < base::win::Version::WIN10_TH2)
- return;
-
- TestWin10ImageLoadLowLabel(false /* is_success_test */);
-}
-
-//------------------------------------------------------------------------------
-// Prefer system32 directory on image load (MITIGATION_IMAGE_LOAD_PREFER_SYS32).
-// >= Win10_RS1 (Anniversary)
-//------------------------------------------------------------------------------
-
-// This test validates that setting the MITIGATION_IMAGE_LOAD_PREFER_SYS32
-// mitigation enables the setting on a process.
-TEST(ProcessMitigationsTest, CheckWin10ImageLoadPreferSys32PolicySuccess) {
- if (base::win::GetVersion() < base::win::Version::WIN10_RS1)
- return;
-
- std::wstring test_command = L"CheckPolicy ";
- test_command += std::to_wstring(TESTPOLICY_LOADPREFERSYS32);
-
- //---------------------------------
- // 1) Test setting pre-startup.
- // ** Currently disabled. All PreferSys32 tests start to explode on
- // >= Win10 1703/RS2 when this mitigation is set pre-startup.
- // Child process creation works fine, but when ::ResumeThread() is called,
- // there is a fatal error: "Entry point ucnv_convertEx_60 could not be
- // located in the DLL ... sbox_integration_tests.exe."
- // This is a character conversion function in a ucnv (unicode) DLL.
- // Potentially the loader is finding a different version of this DLL that
- // we have a dependency on in System32... but it doesn't match up with
- // what we build against???!
- //---------------------------------
-
- //---------------------------------
- // 2) Test setting post-startup.
- //---------------------------------
- TestRunner runner2;
- sandbox::TargetPolicy* policy2 = runner2.GetPolicy();
-
- EXPECT_EQ(
- policy2->SetDelayedProcessMitigations(MITIGATION_IMAGE_LOAD_PREFER_SYS32),
- SBOX_ALL_OK);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str()));
-}
-
-// This baseline test validates that the implicit import works in the local
-// working directory.
-// MITIGATION_IMAGE_LOAD_PREFER_SYS32 mitigation is NOT set.
-//
-// Must run this test as admin/elevated.
-TEST(ProcessMitigationsTest, CheckWin10ImageLoadPreferSys32_Baseline) {
- if (base::win::GetVersion() < base::win::Version::WIN10_RS1)
- return;
-
- ScopedTestMutex mutex(g_hijack_dlls_mutex);
-
- // Baseline test, and expect the DLL to NOT be in system32.
- TestWin10ImageLoadPreferSys32(true /* baseline_test */,
- false /* expect_sys32_path */);
-}
-
-// This test validates that import hijacking succeeds, if the
-// MITIGATION_IMAGE_LOAD_PREFER_SYS32 mitigation is NOT set.
-// (Hijack success.)
-//
-// Must run this test as admin/elevated.
-TEST(ProcessMitigationsTest, CheckWin10ImageLoadPreferSys32_Success) {
- if (base::win::GetVersion() < base::win::Version::WIN10_RS1)
- return;
-
- ScopedTestMutex mutex(g_hijack_dlls_mutex);
-
- // Not a baseline test, and expect the DLL to be in system32.
- TestWin10ImageLoadPreferSys32(false /* baseline_test */,
- true /* expect_sys32_path */);
-}
-
-// This test validates that setting the MITIGATION_IMAGE_LOAD_PREFER_SYS32
-// mitigation prevents import hijacking. (Hijack failure.)
-//
-// Must run this test as admin/elevated.
-TEST(ProcessMitigationsTest, CheckWin10ImageLoadPreferSys32_Failure) {
- if (base::win::GetVersion() < base::win::Version::WIN10_RS1)
- return;
-
- ScopedTestMutex mutex(g_hijack_dlls_mutex);
-
- // Not a baseline test, and expect the DLL to NOT be in system32.
- TestWin10ImageLoadPreferSys32(false /* baseline_test */,
- false /* expect_sys32_path */);
-}
-
-} // namespace sandbox
diff --git a/win/src/process_mitigations_unittest.cc b/win/src/process_mitigations_unittest.cc
index 3113831..1c71649 100644
--- a/win/src/process_mitigations_unittest.cc
+++ b/win/src/process_mitigations_unittest.cc
@@ -1298,4 +1298,99 @@
EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(test_policy_command.c_str()));
}
+TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoRemotePolicySuccess) {
+ if (base::win::GetVersion() < base::win::Version::WIN10_TH2)
+ return;
+
+ std::wstring test_command = L"CheckPolicy ";
+ test_command += std::to_wstring(TESTPOLICY_LOADNOREMOTE);
+
+ //---------------------------------
+ // 1) Test setting pre-startup.
+ //---------------------------------
+ TestRunner runner;
+ sandbox::TargetPolicy* policy = runner.GetPolicy();
+
+ EXPECT_EQ(policy->SetProcessMitigations(MITIGATION_IMAGE_LOAD_NO_REMOTE),
+ SBOX_ALL_OK);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(test_command.c_str()));
+
+ //---------------------------------
+ // 2) Test setting post-startup.
+ //---------------------------------
+ TestRunner runner2;
+ sandbox::TargetPolicy* policy2 = runner2.GetPolicy();
+
+ EXPECT_EQ(
+ policy2->SetDelayedProcessMitigations(MITIGATION_IMAGE_LOAD_NO_REMOTE),
+ SBOX_ALL_OK);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str()));
+}
+
+//---------------
+// This test validates that setting the MITIGATION_IMAGE_LOAD_NO_LOW_LABEL
+// mitigation enables the setting on a process.
+TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoLowLabelPolicySuccess) {
+ if (base::win::GetVersion() < base::win::Version::WIN10_TH2)
+ return;
+
+ std::wstring test_command = L"CheckPolicy ";
+ test_command += std::to_wstring(TESTPOLICY_LOADNOLOW);
+
+ //---------------------------------
+ // 1) Test setting pre-startup.
+ //---------------------------------
+ TestRunner runner;
+ sandbox::TargetPolicy* policy = runner.GetPolicy();
+
+ EXPECT_EQ(policy->SetProcessMitigations(MITIGATION_IMAGE_LOAD_NO_LOW_LABEL),
+ SBOX_ALL_OK);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(test_command.c_str()));
+
+ //---------------------------------
+ // 2) Test setting post-startup.
+ //---------------------------------
+ TestRunner runner2;
+ sandbox::TargetPolicy* policy2 = runner2.GetPolicy();
+
+ EXPECT_EQ(
+ policy2->SetDelayedProcessMitigations(MITIGATION_IMAGE_LOAD_NO_LOW_LABEL),
+ SBOX_ALL_OK);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str()));
+}
+
+// This test validates that setting the MITIGATION_IMAGE_LOAD_PREFER_SYS32
+// mitigation enables the setting on a process.
+TEST(ProcessMitigationsTest, CheckWin10ImageLoadPreferSys32PolicySuccess) {
+ if (base::win::GetVersion() < base::win::Version::WIN10_RS1)
+ return;
+
+ std::wstring test_command = L"CheckPolicy ";
+ test_command += std::to_wstring(TESTPOLICY_LOADPREFERSYS32);
+
+ //---------------------------------
+ // 1) Test setting pre-startup.
+ // ** Currently disabled. All PreferSys32 tests start to explode on
+ // >= Win10 1703/RS2 when this mitigation is set pre-startup.
+ // Child process creation works fine, but when ::ResumeThread() is called,
+ // there is a fatal error: "Entry point ucnv_convertEx_60 could not be
+ // located in the DLL ... sbox_integration_tests.exe."
+ // This is a character conversion function in a ucnv (unicode) DLL.
+ // Potentially the loader is finding a different version of this DLL that
+ // we have a dependency on in System32... but it doesn't match up with
+ // what we build against???!
+ //---------------------------------
+
+ //---------------------------------
+ // 2) Test setting post-startup.
+ //---------------------------------
+ TestRunner runner2;
+ sandbox::TargetPolicy* policy2 = runner2.GetPolicy();
+
+ EXPECT_EQ(
+ policy2->SetDelayedProcessMitigations(MITIGATION_IMAGE_LOAD_PREFER_SYS32),
+ SBOX_ALL_OK);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str()));
+}
+
} // namespace sandbox
diff --git a/win/src/win_utils_unittest.cc b/win/src/win_utils_unittest.cc
index da82ec1..b81721c 100644
--- a/win/src/win_utils_unittest.cc
+++ b/win/src/win_utils_unittest.cc
@@ -12,6 +12,7 @@
#include "base/files/file_path.h"
#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
#include "base/numerics/safe_conversions.h"
#include "base/path_service.h"
#include "base/win/scoped_handle.h"
@@ -61,7 +62,6 @@
TEST(WinUtils, IsReparsePoint) {
using sandbox::IsReparsePoint;
-
// Create a temp file because we need write access to it.
wchar_t temp_directory[MAX_PATH];
wchar_t my_folder[MAX_PATH];
@@ -84,7 +84,7 @@
IsReparsePoint(new_file));
// Replace the directory with a reparse point to %temp%.
- HANDLE dir = ::CreateFile(my_folder, FILE_ALL_ACCESS,
+ HANDLE dir = ::CreateFile(my_folder, FILE_WRITE_DATA,
FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr);
EXPECT_NE(INVALID_HANDLE_VALUE, dir);
@@ -207,19 +207,20 @@
ASSERT_TRUE(base::PathService::Get(base::DIR_SYSTEM, &orig_path));
orig_path = orig_path.Append(L"calc.exe");
- base::FilePath temp_path;
- ASSERT_TRUE(base::PathService::Get(base::DIR_PROGRAM_FILES, &temp_path));
- temp_path = temp_path.Append(L"test_calc.exe");
+ base::ScopedTempDir temp_dir;
+ base::FilePath base_path;
+ ASSERT_TRUE(base::PathService::Get(base::DIR_COMMON_APP_DATA, &base_path));
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDirUnderPath(base_path));
+ base::FilePath temp_path = temp_dir.GetPath().Append(L"test_calc.exe");
ASSERT_TRUE(base::CopyFile(orig_path, temp_path));
- // No more asserts until cleanup.
- // WIN32 long path: "c:\Program Files\test_calc.exe"
+ // WIN32 long path: "C:\ProgramData\%TEMP%\test_calc.exe"
wchar_t short_path[MAX_PATH] = {};
DWORD size =
::GetShortPathNameW(temp_path.value().c_str(), short_path, MAX_PATH);
EXPECT_TRUE(size > 0 && size < MAX_PATH);
- // WIN32 short path: "C:\PROGRA~1\TEST_C~1.exe"
+ // WIN32 short path: "C:\PROGRA~3\%TEMP%\TEST_C~1.exe"
// Sanity check that we actually got a short path above! Small chance
// it was disabled in the filesystem setup.
@@ -228,13 +229,13 @@
std::wstring short_form_native_path;
EXPECT_TRUE(sandbox::GetNtPathFromWin32Path(std::wstring(short_path),
&short_form_native_path));
- // NT short path: "\Device\HarddiskVolume4\PROGRA~1\TEST_C~1.EXE"
+ // NT short path: "\Device\HarddiskVolume4\PROGRA~3\%TEMP%\TEST_C~1.EXE"
// Test 1: convert win32 short path to long:
std::wstring test1(short_path);
EXPECT_TRUE(sandbox::ConvertToLongPath(&test1));
EXPECT_TRUE(::wcsicmp(temp_path.value().c_str(), test1.c_str()) == 0);
- // Expected result: "c:\Program Files\test_calc.exe"
+ // Expected result: "C:\ProgramData\%TEMP%\test_calc.exe"
// Test 2: convert native short path to long:
std::wstring drive_letter = temp_path.value().substr(0, 3);
@@ -247,10 +248,7 @@
std::wstring expected_result = short_form_native_path.substr(0, index + 1);
expected_result.append(temp_path.value().substr(3));
EXPECT_TRUE(::wcsicmp(expected_result.c_str(), test2.c_str()) == 0);
- // Expected result: "\Device\HarddiskVolumeX\Program Files\test_calc.exe"
-
- // clean up
- EXPECT_TRUE(base::DeleteFile(temp_path));
+ // Expected result: "\Device\HarddiskVolumeX\ProgramData\%TEMP%\test_calc.exe"
}
} // namespace sandbox
diff --git a/win/tests/integration_tests/hijack_dll.cc b/win/tests/integration_tests/hijack_dll.cc
deleted file mode 100644
index 5ea4c7b..0000000
--- a/win/tests/integration_tests/hijack_dll.cc
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright 2017 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>
-
-EXTERN_C IMAGE_DOS_HEADER __ImageBase;
-
-// Arg1: pointer to a buffer of size MAX_PATH + 1.
-bool GetPathOnDisk(wchar_t* buffer) {
- if (buffer == nullptr)
- return false;
-
- if (::GetModuleFileNameW((HINSTANCE)&__ImageBase, buffer, MAX_PATH) == 0)
- return false;
-
- return true;
-}
-
-BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, LPVOID reserved) {
- return TRUE;
-}
diff --git a/win/tests/integration_tests/hijack_dll.def b/win/tests/integration_tests/hijack_dll.def
deleted file mode 100644
index c942068..0000000
--- a/win/tests/integration_tests/hijack_dll.def
+++ /dev/null
@@ -1,9 +0,0 @@
-; Copyright 2017 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.
-
-LIBRARY "sbox_integration_test_hijack_dll.dll"
-
-EXPORTS
- GetPathOnDisk
-
diff --git a/win/tests/integration_tests/hijack_shim_dll.cc b/win/tests/integration_tests/hijack_shim_dll.cc
deleted file mode 100644
index 0cd3198..0000000
--- a/win/tests/integration_tests/hijack_shim_dll.cc
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright 2017 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 "sandbox/win/tests/common/controller.h"
-#include "sandbox/win/tests/integration_tests/hijack_shim_dll.h"
-
-// Function definition from hijack dll.
-bool GetPathOnDisk(wchar_t* buffer);
-
-// This shim implicitly links in the GetPathOnDisk function from the test
-// hijack DLL. When this DLL is loaded, the loader resolves the
-// import using standard search path. (The mitigation being tested affects the
-// standard search path.) This test then calls the exported function to
-// determine which DLL on disk was loaded.
-//
-// Arg1: "true" or "false", if the DLL path should be in system32.
-int CheckHijackResult(bool expect_system) {
- wchar_t dll_path[MAX_PATH + 1] = {};
-
- // This should always succeed.
- if (!GetPathOnDisk(dll_path))
- return sandbox::SBOX_TEST_NOT_FOUND;
-
- // Make sure the path is all one case.
- for (size_t i = 0; dll_path[i]; i++)
- dll_path[i] = ::towlower(dll_path[i]);
-
- if (::wcsstr(dll_path, L"system32") == nullptr) {
- // Not in system32.
- return (expect_system) ? sandbox::SBOX_TEST_FAILED
- : sandbox::SBOX_TEST_SUCCEEDED;
- }
-
- return (!expect_system) ? sandbox::SBOX_TEST_FAILED
- : sandbox::SBOX_TEST_SUCCEEDED;
-}
-
-BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, LPVOID reserved) {
- return TRUE;
-}
diff --git a/win/tests/integration_tests/hijack_shim_dll.def b/win/tests/integration_tests/hijack_shim_dll.def
deleted file mode 100644
index 32ff3a1..0000000
--- a/win/tests/integration_tests/hijack_shim_dll.def
+++ /dev/null
@@ -1,9 +0,0 @@
-; Copyright 2017 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.
-
-LIBRARY "sbox_integration_test_hijack_shim_dll.dll"
-
-EXPORTS
- CheckHijackResult
-
diff --git a/win/tests/integration_tests/hijack_shim_dll.h b/win/tests/integration_tests/hijack_shim_dll.h
deleted file mode 100644
index b96ae2a..0000000
--- a/win/tests/integration_tests/hijack_shim_dll.h
+++ /dev/null
@@ -1,15 +0,0 @@
-// Copyright 2017 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.
-
-#ifndef SANDBOX_WIN_TESTS_INTEGRATION_TESTS_HIJACK_SHIM_DLL_H_
-#define SANDBOX_WIN_TESTS_INTEGRATION_TESTS_HIJACK_SHIM_DLL_H_
-
-// Hijack shim dll exported API defines for dynamic lookup.
-constexpr char g_hijack_shim_func[] = "CheckHijackResult";
-
-// Arg1: "true" or "false", if the DLL path should be in system32.
-// Returns a sandbox::SboxTestResult value.
-int CheckHijackResult(bool expect_system);
-
-#endif // SANDBOX_WIN_TESTS_INTEGRATION_TESTS_HIJACK_SHIM_DLL_H_
