Skip to content

GitLab

T
tangled
Commit 23f88f7f authored by wfh's avatar wfh Committed by Commit bot
Browse files
Options

Fix sandbox process policy tests. 

These tests made invalid assumptions about the WOW64 virtualization
state and were partially disabled. Now they all pass on all platforms.

BUG=580800,6944

Review URL: https://codereview.chromium.org/1624083003

Cr-Commit-Position: refs/heads/master@{#371354}
parent 0eefcd63
Hide whitespace changes
Inline Side-by-side
Showing with 48 additions and 59 deletions
sandbox/win/src/process_policy_test.cc
View file @ 23f88f7f
......@@ -5,6 +5,7 @@
#include <memory>
#include <string>
#include "base/memory/scoped_ptr.h"
#include "base/strings/string16.h"
#include "base/strings/sys_string_conversions.h"
#include "base/win/scoped_handle.h"
......@@ -18,39 +19,24 @@
namespace {
// While the shell API provides better calls than this home brew function
// we use GetSystemWindowsDirectoryW which does not query the registry so
// it is safe to use after revert.
base::string16 MakeFullPathToSystem32(const wchar_t* name) {
wchar_t windows_path[MAX_PATH] = {0};
::GetSystemWindowsDirectoryW(windows_path, MAX_PATH);
base::string16 full_path(windows_path);
if (full_path.empty()) {
return full_path;
}
full_path += L"\\system32\\";
full_path += name;
return full_path;
}
// Creates a process with the |exe| and |command| parameter using the
// unicode and ascii version of the api.
sandbox::SboxTestResult CreateProcessHelper(const base::string16& exe,
const base::string16& command) {
base::win::ScopedProcessInformation pi;
STARTUPINFOW si = {sizeof(si)};
const wchar_t *exe_name = NULL;
const wchar_t* exe_name = NULL;
if (!exe.empty())
exe_name = exe.c_str();
base::string16 writable_command = command;
scoped_ptr<wchar_t, base::FreeDeleter> writable_command(
_wcsdup(command.c_str()));
// Create the process with the unicode version of the API.
sandbox::SboxTestResult ret1 = sandbox::SBOX_TEST_FAILED;
PROCESS_INFORMATION temp_process_info = {};
if (::CreateProcessW(exe_name,
command.empty() ? NULL : &writable_command[0],
command.empty() ? NULL : writable_command.get(),
NULL,
NULL,
FALSE,
......@@ -114,7 +100,7 @@ SBOX_TESTS_COMMAND int Process_RunApp1(int argc, wchar_t **argv) {
if ((NULL == argv) || (NULL == argv[0])) {
return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
}
base::string16 path = MakeFullPathToSystem32(argv[0]);
base::string16 path = MakePathToSys(argv[0], false);
// TEST 1: Try with the path in the app_name.
return CreateProcessHelper(path, base::string16());
......@@ -127,7 +113,7 @@ SBOX_TESTS_COMMAND int Process_RunApp2(int argc, wchar_t **argv) {
if ((NULL == argv) || (NULL == argv[0])) {
return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
}
base::string16 path = MakeFullPathToSystem32(argv[0]);
base::string16 path = MakePathToSys(argv[0], false);
// TEST 2: Try with the path in the cmd_line.
base::string16 cmd_line = L"\"";
......@@ -143,7 +129,6 @@ SBOX_TESTS_COMMAND int Process_RunApp3(int argc, wchar_t **argv) {
if ((NULL == argv) || (NULL == argv[0])) {
return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
}
// TEST 3: Try file name in the cmd_line.
return CreateProcessHelper(base::string16(), argv[0]);
}
......@@ -157,7 +142,7 @@ SBOX_TESTS_COMMAND int Process_RunApp4(int argc, wchar_t **argv) {
}
// TEST 4: Try file name in the app_name and current directory sets correctly.
base::string16 system32 = MakeFullPathToSystem32(L"");
base::string16 system32 = MakePathToSys(L"", false);
wchar_t current_directory[MAX_PATH + 1];
DWORD ret = ::GetCurrentDirectory(MAX_PATH, current_directory);
if (!ret)
......@@ -182,7 +167,7 @@ SBOX_TESTS_COMMAND int Process_RunApp5(int argc, wchar_t **argv) {
if ((NULL == argv) || (NULL == argv[0])) {
return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
}
base::string16 path = MakeFullPathToSystem32(argv[0]);
base::string16 path = MakePathToSys(argv[0], false);
// TEST 5: Try with the path in the cmd_line and arguments.
base::string16 cmd_line = L"\"";
......@@ -213,7 +198,7 @@ SBOX_TESTS_COMMAND int Process_GetChildProcessToken(int argc, wchar_t **argv) {
if ((NULL == argv) || (NULL == argv[0]))
return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
base::string16 path = MakeFullPathToSystem32(argv[0]);
base::string16 path = MakePathToSys(argv[0], false);
STARTUPINFOW si = {sizeof(si)};
......@@ -307,17 +292,23 @@ TEST(ProcessPolicyTest, TestAllAccess) {
TEST(ProcessPolicyTest, CreateProcessAW) {
TestRunner runner;
base::string16 exe_path = MakeFullPathToSystem32(L"findstr.exe");
base::string16 system32 = MakeFullPathToSystem32(L"");
ASSERT_TRUE(!exe_path.empty());
base::string16 maybe_virtual_exe_path = MakePathToSys(L"findstr.exe", false);
base::string16 non_virtual_exe_path = MakePathToSys32(L"findstr.exe", false);
ASSERT_TRUE(!maybe_virtual_exe_path.empty());
EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_PROCESS,
TargetPolicy::PROCESS_MIN_EXEC,
exe_path.c_str()));
maybe_virtual_exe_path.c_str()));
if (non_virtual_exe_path != maybe_virtual_exe_path) {
EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_PROCESS,
TargetPolicy::PROCESS_MIN_EXEC,
non_virtual_exe_path.c_str()));
}
// Need to add directory rules for the directories that we use in
// SetCurrentDirectory.
EXPECT_TRUE(runner.AddFsRule(TargetPolicy::FILES_ALLOW_DIR_ANY,
system32.c_str()));
EXPECT_TRUE(runner.AddRuleSys32(TargetPolicy::FILES_ALLOW_DIR_ANY, L""));
wchar_t current_directory[MAX_PATH];
DWORD ret = ::GetCurrentDirectory(MAX_PATH, current_directory);
......@@ -330,6 +321,7 @@ TEST(ProcessPolicyTest, CreateProcessAW) {
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"Process_RunApp1 calc.exe"));
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"Process_RunApp2 calc.exe"));
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"Process_RunApp3 calc.exe"));
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"Process_RunApp4 calc.exe"));
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"Process_RunApp5 calc.exe"));
EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"Process_RunApp6 calc.exe"));
......@@ -339,22 +331,12 @@ TEST(ProcessPolicyTest, CreateProcessAW) {
runner.RunTest(L"Process_RunApp2 findstr.exe"));
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
runner.RunTest(L"Process_RunApp3 findstr.exe"));
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
runner.RunTest(L"Process_RunApp4 findstr.exe"));
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
runner.RunTest(L"Process_RunApp5 findstr.exe"));
EXPECT_EQ(SBOX_TEST_SUCCEEDED,
runner.RunTest(L"Process_RunApp6 findstr.exe"));
#if !defined(_WIN64)
if (base::win::OSInfo::GetInstance()->version() >= base::win::VERSION_VISTA &&
base::win::OSInfo::GetInstance()->architecture() !=
base::win::OSInfo::X86_ARCHITECTURE) { // http://crbug.com/580800
// WinXP results are not reliable.
EXPECT_EQ(SBOX_TEST_SECOND_ERROR,
runner.RunTest(L"Process_RunApp4 calc.exe"));
EXPECT_EQ(SBOX_TEST_SECOND_ERROR,
runner.RunTest(L"Process_RunApp4 findstr.exe"));
}
#endif
}
TEST(ProcessPolicyTest, OpenToken) {
......@@ -364,7 +346,7 @@ TEST(ProcessPolicyTest, OpenToken) {
TEST(ProcessPolicyTest, TestGetProcessTokenMinAccess) {
TestRunner runner;
base::string16 exe_path = MakeFullPathToSystem32(L"findstr.exe");
base::string16 exe_path = MakePathToSys(L"findstr.exe", false);
ASSERT_TRUE(!exe_path.empty());
EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_PROCESS,
TargetPolicy::PROCESS_MIN_EXEC,
......@@ -376,7 +358,7 @@ TEST(ProcessPolicyTest, TestGetProcessTokenMinAccess) {
TEST(ProcessPolicyTest, TestGetProcessTokenMaxAccess) {
TestRunner runner(JOB_UNPROTECTED, USER_INTERACTIVE, USER_INTERACTIVE);
base::string16 exe_path = MakeFullPathToSystem32(L"findstr.exe");
base::string16 exe_path = MakePathToSys(L"findstr.exe", false);
ASSERT_TRUE(!exe_path.empty());
EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_PROCESS,
TargetPolicy::PROCESS_ALL_EXEC,
......@@ -388,7 +370,7 @@ TEST(ProcessPolicyTest, TestGetProcessTokenMaxAccess) {
TEST(ProcessPolicyTest, TestGetProcessTokenMinAccessNoJob) {
TestRunner runner(JOB_NONE, USER_RESTRICTED_SAME_ACCESS, USER_LOCKDOWN);
base::string16 exe_path = MakeFullPathToSystem32(L"findstr.exe");
base::string16 exe_path = MakePathToSys(L"findstr.exe", false);
ASSERT_TRUE(!exe_path.empty());
EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_PROCESS,
TargetPolicy::PROCESS_MIN_EXEC,
......@@ -400,7 +382,7 @@ TEST(ProcessPolicyTest, TestGetProcessTokenMinAccessNoJob) {
TEST(ProcessPolicyTest, TestGetProcessTokenMaxAccessNoJob) {
TestRunner runner(JOB_NONE, USER_INTERACTIVE, USER_INTERACTIVE);
base::string16 exe_path = MakeFullPathToSystem32(L"findstr.exe");
base::string16 exe_path = MakePathToSys(L"findstr.exe", false);
ASSERT_TRUE(!exe_path.empty());
EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_PROCESS,
TargetPolicy::PROCESS_ALL_EXEC,
......
sandbox/win/tests/common/controller.cc
View file @ 23f88f7f
......@@ -18,6 +18,17 @@ namespace {
static const int kDefaultTimeout = 60000;
bool IsProcessRunning(HANDLE process) {
DWORD exit_code = 0;
if (::GetExitCodeProcess(process, &exit_code))
return exit_code == STILL_ACTIVE;
return false;
}
} // namespace
namespace sandbox {
// Constructs a full path to a file inside the system32 folder.
base::string16 MakePathToSys32(const wchar_t* name, bool is_obj_man_path) {
wchar_t windows_path[MAX_PATH] = {0};
......@@ -54,17 +65,6 @@ base::string16 MakePathToSysWow64(const wchar_t* name, bool is_obj_man_path) {
return full_path;
}
bool IsProcessRunning(HANDLE process) {
DWORD exit_code = 0;
if (::GetExitCodeProcess(process, &exit_code))
return exit_code == STILL_ACTIVE;
return false;
}
} // namespace
namespace sandbox {
base::string16 MakePathToSys(const wchar_t* name, bool is_obj_man_path) {
return (base::win::OSInfo::GetInstance()->wow64_status() ==
base::win::OSInfo::WOW64_ENABLED) ?
......
sandbox/win/tests/common/controller.h
View file @ 23f88f7f
......@@ -148,7 +148,14 @@ class TestRunner {
// Returns the broker services.
BrokerServices* GetBroker();
// Constructs a full path to a file inside the system32 (or syswow64) folder.
// Constructs a full path to a file inside the system32 folder.
base::string16 MakePathToSys32(const wchar_t* name, bool is_obj_man_path);
// Constructs a full path to a file inside the syswow64 folder.
base::string16 MakePathToSysWow64(const wchar_t* name, bool is_obj_man_path);
// Constructs a full path to a file inside the system32 (or syswow64) folder
// depending on whether process is running in wow64 or not.
base::string16 MakePathToSys(const wchar_t* name, bool is_obj_man_path);
// Runs the given test on the target process.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment