Prevent sandbox::TestRunner from reusing Policy
In Chrome we only use each Policy for a single process, while
in tests we often apply the same policy to multiple processes.
The critical part of this change is the CHECK in controller.cc,
everything else is introducing fresh |TestRunner|s for each call
to RunTest(). Where the runner setup was simple we introduce
multiple TestRunner within each TEST() function. Where the setup
is more complex we provide a helper returning a
std::unique_ptr<TestRunner>() and call it multiple times to reset
the runner.
Bug: 1270309,1291884
Change-Id: I353051c59e0fb43187961f435530795abd81fc08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3426302
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#967561}
NOKEYCHECK=True
GitOrigin-RevId: 137d2680a17eb69b63130b0015407ae105cb3cf2
diff --git a/win/src/app_container_test.cc b/win/src/app_container_test.cc
index 5eef889..73bb402 100644
--- a/win/src/app_container_test.cc
+++ b/win/src/app_container_test.cc
@@ -491,7 +491,8 @@
::CreateEvent(nullptr, false, false, event_name.c_str()));
ASSERT_TRUE(event.IsValid());
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"AppContainerEvent_Open test"));
+ TestRunner runner2(JOB_UNPROTECTED, USER_UNPROTECTED, USER_UNPROTECTED);
+ EXPECT_EQ(SBOX_TEST_DENIED, runner2.RunTest(L"AppContainerEvent_Open test"));
}
TEST_F(AppContainerTest, CheckIncompatibleOptions) {
diff --git a/win/src/file_policy_test.cc b/win/src/file_policy_test.cc
index 08de096..dd02b5c 100644
--- a/win/src/file_policy_test.cc
+++ b/win/src/file_policy_test.cc
@@ -277,49 +277,60 @@
EXPECT_TRUE(runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"calc.txt"));
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"File_CreateSys32 calc.exe"));
- runner.SetTestState(BEFORE_REVERT);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"File_CreateSys32 calc.exe"));
+ TestRunner before_revert;
+ EXPECT_TRUE(
+ before_revert.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"calc.txt"));
+ before_revert.SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED,
+ before_revert.RunTest(L"File_CreateSys32 calc.exe"));
}
TEST(FilePolicyTest, AllowNtCreateCalc) {
TestRunner runner;
EXPECT_TRUE(runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"calc.exe"));
-
EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"File_CreateSys32 calc.exe"));
- runner.SetTestState(BEFORE_REVERT);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"File_CreateSys32 calc.exe"));
+ TestRunner before_revert;
+ EXPECT_TRUE(
+ before_revert.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"calc.exe"));
+ before_revert.SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED,
+ before_revert.RunTest(L"File_CreateSys32 calc.exe"));
}
TEST(FilePolicyTest, AllowNtCreateWithNativePath) {
std::wstring calc = MakePathToSys(L"calc.exe", false);
std::wstring nt_path;
ASSERT_TRUE(GetNtPathFromWin32Path(calc, &nt_path));
+
TestRunner runner;
runner.AddFsRule(TargetPolicy::FILES_ALLOW_READONLY, nt_path.c_str());
-
wchar_t buff[MAX_PATH];
::wsprintfW(buff, L"File_CreateSys32 %s", nt_path.c_str());
EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(buff));
+ TestRunner runner2;
+ runner2.AddFsRule(TargetPolicy::FILES_ALLOW_READONLY, nt_path.c_str());
for (wchar_t& c : nt_path)
c = std::tolower(c);
::wsprintfW(buff, L"File_CreateSys32 %s", nt_path.c_str());
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(buff));
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(buff));
+}
+
+std::unique_ptr<TestRunner> AllowReadOnlyRunner(wchar_t* temp_file_name) {
+ auto runner = std::make_unique<TestRunner>();
+ EXPECT_TRUE(
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_READONLY, temp_file_name));
+ return runner;
}
TEST(FilePolicyTest, AllowReadOnly) {
- TestRunner runner;
-
// Create a temp file because we need write access to it.
wchar_t temp_directory[MAX_PATH];
wchar_t temp_file_name[MAX_PATH];
ASSERT_NE(::GetTempPath(MAX_PATH, temp_directory), 0u);
ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name), 0u);
- EXPECT_TRUE(
- runner.AddFsRule(TargetPolicy::FILES_ALLOW_READONLY, temp_file_name));
-
wchar_t command_read[MAX_PATH + 20] = {};
wsprintf(command_read, L"File_Create Read \"%ls\"", temp_file_name);
wchar_t command_read_create[MAX_PATH + 20] = {};
@@ -329,25 +340,27 @@
wsprintf(command_write, L"File_Create Write \"%ls\"", temp_file_name);
// Verify that we cannot create the file after revert.
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command_read_create));
+ auto runner = AllowReadOnlyRunner(temp_file_name);
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(command_read_create));
// Verify that we don't have write access after revert.
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command_write));
+ runner = AllowReadOnlyRunner(temp_file_name);
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(command_write));
// Verify that we have read access after revert.
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(command_read));
+ runner = AllowReadOnlyRunner(temp_file_name);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(command_read));
// Verify that we really have write access to the file.
- runner.SetTestState(BEFORE_REVERT);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(command_write));
+ runner = AllowReadOnlyRunner(temp_file_name);
+ runner->SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(command_write));
DeleteFile(temp_file_name);
}
// Tests support of "\\\\.\\DeviceName" kind of paths.
TEST(FilePolicyTest, AllowImplicitDeviceName) {
- TestRunner runner;
-
wchar_t temp_directory[MAX_PATH];
wchar_t temp_file_name[MAX_PATH];
ASSERT_NE(::GetTempPath(MAX_PATH, temp_directory), 0u);
@@ -362,9 +375,13 @@
wsprintf(command, L"File_Create Read \"\\\\.\\%ls\"", path.c_str());
path = std::wstring(kNTPrefix) + path;
+ TestRunner runner;
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command));
- EXPECT_TRUE(runner.AddFsRule(TargetPolicy::FILES_ALLOW_ANY, path.c_str()));
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(command));
+
+ TestRunner runner_with_rule;
+ EXPECT_TRUE(
+ runner_with_rule.AddFsRule(TargetPolicy::FILES_ALLOW_ANY, path.c_str()));
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner_with_rule.RunTest(command));
DeleteFile(temp_file_name);
}
@@ -390,18 +407,28 @@
DeleteFile(temp_file_name);
}
+std::unique_ptr<TestRunner> AllowNtCreatePatternRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ EXPECT_TRUE(runner->AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"App*.dll"));
+ return runner;
+}
+
TEST(FilePolicyTest, AllowNtCreatePatternRule) {
- TestRunner runner;
- EXPECT_TRUE(runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"App*.dll"));
-
+ auto runner = AllowNtCreatePatternRunner();
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"File_OpenSys32 apphelp.dll"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"File_OpenSys32 appwiz.cpl"));
+ runner->RunTest(L"File_OpenSys32 apphelp.dll"));
- runner.SetTestState(BEFORE_REVERT);
+ runner = AllowNtCreatePatternRunner();
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"File_OpenSys32 appwiz.cpl"));
+
+ runner = AllowNtCreatePatternRunner();
+ runner->SetTestState(BEFORE_REVERT);
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"File_OpenSys32 apphelp.dll"));
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"File_OpenSys32 appwiz.cpl"));
+ runner->RunTest(L"File_OpenSys32 apphelp.dll"));
+
+ runner = AllowNtCreatePatternRunner();
+ runner->SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"File_OpenSys32 appwiz.cpl"));
}
TEST(FilePolicyTest, CheckNotFound) {
@@ -417,30 +444,38 @@
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"File_CreateSys32 notfound.exe"));
}
+std::unique_ptr<TestRunner> QueryAttributesFileRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ EXPECT_TRUE(
+ runner->AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"apphelp.dll"));
+ EXPECT_TRUE(
+ runner->AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"notfound.exe"));
+ EXPECT_TRUE(runner->AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"drivers"));
+ EXPECT_TRUE(
+ runner->AddRuleSys32(TargetPolicy::FILES_ALLOW_QUERY, L"ipconfig.exe"));
+ return runner;
+}
+
TEST(FilePolicyTest, TestQueryAttributesFile) {
- TestRunner runner;
- EXPECT_TRUE(
- runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"apphelp.dll"));
- EXPECT_TRUE(
- runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"notfound.exe"));
- EXPECT_TRUE(runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"drivers"));
- EXPECT_TRUE(
- runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_QUERY, L"ipconfig.exe"));
-
+ auto runner = QueryAttributesFileRunner();
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"File_QueryAttributes drivers d"));
+ runner->RunTest(L"File_QueryAttributes drivers d"));
+ runner = QueryAttributesFileRunner();
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"File_QueryAttributes apphelp.dll f"));
+ runner->RunTest(L"File_QueryAttributes apphelp.dll f"));
+ runner = QueryAttributesFileRunner();
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"File_QueryAttributes ipconfig.exe f"));
+ runner->RunTest(L"File_QueryAttributes ipconfig.exe f"));
+ runner = QueryAttributesFileRunner();
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"File_QueryAttributes ftp.exe f"));
+ runner->RunTest(L"File_QueryAttributes ftp.exe f"));
+ runner = QueryAttributesFileRunner();
EXPECT_EQ(SBOX_TEST_NOT_FOUND,
- runner.RunTest(L"File_QueryAttributes notfound.exe f"));
+ runner->RunTest(L"File_QueryAttributes notfound.exe f"));
}
// Makes sure that we don't leak information when there is not policy to allow
@@ -450,121 +485,139 @@
EXPECT_EQ(SBOX_TEST_DENIED,
runner.RunTest(L"File_QueryAttributes ftp.exe f"));
+ TestRunner runner2;
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"File_QueryAttributes notfound.exe f"));
+ runner2.RunTest(L"File_QueryAttributes notfound.exe f"));
+}
+
+// Expects 8 file names. Attempts to copy even to odd files will happen.
+std::unique_ptr<TestRunner> RenameRunner(
+ std::vector<std::wstring>& temp_files) {
+ auto runner = std::make_unique<TestRunner>();
+ // Add rules to make file0->file1 succeed.
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_files[0].c_str());
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_files[1].c_str());
+
+ // Add rules to make file2->file3 fail.
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_files[2].c_str());
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_READONLY, temp_files[3].c_str());
+
+ // Add rules to make file4->file5 fail.
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_READONLY, temp_files[4].c_str());
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_files[5].c_str());
+
+ // Add rules to make file6->no_pol_file fail.
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_files[6].c_str());
+ return runner;
}
TEST(FilePolicyTest, TestRename) {
- TestRunner runner;
+ const size_t nFiles = 8;
// Give access to the temp directory.
wchar_t temp_directory[MAX_PATH];
- wchar_t temp_file_name1[MAX_PATH];
- wchar_t temp_file_name2[MAX_PATH];
- wchar_t temp_file_name3[MAX_PATH];
- wchar_t temp_file_name4[MAX_PATH];
- wchar_t temp_file_name5[MAX_PATH];
- wchar_t temp_file_name6[MAX_PATH];
- wchar_t temp_file_name7[MAX_PATH];
- wchar_t temp_file_name8[MAX_PATH];
+ std::vector<std::wstring> temp_files;
ASSERT_NE(::GetTempPath(MAX_PATH, temp_directory), 0u);
- ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name1), 0u);
- ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name2), 0u);
- ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name3), 0u);
- ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name4), 0u);
- ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name5), 0u);
- ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name6), 0u);
- ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name7), 0u);
- ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file_name8), 0u);
-
- // Add rules to make file1->file2 succeed.
- ASSERT_TRUE(runner.AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_file_name1));
- ASSERT_TRUE(runner.AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_file_name2));
-
- // Add rules to make file3->file4 fail.
- ASSERT_TRUE(runner.AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_file_name3));
- ASSERT_TRUE(
- runner.AddFsRule(TargetPolicy::FILES_ALLOW_READONLY, temp_file_name4));
-
- // Add rules to make file5->file6 fail.
- ASSERT_TRUE(
- runner.AddFsRule(TargetPolicy::FILES_ALLOW_READONLY, temp_file_name5));
- ASSERT_TRUE(runner.AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_file_name6));
-
- // Add rules to make file7->no_pol_file fail.
- ASSERT_TRUE(runner.AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_file_name7));
+ for (size_t i = 0; i < nFiles; i++) {
+ wchar_t temp_file[MAX_PATH];
+ ASSERT_NE(::GetTempFileName(temp_directory, L"test", 0, temp_file), 0u);
+ temp_files.push_back(std::wstring(temp_file));
+ }
// Delete the files where the files are going to be renamed to.
- ::DeleteFile(temp_file_name2);
- ::DeleteFile(temp_file_name4);
- ::DeleteFile(temp_file_name6);
- ::DeleteFile(temp_file_name8);
+ ::DeleteFile(temp_files[1].c_str());
+ ::DeleteFile(temp_files[3].c_str());
+ ::DeleteFile(temp_files[5].c_str());
+ ::DeleteFile(temp_files[7].c_str());
+ auto runner = RenameRunner(temp_files);
wchar_t command[MAX_PATH * 2 + 20] = {};
- wsprintf(command, L"File_Rename \"%ls\" \"%ls\"", temp_file_name1,
- temp_file_name2);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(command));
+ wsprintf(command, L"File_Rename \"%ls\" \"%ls\"", temp_files[0].c_str(),
+ temp_files[1].c_str());
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(command));
- wsprintf(command, L"File_Rename \"%ls\" \"%ls\"", temp_file_name3,
- temp_file_name4);
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command));
+ runner = RenameRunner(temp_files);
+ wsprintf(command, L"File_Rename \"%ls\" \"%ls\"", temp_files[2].c_str(),
+ temp_files[3].c_str());
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(command));
- wsprintf(command, L"File_Rename \"%ls\" \"%ls\"", temp_file_name5,
- temp_file_name6);
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command));
+ runner = RenameRunner(temp_files);
+ wsprintf(command, L"File_Rename \"%ls\" \"%ls\"", temp_files[4].c_str(),
+ temp_files[5].c_str());
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(command));
- wsprintf(command, L"File_Rename \"%ls\" \"%ls\"", temp_file_name7,
- temp_file_name8);
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command));
+ runner = RenameRunner(temp_files);
+ wsprintf(command, L"File_Rename \"%ls\" \"%ls\"", temp_files[6].c_str(),
+ temp_files[7].c_str());
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(command));
// Delete all the files in case they are still there.
- ::DeleteFile(temp_file_name1);
- ::DeleteFile(temp_file_name2);
- ::DeleteFile(temp_file_name3);
- ::DeleteFile(temp_file_name4);
- ::DeleteFile(temp_file_name5);
- ::DeleteFile(temp_file_name6);
- ::DeleteFile(temp_file_name7);
- ::DeleteFile(temp_file_name8);
+ for (auto& file : temp_files)
+ ::DeleteFile(file.c_str());
+}
+
+std::unique_ptr<TestRunner> AllowNotepadRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"notepad.exe");
+ return runner;
}
TEST(FilePolicyTest, OpenSys32FilesAllowNotepad) {
- TestRunner runner;
- EXPECT_TRUE(
- runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_ANY, L"notepad.exe"));
-
+ auto runner = AllowNotepadRunner();
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"File_Win32Create notepad.exe"));
+ runner->RunTest(L"File_Win32Create notepad.exe"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"File_Win32Create calc.exe"));
+ runner = AllowNotepadRunner();
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"File_Win32Create calc.exe"));
- runner.SetTestState(BEFORE_REVERT);
+ runner = AllowNotepadRunner();
+ runner->SetTestState(BEFORE_REVERT);
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"File_Win32Create notepad.exe"));
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"File_Win32Create calc.exe"));
+ runner->RunTest(L"File_Win32Create notepad.exe"));
+
+ runner = AllowNotepadRunner();
+ runner->SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"File_Win32Create calc.exe"));
+}
+
+std::unique_ptr<TestRunner> FileGetDiskSpaceRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->AddRuleSys32(TargetPolicy::FILES_ALLOW_READONLY, L"");
+ return runner;
}
TEST(FilePolicyTest, FileGetDiskSpace) {
- TestRunner runner;
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"File_GetDiskSpace"));
- runner.SetTestState(BEFORE_REVERT);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"File_GetDiskSpace"));
+ auto runner = std::make_unique<TestRunner>();
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"File_GetDiskSpace"));
+
+ runner = std::make_unique<TestRunner>();
+ runner->SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"File_GetDiskSpace"));
// Add an 'allow' rule in the windows\system32 such that GetDiskFreeSpaceEx
// succeeds (it does an NtOpenFile) but windows\system32\notepad.exe is
// denied since there is no wild card in the rule.
- EXPECT_TRUE(runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_READONLY, L""));
- runner.SetTestState(BEFORE_REVERT);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"File_GetDiskSpace"));
+ runner = FileGetDiskSpaceRunner();
+ runner->SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"File_GetDiskSpace"));
- runner.SetTestState(AFTER_REVERT);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"File_GetDiskSpace"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"File_Win32Create notepad.exe"));
+ runner = FileGetDiskSpaceRunner();
+ runner->SetTestState(AFTER_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"File_GetDiskSpace"));
+
+ runner = FileGetDiskSpaceRunner();
+ runner->SetTestState(AFTER_REVERT);
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"File_Win32Create notepad.exe"));
+}
+
+std::unique_ptr<TestRunner> ReparsePointRunner(
+ std::wstring& temp_dir_wildcard) {
+ auto runner = std::make_unique<TestRunner>();
+ runner->AddFsRule(TargetPolicy::FILES_ALLOW_ANY, temp_dir_wildcard.c_str());
+ return runner;
}
TEST(FilePolicyTest, TestReparsePoint) {
- TestRunner runner;
-
// Create a temp file because we need write access to it.
wchar_t temp_directory[MAX_PATH];
wchar_t temp_file_name[MAX_PATH];
@@ -597,8 +650,7 @@
// Give write access to the temp directory.
std::wstring temp_dir_wildcard = temp_dir + L"*";
- EXPECT_TRUE(runner.AddFsRule(TargetPolicy::FILES_ALLOW_ANY,
- temp_dir_wildcard.c_str()));
+ auto runner = ReparsePointRunner(temp_dir_wildcard);
// Prepare the command to execute.
std::wstring command_write;
@@ -607,7 +659,7 @@
command_write += L"\"";
// Verify that we have write access to the original file
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(command_write.c_str()));
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(command_write.c_str()));
// Replace the subfolder by a reparse point to %temp%.
::DeleteFile(temp_file.c_str());
@@ -623,7 +675,8 @@
EXPECT_TRUE(::CloseHandle(dir));
// Try to open the file again.
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command_write.c_str()));
+ runner = ReparsePointRunner(temp_dir_wildcard);
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(command_write.c_str()));
// Remove the reparse point.
dir = ::CreateFile(subfolder.c_str(), FILE_WRITE_DATA,
diff --git a/win/src/integrity_level_test.cc b/win/src/integrity_level_test.cc
index 0fa2627..b698cc1 100644
--- a/win/src/integrity_level_test.cc
+++ b/win/src/integrity_level_test.cc
@@ -42,31 +42,39 @@
return SBOX_TEST_DENIED;
}
+std::unique_ptr<TestRunner> LowILRealRunner() {
+ auto runner = std::make_unique<TestRunner>(JOB_LOCKDOWN, USER_INTERACTIVE,
+ USER_INTERACTIVE);
+ runner->SetTimeout(INFINITE);
+ runner->GetPolicy()->SetAlternateDesktop(true);
+ runner->GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
+ return runner;
+}
+
TEST(IntegrityLevelTest, TestLowILReal) {
- TestRunner runner(JOB_LOCKDOWN, USER_INTERACTIVE, USER_INTERACTIVE);
+ auto runner = LowILRealRunner();
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"CheckIntegrityLevel"));
- runner.SetTimeout(INFINITE);
+ runner = LowILRealRunner();
+ runner->SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"CheckIntegrityLevel"));
+}
- runner.GetPolicy()->SetAlternateDesktop(true);
- runner.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
-
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"CheckIntegrityLevel"));
-
- runner.SetTestState(BEFORE_REVERT);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"CheckIntegrityLevel"));
+std::unique_ptr<TestRunner> LowILDelayedRunner() {
+ auto runner = std::make_unique<TestRunner>(JOB_LOCKDOWN, USER_INTERACTIVE,
+ USER_INTERACTIVE);
+ runner->SetTimeout(INFINITE);
+ runner->GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_LOW);
+ return runner;
}
TEST(DelayedIntegrityLevelTest, TestLowILDelayed) {
- TestRunner runner(JOB_LOCKDOWN, USER_INTERACTIVE, USER_INTERACTIVE);
+ auto runner = LowILDelayedRunner();
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"CheckIntegrityLevel"));
- runner.SetTimeout(INFINITE);
-
- runner.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_LOW);
-
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"CheckIntegrityLevel"));
-
- runner.SetTestState(BEFORE_REVERT);
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"CheckIntegrityLevel"));
+ runner = LowILDelayedRunner();
+ runner->SetTestState(BEFORE_REVERT);
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"CheckIntegrityLevel"));
}
TEST(IntegrityLevelTest, TestNoILChange) {
diff --git a/win/src/ipc_ping_test.cc b/win/src/ipc_ping_test.cc
index 44f6be4..39aa632 100644
--- a/win/src/ipc_ping_test.cc
+++ b/win/src/ipc_ping_test.cc
@@ -48,11 +48,14 @@
}
TEST(IPCTest, IPCPingTestWithOutput) {
- TestRunner runner;
- runner.SetTimeout(2000);
- runner.SetTestState(EVERY_STATE);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"IPC_Ping 2"));
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"IPC_Ping 2"));
+ TestRunner runner1;
+ runner1.SetTimeout(2000);
+ runner1.SetTestState(EVERY_STATE);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner1.RunTest(L"IPC_Ping 2"));
+ TestRunner runner2;
+ runner2.SetTimeout(2000);
+ runner2.SetTestState(EVERY_STATE);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(L"IPC_Ping 2"));
}
} // namespace sandbox
diff --git a/win/src/named_pipe_policy_test.cc b/win/src/named_pipe_policy_test.cc
index 3d8db97..817aa43 100644
--- a/win/src/named_pipe_policy_test.cc
+++ b/win/src/named_pipe_policy_test.cc
@@ -52,39 +52,52 @@
return SBOX_TEST_SUCCEEDED;
}
-// Tests if we can create a pipe in the sandbox.
-TEST(NamedPipePolicyTest, CreatePipe) {
- TestRunner runner;
+std::unique_ptr<TestRunner> CreatePipeRunner() {
+ auto runner = std::make_unique<TestRunner>();
// TODO(nsylvain): This policy is wrong because "*" is a valid char in a
// namedpipe name. Here we apply it like a wildcard. http://b/893603
- EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_NAMED_PIPES,
- TargetPolicy::NAMEDPIPES_ALLOW_ANY,
- L"\\\\.\\pipe\\test*"));
+ runner->AddRule(TargetPolicy::SUBSYS_NAMED_PIPES,
+ TargetPolicy::NAMEDPIPES_ALLOW_ANY, L"\\\\.\\pipe\\test*");
+ return runner;
+}
+// Tests if we can create a pipe in the sandbox.
+TEST(NamedPipePolicyTest, CreatePipe) {
+ auto runner = CreatePipeRunner();
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"NamedPipe_Create \\\\.\\pipe\\testbleh"));
+ runner->RunTest(L"NamedPipe_Create \\\\.\\pipe\\testbleh"));
+ runner = CreatePipeRunner();
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"NamedPipe_Create \\\\.\\pipe\\bleh"));
+ runner->RunTest(L"NamedPipe_Create \\\\.\\pipe\\bleh"));
+}
+
+std::unique_ptr<TestRunner> PipeTraversalRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ // TODO(nsylvain): This policy is wrong because "*" is a valid char in a
+ // namedpipe name. Here we apply it like a wildcard. http://b/893603
+ runner->AddRule(TargetPolicy::SUBSYS_NAMED_PIPES,
+ TargetPolicy::NAMEDPIPES_ALLOW_ANY, L"\\\\.\\pipe\\test*");
+ return runner;
}
// Tests if we can create a pipe with a path traversal in the sandbox.
TEST(NamedPipePolicyTest, CreatePipeTraversal) {
- TestRunner runner;
- // TODO(nsylvain): This policy is wrong because "*" is a valid char in a
- // namedpipe name. Here we apply it like a wildcard. http://b/893603
- EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_NAMED_PIPES,
- TargetPolicy::NAMEDPIPES_ALLOW_ANY,
- L"\\\\.\\pipe\\test*"));
+ auto runner = PipeTraversalRunner();
+ EXPECT_EQ(SBOX_TEST_DENIED,
+ runner->RunTest(L"NamedPipe_Create \\\\.\\pipe\\test\\..\\bleh"));
+ runner = PipeTraversalRunner();
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"NamedPipe_Create \\\\.\\pipe\\test\\..\\bleh"));
+ runner->RunTest(L"NamedPipe_Create \\\\.\\pipe\\test/../bleh"));
+
+ runner = PipeTraversalRunner();
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"NamedPipe_Create \\\\.\\pipe\\test/../bleh"));
+ runner->RunTest(L"NamedPipe_Create \\\\.\\pipe\\test\\../bleh"));
+
+ runner = PipeTraversalRunner();
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"NamedPipe_Create \\\\.\\pipe\\test\\../bleh"));
- EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"NamedPipe_Create \\\\.\\pipe\\test/..\\bleh"));
+ runner->RunTest(L"NamedPipe_Create \\\\.\\pipe\\test/..\\bleh"));
}
// This tests that path canonicalization is actually disabled if we use \\?\
@@ -100,22 +113,25 @@
NamedPipe_Create(2, const_cast<wchar_t**>(argv)));
}
-// The same test as CreatePipe but this time using strict interceptions.
-TEST(NamedPipePolicyTest, CreatePipeStrictInterceptions) {
- TestRunner runner;
- runner.GetPolicy()->SetStrictInterceptions();
-
+std::unique_ptr<TestRunner> StrictInterceptionsRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->GetPolicy()->SetStrictInterceptions();
// TODO(nsylvain): This policy is wrong because "*" is a valid char in a
// namedpipe name. Here we apply it like a wildcard. http://b/893603
- EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_NAMED_PIPES,
- TargetPolicy::NAMEDPIPES_ALLOW_ANY,
- L"\\\\.\\pipe\\test*"));
+ runner->AddRule(TargetPolicy::SUBSYS_NAMED_PIPES,
+ TargetPolicy::NAMEDPIPES_ALLOW_ANY, L"\\\\.\\pipe\\test*");
+ return runner;
+}
+// The same test as CreatePipe but this time using strict interceptions.
+TEST(NamedPipePolicyTest, CreatePipeStrictInterceptions) {
+ auto runner = StrictInterceptionsRunner();
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"NamedPipe_Create \\\\.\\pipe\\testbleh"));
+ runner->RunTest(L"NamedPipe_Create \\\\.\\pipe\\testbleh"));
+ runner = StrictInterceptionsRunner();
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"NamedPipe_Create \\\\.\\pipe\\bleh"));
+ runner->RunTest(L"NamedPipe_Create \\\\.\\pipe\\bleh"));
}
} // namespace sandbox
diff --git a/win/src/policy_target_test.cc b/win/src/policy_target_test.cc
index b8b6859..1712142 100644
--- a/win/src/policy_target_test.cc
+++ b/win/src/policy_target_test.cc
@@ -180,11 +180,13 @@
runner.SetTestState(BEFORE_REVERT);
EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"PolicyTargetTest_token"));
- runner.SetTestState(AFTER_REVERT);
- EXPECT_EQ(ERROR_NO_TOKEN, runner.RunTest(L"PolicyTargetTest_token"));
+ TestRunner runner1;
+ runner1.SetTestState(AFTER_REVERT);
+ EXPECT_EQ(ERROR_NO_TOKEN, runner1.RunTest(L"PolicyTargetTest_token"));
- runner.SetTestState(EVERY_STATE);
- EXPECT_EQ(SBOX_TEST_FAILED, runner.RunTest(L"PolicyTargetTest_steal"));
+ TestRunner runner2;
+ runner2.SetTestState(EVERY_STATE);
+ EXPECT_EQ(SBOX_TEST_FAILED, runner2.RunTest(L"PolicyTargetTest_steal"));
}
TEST(PolicyTargetTest, OpenThreadToken) {
@@ -192,18 +194,19 @@
runner.SetTestState(BEFORE_REVERT);
EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"PolicyTargetTest_token2"));
- runner.SetTestState(AFTER_REVERT);
- EXPECT_EQ(ERROR_NO_TOKEN, runner.RunTest(L"PolicyTargetTest_token2"));
+ TestRunner runner2;
+ runner2.SetTestState(AFTER_REVERT);
+ EXPECT_EQ(ERROR_NO_TOKEN, runner2.RunTest(L"PolicyTargetTest_token2"));
}
TEST(PolicyTargetTest, OpenThreadTokenEx) {
TestRunner runner;
-
runner.SetTestState(BEFORE_REVERT);
EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"PolicyTargetTest_token3"));
- runner.SetTestState(AFTER_REVERT);
- EXPECT_EQ(ERROR_NO_TOKEN, runner.RunTest(L"PolicyTargetTest_token3"));
+ TestRunner runner2;
+ runner2.SetTestState(AFTER_REVERT);
+ EXPECT_EQ(ERROR_NO_TOKEN, runner2.RunTest(L"PolicyTargetTest_token3"));
}
TEST(PolicyTargetTest, OpenThread) {
@@ -211,7 +214,8 @@
EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"PolicyTargetTest_thread"))
<< "Opens the current thread";
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"PolicyTargetTest_thread2"))
+ TestRunner runner2;
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(L"PolicyTargetTest_thread2"))
<< "Creates a new thread and opens it";
}
diff --git a/win/src/process_mitigations_dyncode_unittest.cc b/win/src/process_mitigations_dyncode_unittest.cc
index 21d7021..b4dc8f2 100644
--- a/win/src/process_mitigations_dyncode_unittest.cc
+++ b/win/src/process_mitigations_dyncode_unittest.cc
@@ -244,6 +244,15 @@
int return_code_;
};
+// Helpers to set up rules for dynamic code tests, needed as policy
+// (from the TestRunner) can only be applied to a single process.
+std::unique_ptr<sandbox::TestRunner> RunnerWithMitigation(
+ sandbox::MitigationFlags mitigations) {
+ auto runner = std::make_unique<sandbox::TestRunner>();
+ runner->GetPolicy()->SetDelayedProcessMitigations(mitigations);
+ return runner;
+}
+
//------------------------------------------------------------------------------
// DisableDynamicCode test harness helper function. Tests numerous APIs.
// - APIs fail with ERROR_DYNAMIC_CODE_BLOCKED if this mitigation is
@@ -265,14 +274,6 @@
return;
}
- sandbox::TestRunner runner;
- sandbox::TargetPolicy* policy = runner.GetPolicy();
-
- if (enable_mitigation) {
- EXPECT_EQ(policy->SetDelayedProcessMitigations(which_mitigation),
- sandbox::SBOX_ALL_OK);
- }
-
std::wstring shared =
(which_mitigation == sandbox::MITIGATION_DYNAMIC_CODE_DISABLE)
? L"TestWin81DynamicCode "
@@ -283,32 +284,34 @@
}
// Test 1:
+ auto runner = enable_mitigation ? RunnerWithMitigation(which_mitigation)
+ : std::make_unique<sandbox::TestRunner>();
std::wstring test =
base::StringPrintf(L"%ls %u", shared.c_str(), VIRTUALALLOC);
EXPECT_EQ((expect_success ? sandbox::SBOX_TEST_SUCCEEDED
: ERROR_DYNAMIC_CODE_BLOCKED),
- runner.RunTest(test.c_str()));
+ runner->RunTest(test.c_str()));
// Test 2:
+ runner = enable_mitigation ? RunnerWithMitigation(which_mitigation)
+ : std::make_unique<sandbox::TestRunner>();
test = base::StringPrintf(L"%ls %u", shared.c_str(), VIRTUALPROTECT);
EXPECT_EQ((expect_success ? sandbox::SBOX_TEST_SUCCEEDED
: ERROR_DYNAMIC_CODE_BLOCKED),
- runner.RunTest(test.c_str()));
+ runner->RunTest(test.c_str()));
// Test 3:
// Need token level >= USER_LIMITED to be able to successfully run test 3.
- policy->SetTokenLevel(sandbox::TokenLevel::USER_RESTRICTED_SAME_ACCESS,
- sandbox::TokenLevel::USER_LIMITED);
+ runner = enable_mitigation ? RunnerWithMitigation(which_mitigation)
+ : std::make_unique<sandbox::TestRunner>();
+ runner->GetPolicy()->SetTokenLevel(
+ sandbox::TokenLevel::USER_RESTRICTED_SAME_ACCESS,
+ sandbox::TokenLevel::USER_LIMITED);
test = base::StringPrintf(L"%ls %u", shared.c_str(), MAPVIEWCUSTOM);
EXPECT_EQ((expect_success ? sandbox::SBOX_TEST_SUCCEEDED
: ERROR_DYNAMIC_CODE_BLOCKED),
- runner.RunTest(test.c_str()));
-
- // Test 4:
- // Set token levels back to default.
- policy->SetTokenLevel(sandbox::TokenLevel::USER_RESTRICTED_SAME_ACCESS,
- sandbox::TokenLevel::USER_LOCKDOWN);
+ runner->RunTest(test.c_str()));
// Ensure sandbox access to the file on disk.
base::FilePath dll_path;
@@ -322,14 +325,16 @@
temp_dir.GetPath().Append(hooking_dll::g_hook_dll_file);
ASSERT_TRUE(base::CopyFile(dll_path, temp_dll_path));
- EXPECT_TRUE(runner.AddFsRule(sandbox::TargetPolicy::FILES_ALLOW_ANY,
- temp_dll_path.value().c_str()));
+ runner = enable_mitigation ? RunnerWithMitigation(which_mitigation)
+ : std::make_unique<sandbox::TestRunner>();
+ EXPECT_TRUE(runner->AddFsRule(sandbox::TargetPolicy::FILES_ALLOW_ANY,
+ temp_dll_path.value().c_str()));
test = base::StringPrintf(L"%ls %u \"%ls\"", shared.c_str(), MAPVIEWFILE,
temp_dll_path.value().c_str());
EXPECT_EQ((expect_success ? sandbox::SBOX_TEST_SUCCEEDED
: ERROR_DYNAMIC_CODE_BLOCKED),
- runner.RunTest(test.c_str()));
+ runner->RunTest(test.c_str()));
}
} // namespace
diff --git a/win/src/unload_dll_test.cc b/win/src/unload_dll_test.cc
index fc85c0c..2067f77 100644
--- a/win/src/unload_dll_test.cc
+++ b/win/src/unload_dll_test.cc
@@ -42,6 +42,17 @@
return SBOX_TEST_SUCCEEDED;
}
+std::unique_ptr<TestRunner> BaselineAvicapRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->SetTestState(BEFORE_REVERT);
+ runner->SetTimeout(2000);
+ // Add a registry rule, because that ensures that the interception agent has
+ // more than one item in its internal table.
+ runner->AddRule(TargetPolicy::SUBSYS_FILES, TargetPolicy::FILES_ALLOW_QUERY,
+ L"\\??\\*.exe");
+ return runner;
+}
+
// Fails on Windows ARM64: https://crbug.com/905526
#if defined(ARCH_CPU_ARM64)
#define MAYBE_BaselineAvicapDll DISABLED_BaselineAvicapDll
@@ -49,49 +60,51 @@
#define MAYBE_BaselineAvicapDll BaselineAvicapDll
#endif
TEST(UnloadDllTest, MAYBE_BaselineAvicapDll) {
- TestRunner runner;
- runner.SetTestState(BEFORE_REVERT);
- runner.SetTimeout(2000);
- // Add a registry rule, because that ensures that the interception agent has
- // more than one item in its internal table.
- EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_FILES,
- TargetPolicy::FILES_ALLOW_QUERY, L"\\??\\*.exe"));
-
+ auto runner = BaselineAvicapRunner();
// Note for the puzzled: avicap32.dll is a 64-bit dll in 64-bit versions of
// windows so this test and the others just work.
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"UseOneDLL L avicap32.dll"));
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"UseOneDLL B avicap32.dll"));
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"UseOneDLL L avicap32.dll"));
+ runner = BaselineAvicapRunner();
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"UseOneDLL B avicap32.dll"));
+}
+
+std::unique_ptr<TestRunner> UnloadAvicapNoPatchingRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->SetTestState(BEFORE_REVERT);
+ runner->SetTimeout(2000);
+ runner->GetPolicy()->AddDllToUnload(L"avicap32.dll");
+ return runner;
}
TEST(UnloadDllTest, UnloadAviCapDllNoPatching) {
- TestRunner runner;
- runner.SetTestState(BEFORE_REVERT);
- runner.SetTimeout(2000);
- sandbox::TargetPolicy* policy = runner.GetPolicy();
- policy->AddDllToUnload(L"avicap32.dll");
- EXPECT_EQ(SBOX_TEST_FAILED, runner.RunTest(L"UseOneDLL L avicap32.dll"));
- EXPECT_EQ(SBOX_TEST_FAILED, runner.RunTest(L"UseOneDLL B avicap32.dll"));
+ auto runner = UnloadAvicapNoPatchingRunner();
+ EXPECT_EQ(SBOX_TEST_FAILED, runner->RunTest(L"UseOneDLL L avicap32.dll"));
+ runner = UnloadAvicapNoPatchingRunner();
+ EXPECT_EQ(SBOX_TEST_FAILED, runner->RunTest(L"UseOneDLL B avicap32.dll"));
}
-TEST(UnloadDllTest, UnloadAviCapDllWithPatching) {
- TestRunner runner;
- runner.SetTimeout(2000);
- runner.SetTestState(BEFORE_REVERT);
- sandbox::TargetPolicy* policy = runner.GetPolicy();
- policy->AddDllToUnload(L"avicap32.dll");
-
+std::unique_ptr<TestRunner> UnloadAvicapWithPatchingRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->SetTestState(BEFORE_REVERT);
+ runner->SetTimeout(2000);
+ runner->GetPolicy()->AddDllToUnload(L"avicap32.dll");
// Add a couple of rules that ensures that the interception agent add EAT
// patching on the client which makes sure that the unload dll record does
// not interact badly with them.
- EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_FILES,
- TargetPolicy::FILES_ALLOW_QUERY, L"\\??\\*.exe"));
- EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_FILES,
- TargetPolicy::FILES_ALLOW_QUERY, L"\\??\\*.log"));
+ runner->AddRule(TargetPolicy::SUBSYS_FILES, TargetPolicy::FILES_ALLOW_QUERY,
+ L"\\??\\*.exe");
+ runner->AddRule(TargetPolicy::SUBSYS_FILES, TargetPolicy::FILES_ALLOW_QUERY,
+ L"\\??\\*.log");
+ return runner;
+}
- EXPECT_EQ(SBOX_TEST_FAILED, runner.RunTest(L"UseOneDLL L avicap32.dll"));
+TEST(UnloadDllTest, UnloadAviCapDllWithPatching) {
+ auto runner = UnloadAvicapWithPatchingRunner();
+ EXPECT_EQ(SBOX_TEST_FAILED, runner->RunTest(L"UseOneDLL L avicap32.dll"));
- runner.SetTestState(AFTER_REVERT);
- EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"OpenExecutablePath"));
+ runner = UnloadAvicapWithPatchingRunner();
+ runner->SetTestState(AFTER_REVERT);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner->RunTest(L"OpenExecutablePath"));
}
} // namespace sandbox
diff --git a/win/tests/common/controller.cc b/win/tests/common/controller.cc
index 27bf646..ffab034 100644
--- a/win/tests/common/controller.cc
+++ b/win/tests/common/controller.cc
@@ -6,6 +6,7 @@
#include <string>
+#include "base/dcheck_is_on.h"
#include "base/memory/platform_shared_memory_region.h"
#include "base/memory/read_only_shared_memory_region.h"
#include "base/process/process.h"
@@ -226,6 +227,11 @@
return SBOX_ERROR_GENERIC;
}
} else {
+#if DCHECK_IS_ON()
+ // Policy can be applied to one target only.
+ DCHECK(!policy_applied_);
+ policy_applied_ = true;
+#endif
result = broker_->SpawnTarget(prog_name, arguments.c_str(), policy_,
&warning_result, &last_error, &target);
}
diff --git a/win/tests/common/controller.h b/win/tests/common/controller.h
index eaa5bb3..2997244 100644
--- a/win/tests/common/controller.h
+++ b/win/tests/common/controller.h
@@ -8,6 +8,7 @@
#include <windows.h>
#include <string>
+#include "base/dcheck_is_on.h"
#include "base/memory/raw_ptr.h"
#include "base/time/time.h"
#include "base/win/scoped_handle.h"
@@ -169,6 +170,10 @@
bool disable_csrss_;
bool kill_on_destruction_;
bool release_policy_in_run_ = false;
+#if DCHECK_IS_ON()
+ // We only allow the policy to be applied to a target once.
+ bool policy_applied_ = false;
+#endif
base::win::ScopedHandle target_process_;
DWORD target_process_id_;
};
diff --git a/win/tests/integration_tests/integration_tests_test.cc b/win/tests/integration_tests/integration_tests_test.cc
index 662b61a..a040d1e 100644
--- a/win/tests/integration_tests/integration_tests_test.cc
+++ b/win/tests/integration_tests/integration_tests_test.cc
@@ -223,8 +223,12 @@
runner.SetTimeout(TestTimeouts::action_timeout());
runner.SetTestState(BEFORE_INIT);
ASSERT_EQ(1, runner.RunTest(L"IntegrationTestsTest_args first"));
- ASSERT_EQ(4, runner.RunTest(L"IntegrationTestsTest_args first second third "
- L"fourth"));
+
+ TestRunner runner2;
+ runner2.SetTimeout(TestTimeouts::action_timeout());
+ runner2.SetTestState(BEFORE_INIT);
+ ASSERT_EQ(4, runner2.RunTest(L"IntegrationTestsTest_args first second third "
+ L"fourth"));
}
TEST(IntegrationTestsTest, WaitForStuckChild) {
@@ -306,37 +310,40 @@
::TerminateProcess(runner2.process(), 0);
}
-TEST(IntegrationTestsTest, MultipleStuckChildrenSequential) {
- TestRunner runner;
- runner.SetTimeout(TestTimeouts::action_timeout());
- runner.SetAsynchronous(true);
- runner.SetKillOnDestruction(false);
- TestRunner runner2(JOB_NONE, USER_RESTRICTED_SAME_ACCESS, USER_LOCKDOWN);
- runner2.SetTimeout(TestTimeouts::action_timeout());
- runner2.SetAsynchronous(true);
- runner2.SetKillOnDestruction(false);
+std::unique_ptr<TestRunner> StuckChildrenRunner() {
+ auto runner = std::make_unique<TestRunner>(
+ JOB_NONE, USER_RESTRICTED_SAME_ACCESS, USER_LOCKDOWN);
+ runner->SetTimeout(TestTimeouts::action_timeout());
+ runner->SetAsynchronous(true);
+ runner->SetKillOnDestruction(false);
+ return runner;
+}
+TEST(IntegrationTestsTest, MultipleStuckChildrenSequential) {
+ auto runner = StuckChildrenRunner();
ASSERT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"IntegrationTestsTest_stuck 100"));
- // Actually both runners share the same singleton broker.
- ASSERT_EQ(SBOX_ALL_OK, runner.broker()->WaitForAllTargets());
+ runner->RunTest(L"IntegrationTestsTest_stuck 100"));
+ // All runners share the same singleton broker.
+ auto* broker = runner->broker();
+ ASSERT_EQ(SBOX_ALL_OK, broker->WaitForAllTargets());
+
+ runner = StuckChildrenRunner();
ASSERT_EQ(SBOX_TEST_SUCCEEDED,
- runner2.RunTest(L"IntegrationTestsTest_stuck 2000"));
- // Actually both runners share the same singleton broker.
- ASSERT_EQ(SBOX_ALL_OK, runner.broker()->WaitForAllTargets());
+ runner->RunTest(L"IntegrationTestsTest_stuck 2000"));
+ ASSERT_EQ(SBOX_ALL_OK, broker->WaitForAllTargets());
DWORD exit_code;
// Checking the exit code for |runner| is flaky on the slow bots but at
// least we know that the wait above has succeeded if we are here.
- ASSERT_TRUE(::GetExitCodeProcess(runner2.process(), &exit_code));
+ ASSERT_TRUE(::GetExitCodeProcess(runner->process(), &exit_code));
ASSERT_EQ(STILL_ACTIVE, exit_code);
// Terminate the test process now.
- ::TerminateProcess(runner2.process(), 0);
+ ::TerminateProcess(runner->process(), 0);
+ runner = StuckChildrenRunner();
ASSERT_EQ(SBOX_TEST_SUCCEEDED,
- runner.RunTest(L"IntegrationTestsTest_stuck 100"));
- // Actually both runners share the same singleton broker.
- ASSERT_EQ(SBOX_ALL_OK, runner.broker()->WaitForAllTargets());
+ runner->RunTest(L"IntegrationTestsTest_stuck 100"));
+ ASSERT_EQ(SBOX_ALL_OK, broker->WaitForAllTargets());
}
// Running from inside job that allows us to escape from it should be ok.
diff --git a/win/tests/validation_tests/suite.cc b/win/tests/validation_tests/suite.cc
index dbfd802..684ac70 100644
--- a/win/tests/validation_tests/suite.cc
+++ b/win/tests/validation_tests/suite.cc
@@ -13,32 +13,55 @@
#include "sandbox/win/tests/common/controller.h"
#include "testing/gtest/include/gtest/gtest.h"
+// Callback that generates fresh TestRunners for process access tests.
+typedef std::unique_ptr<sandbox::TestRunner> (*RunnerGenerator)();
+
namespace {
-void TestProcessAccess(sandbox::TestRunner* runner, DWORD target) {
+void TestProcessAccess(RunnerGenerator runner_gen, DWORD target) {
const wchar_t *kCommandTemplate = L"OpenProcessCmd %d %d";
wchar_t command[1024] = {0};
+ std::unique_ptr<sandbox::TestRunner> runner = nullptr;
// Test all the scary process permissions.
wsprintf(command, kCommandTemplate, target, PROCESS_CREATE_THREAD);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, PROCESS_DUP_HANDLE);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, PROCESS_SET_INFORMATION);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, PROCESS_VM_OPERATION);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, PROCESS_VM_READ);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, PROCESS_VM_WRITE);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, PROCESS_QUERY_INFORMATION);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, WRITE_DAC);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, WRITE_OWNER);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
+
wsprintf(command, kCommandTemplate, target, READ_CONTROL);
+ runner = runner_gen();
EXPECT_EQ(sandbox::SBOX_TEST_DENIED, runner->RunTest(command));
}
@@ -78,50 +101,86 @@
ASSERT_TRUE(VolumeSupportsACLs(L"%Temp%\\"));
ASSERT_TRUE(VolumeSupportsACLs(L"%AppData%\\"));
- TestRunner runner;
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenFileCmd %SystemDrive%"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenFileCmd %SystemRoot%"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenFileCmd %ProgramFiles%"));
+ TestRunner runner_sysdrive;
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"OpenFileCmd %SystemRoot%\\System32"));
+ runner_sysdrive.RunTest(L"OpenFileCmd %SystemDrive%"));
+
+ TestRunner runner_sysroot;
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"OpenFileCmd %SystemRoot%\\explorer.exe"));
+ runner_sysroot.RunTest(L"OpenFileCmd %SystemRoot%"));
+
+ TestRunner runner_programfiles;
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"OpenFileCmd %SystemRoot%\\Cursors\\arrow_i.cur"));
+ runner_programfiles.RunTest(L"OpenFileCmd %ProgramFiles%"));
+
+ TestRunner runner_system32;
EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(L"OpenFileCmd %AllUsersProfile%"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenFileCmd %Temp%"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenFileCmd %AppData%"));
+ runner_system32.RunTest(L"OpenFileCmd %SystemRoot%\\System32"));
+
+ TestRunner runner_explorer;
+ EXPECT_EQ(SBOX_TEST_DENIED,
+ runner_explorer.RunTest(L"OpenFileCmd %SystemRoot%\\explorer.exe"));
+
+ TestRunner runner_cursors;
+ EXPECT_EQ(SBOX_TEST_DENIED,
+ runner_cursors.RunTest(
+ L"OpenFileCmd %SystemRoot%\\Cursors\\arrow_i.cur"));
+
+ TestRunner runner_profiles;
+ EXPECT_EQ(SBOX_TEST_DENIED,
+ runner_profiles.RunTest(L"OpenFileCmd %AllUsersProfile%"));
+
+ TestRunner runner_temp;
+ EXPECT_EQ(SBOX_TEST_DENIED, runner_temp.RunTest(L"OpenFileCmd %Temp%"));
+
+ TestRunner runner_appdata;
+ EXPECT_EQ(SBOX_TEST_DENIED, runner_appdata.RunTest(L"OpenFileCmd %AppData%"));
}
// Tests if the registry is correctly protected by the sandbox.
TEST(ValidationSuite, TestRegistry) {
- TestRunner runner;
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenKey HKLM"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenKey HKCU"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenKey HKU"));
- EXPECT_EQ(SBOX_TEST_DENIED,
- runner.RunTest(
+ TestRunner runner_hklm;
+ EXPECT_EQ(SBOX_TEST_DENIED, runner_hklm.RunTest(L"OpenKey HKLM"));
+
+ TestRunner runner_hkcu;
+ EXPECT_EQ(SBOX_TEST_DENIED, runner_hkcu.RunTest(L"OpenKey HKCU"));
+
+ TestRunner runner_hku;
+ EXPECT_EQ(SBOX_TEST_DENIED, runner_hku.RunTest(L"OpenKey HKU"));
+
+ TestRunner runner_hklm_key;
+ EXPECT_EQ(
+ SBOX_TEST_DENIED,
+ runner_hklm_key.RunTest(
L"OpenKey HKLM "
L"\"Software\\Microsoft\\Windows NT\\CurrentVersion\\WinLogon\""));
}
+std::unique_ptr<TestRunner> DesktopRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->GetPolicy()->SetAlternateDesktop(true);
+ runner->GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
+ return runner;
+}
+
// Tests that the permissions on the Windowstation does not allow the sandbox
// to get to the interactive desktop or to make the sbox desktop interactive.
TEST(ValidationSuite, TestDesktop) {
- TestRunner runner;
- runner.GetPolicy()->SetAlternateDesktop(true);
- runner.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenInteractiveDesktop NULL"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"SwitchToSboxDesktop NULL"));
+ auto runner = DesktopRunner();
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"OpenInteractiveDesktop NULL"));
+
+ runner = DesktopRunner();
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"SwitchToSboxDesktop NULL"));
}
// Tests that the permissions on the Windowstation does not allow the sandbox
// to get to the interactive desktop or to make the sbox desktop interactive.
TEST(ValidationSuite, TestAlternateDesktop) {
- TestRunner runner;
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"EnumAlternateWinsta NULL"));
+ TestRunner runner_no_policy;
+ EXPECT_EQ(SBOX_TEST_DENIED,
+ runner_no_policy.RunTest(L"EnumAlternateWinsta NULL"));
+ TestRunner runner;
wchar_t command[1024] = {0};
runner.SetTimeout(3600000);
runner.GetPolicy()->SetAlternateDesktop(true);
@@ -132,14 +191,21 @@
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command));
}
+std::unique_ptr<TestRunner> AlternateDesktopLocalWinstationRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->GetPolicy()->SetAlternateDesktop(false);
+ runner->GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
+ return runner;
+}
+
// Same as TestDesktop, but uses the local winstation, instead of an alternate
// one.
TEST(ValidationSuite, TestAlternateDesktopLocalWinstation) {
- TestRunner runner;
- runner.GetPolicy()->SetAlternateDesktop(false);
- runner.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenInteractiveDesktop NULL"));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"SwitchToSboxDesktop NULL"));
+ auto runner = AlternateDesktopLocalWinstationRunner();
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"OpenInteractiveDesktop NULL"));
+
+ runner = AlternateDesktopLocalWinstationRunner();
+ EXPECT_EQ(SBOX_TEST_DENIED, runner->RunTest(L"SwitchToSboxDesktop NULL"));
}
// Tests if the windows are correctly protected by the sandbox.
@@ -152,65 +218,72 @@
return;
}
- TestRunner runner;
wchar_t command[1024] = {0};
+ TestRunner runner_getshellwindow;
wsprintf(command, L"ValidWindow %Id",
reinterpret_cast<size_t>(::GetShellWindow()));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command));
+ EXPECT_EQ(SBOX_TEST_DENIED, runner_getshellwindow.RunTest(command));
+ TestRunner runner_findwindow;
wsprintf(command, L"ValidWindow %Id",
reinterpret_cast<size_t>(::FindWindow(NULL, NULL)));
- EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command));
+ EXPECT_EQ(SBOX_TEST_DENIED, runner_findwindow.RunTest(command));
+}
+
+std::unique_ptr<TestRunner> ProcessDenyLockdownRunner() {
+ return std::make_unique<TestRunner>();
}
// Tests that a locked-down process cannot open another locked-down process.
TEST(ValidationSuite, TestProcessDenyLockdown) {
- TestRunner runner;
TestRunner target;
-
target.SetAsynchronous(true);
EXPECT_EQ(SBOX_TEST_SUCCEEDED, target.RunTest(L"SleepCmd 30000"));
- TestProcessAccess(&runner, target.process_id());
+ TestProcessAccess(ProcessDenyLockdownRunner, target.process_id());
+}
+
+std::unique_ptr<TestRunner> ProcessDenyLowIntegrityRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
+ runner->GetPolicy()->SetTokenLevel(USER_RESTRICTED_SAME_ACCESS,
+ USER_INTERACTIVE);
+ return runner;
}
// Tests that a low-integrity process cannot open a locked-down process (due
// to the integrity label changing after startup via SetDelayedIntegrityLevel).
TEST(ValidationSuite, TestProcessDenyLowIntegrity) {
- TestRunner runner;
TestRunner target;
-
target.SetAsynchronous(true);
target.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_LOW);
- runner.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
- runner.GetPolicy()->SetTokenLevel(USER_RESTRICTED_SAME_ACCESS,
- USER_INTERACTIVE);
-
EXPECT_EQ(SBOX_TEST_SUCCEEDED, target.RunTest(L"SleepCmd 30000"));
- TestProcessAccess(&runner, target.process_id());
+ TestProcessAccess(ProcessDenyLowIntegrityRunner, target.process_id());
+}
+
+std::unique_ptr<TestRunner> ProcessDenyBelowLowIntegrityRunner() {
+ auto runner = std::make_unique<TestRunner>();
+ runner->GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_UNTRUSTED);
+ runner->GetPolicy()->SetTokenLevel(USER_RESTRICTED_SAME_ACCESS,
+ USER_INTERACTIVE);
+ return runner;
}
// Tests that a locked-down process cannot open a low-integrity process.
TEST(ValidationSuite, TestProcessDenyBelowLowIntegrity) {
- TestRunner runner;
TestRunner target;
-
target.SetAsynchronous(true);
target.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
target.GetPolicy()->SetTokenLevel(USER_RESTRICTED_SAME_ACCESS,
USER_INTERACTIVE);
- runner.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_UNTRUSTED);
- runner.GetPolicy()->SetTokenLevel(USER_RESTRICTED_SAME_ACCESS,
- USER_INTERACTIVE);
-
EXPECT_EQ(SBOX_TEST_SUCCEEDED, target.RunTest(L"SleepCmd 30000"));
- TestProcessAccess(&runner, target.process_id());
+ TestProcessAccess(ProcessDenyBelowLowIntegrityRunner, target.process_id());
}
// Tests if the threads are correctly protected by the sandbox.
