diff --git a/sandbox/win/sandbox_poc/main_ui_window.cc b/sandbox/win/sandbox_poc/main_ui_window.cc index 1671424942c9fa..243075c0497e92 100644 --- a/sandbox/win/sandbox_poc/main_ui_window.cc +++ b/sandbox/win/sandbox_poc/main_ui_window.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2014 The Chromium Authors. All rights reserved. +// Copyright (c) 2006-2010 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. @@ -553,9 +553,8 @@ bool MainUIWindow::SpawnTarget() { if (INVALID_HANDLE_VALUE == pipe_handle_) AddDebugMessage(L"Failed to create pipe. Error %d", ::GetLastError()); - if (!sandbox::AddKnownSidToObject(pipe_handle_, SE_KERNEL_OBJECT, - WinWorldSid, GRANT_ACCESS, - FILE_ALL_ACCESS)) + if (!sandbox::AddKnownSidToKernelObject(pipe_handle_, WinWorldSid, + FILE_ALL_ACCESS)) AddDebugMessage(L"Failed to set security on pipe. Error %d", ::GetLastError()); diff --git a/sandbox/win/src/acl.cc b/sandbox/win/src/acl.cc index 0f2936ab1ae4b5..70d2a8d31097e6 100644 --- a/sandbox/win/src/acl.cc +++ b/sandbox/win/src/acl.cc @@ -36,10 +36,10 @@ bool GetDefaultDacl(HANDLE token, return true; } -bool AddSidToDacl(const Sid& sid, ACL* old_dacl, ACCESS_MODE access_mode, - ACCESS_MASK access, ACL** new_dacl) { +bool AddSidToDacl(const Sid& sid, ACL* old_dacl, ACCESS_MASK access, + ACL** new_dacl) { EXPLICIT_ACCESS new_access = {0}; - new_access.grfAccessMode = access_mode; + new_access.grfAccessMode = GRANT_ACCESS; new_access.grfAccessPermissions = access; new_access.grfInheritance = NO_INHERITANCE; @@ -64,8 +64,7 @@ bool AddSidToDefaultDacl(HANDLE token, const Sid& sid, ACCESS_MASK access) { return false; ACL* new_dacl = NULL; - if (!AddSidToDacl(sid, default_dacl->DefaultDacl, GRANT_ACCESS, access, - &new_dacl)) + if (!AddSidToDacl(sid, default_dacl->DefaultDacl, access, &new_dacl)) return false; TOKEN_DEFAULT_DACL new_token_dacl = {0}; @@ -91,9 +90,8 @@ bool AddUserSidToDefaultDacl(HANDLE token, ACCESS_MASK access) { access); } -bool AddKnownSidToObject(HANDLE object, SE_OBJECT_TYPE object_type, - const Sid& sid, ACCESS_MODE access_mode, - ACCESS_MASK access) { +bool AddKnownSidToKernelObject(HANDLE object, const Sid& sid, + ACCESS_MASK access) { PSECURITY_DESCRIPTOR descriptor = NULL; PACL old_dacl = NULL; PACL new_dacl = NULL; @@ -103,12 +101,12 @@ bool AddKnownSidToObject(HANDLE object, SE_OBJECT_TYPE object_type, &old_dacl, NULL, &descriptor)) return false; - if (!AddSidToDacl(sid.GetPSID(), old_dacl, access_mode, access, &new_dacl)) { + if (!AddSidToDacl(sid.GetPSID(), old_dacl, access, &new_dacl)) { ::LocalFree(descriptor); return false; } - DWORD result = ::SetSecurityInfo(object, object_type, + DWORD result = ::SetSecurityInfo(object, SE_KERNEL_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, new_dacl, NULL); diff --git a/sandbox/win/src/acl.h b/sandbox/win/src/acl.h index 531259fbebde2d..25d5cdb1611b77 100644 --- a/sandbox/win/src/acl.h +++ b/sandbox/win/src/acl.h @@ -5,7 +5,6 @@ #ifndef SANDBOX_SRC_ACL_H_ #define SANDBOX_SRC_ACL_H_ -#include #include #include "base/memory/scoped_ptr.h" @@ -17,11 +16,11 @@ namespace sandbox { bool GetDefaultDacl(HANDLE token, scoped_ptr_malloc* default_dacl); -// Appends an ACE represented by |sid|, |access_mode|, and |access| to -// |old_dacl|. If the function succeeds, new_dacl contains the new dacl and -// must be freed using LocalFree. -bool AddSidToDacl(const Sid& sid, ACL* old_dacl, ACCESS_MODE access_mode, - ACCESS_MASK access, ACL** new_dacl); +// Appends an ACE represented by |sid| and |access| to |old_dacl|. If the +// function succeeds, new_dacl contains the new dacl and must be freed using +// LocalFree. +bool AddSidToDacl(const Sid& sid, ACL* old_dacl, ACCESS_MASK access, + ACL** new_dacl); // Adds and ACE represented by |sid| and |access| to the default dacl present // in the token. @@ -31,11 +30,10 @@ bool AddSidToDefaultDacl(HANDLE token, const Sid& sid, ACCESS_MASK access); // present in the token. bool AddUserSidToDefaultDacl(HANDLE token, ACCESS_MASK access); -// Adds an ACE represented by |known_sid|, |access_mode|, and |access| to -// the dacl of the kernel object referenced by |object| and of |object_type|. -bool AddKnownSidToObject(HANDLE object, SE_OBJECT_TYPE object_type, - const Sid& sid, ACCESS_MODE access_mode, - ACCESS_MASK access); +// Adds an ACE represented by |known_sid| and |access| to the dacl of the kernel +// object referenced by |object|. +bool AddKnownSidToKernelObject(HANDLE object, const Sid& sid, + ACCESS_MASK access); } // namespace sandbox diff --git a/sandbox/win/src/window.cc b/sandbox/win/src/window.cc index 32c02b22ac95ee..d21858a42a4a2c 100644 --- a/sandbox/win/src/window.cc +++ b/sandbox/win/src/window.cc @@ -8,8 +8,6 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" -#include "sandbox/win/src/acl.h" -#include "sandbox/win/src/sid.h" namespace { @@ -48,20 +46,8 @@ ResultCode CreateAltWindowStation(HWINSTA* winsta) { *winsta = ::CreateWindowStationW(NULL, 0, WINSTA_ALL_ACCESS, &attributes); LocalFree(attributes.lpSecurityDescriptor); - if (*winsta) { - // Replace the DACL on the new Winstation with a reduced privilege version. - // We can soft fail on this for now, as it's just an extra mitigation. - static const ACCESS_MASK kWinstaDenyMask = DELETE | WRITE_DAC | - WRITE_OWNER | - WINSTA_ACCESSCLIPBOARD | - WINSTA_CREATEDESKTOP | - WINSTA_ENUMDESKTOPS | - WINSTA_ENUMERATE | - WINSTA_EXITWINDOWS; - AddKnownSidToObject(*winsta, SE_WINDOW_OBJECT, Sid(WinRestrictedCodeSid), - DENY_ACCESS, kWinstaDenyMask); + if (*winsta) return SBOX_ALL_OK; - } return SBOX_ERROR_CANNOT_CREATE_WINSTATION; } @@ -108,18 +94,8 @@ ResultCode CreateAltDesktop(HWINSTA winsta, HDESK* desktop) { } } - if (*desktop) { - // Replace the DACL on the new Desktop with a reduced privilege version. - // We can soft fail on this for now, as it's just an extra mitigation. - static const ACCESS_MASK kDesktopDenyMask = WRITE_DAC | WRITE_OWNER | - DESKTOP_HOOKCONTROL | - DESKTOP_JOURNALPLAYBACK | - DESKTOP_JOURNALRECORD | - DESKTOP_SWITCHDESKTOP; - AddKnownSidToObject(*desktop, SE_WINDOW_OBJECT, Sid(WinRestrictedCodeSid), - DENY_ACCESS, kDesktopDenyMask); + if (*desktop) return SBOX_ALL_OK; - } return SBOX_ERROR_CANNOT_CREATE_DESKTOP; } diff --git a/sandbox/win/tests/validation_tests/commands.cc b/sandbox/win/tests/validation_tests/commands.cc index 357319a928cb53..eefc498c6e3882 100644 --- a/sandbox/win/tests/validation_tests/commands.cc +++ b/sandbox/win/tests/validation_tests/commands.cc @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include #include #include @@ -241,62 +240,11 @@ SBOX_TESTS_COMMAND int SwitchToSboxDesktop(int, wchar_t **) { } int TestSwitchDesktop() { - HDESK desktop = ::GetThreadDesktop(::GetCurrentThreadId()); - if (NULL == desktop) { + HDESK sbox_desk = ::GetThreadDesktop(::GetCurrentThreadId()); + if (NULL == sbox_desk) { return SBOX_TEST_FAILED; } - if (::SwitchDesktop(desktop)) { - return SBOX_TEST_SUCCEEDED; - } - return SBOX_TEST_DENIED; -} - -SBOX_TESTS_COMMAND int OpenAlternateDesktop(int, wchar_t **argv) { - return TestOpenAlternateDesktop(argv[0]); -} - -int TestOpenAlternateDesktop(wchar_t *desktop_name) { - // Test for WRITE_DAC permission on the handle. - HDESK desktop = ::GetThreadDesktop(::GetCurrentThreadId()); - if (desktop) { - HANDLE test_handle; - if (::DuplicateHandle(::GetCurrentProcess(), desktop, - ::GetCurrentProcess(), &test_handle, - WRITE_DAC, FALSE, 0)) { - DWORD result = ::SetSecurityInfo(test_handle, SE_WINDOW_OBJECT, - DACL_SECURITY_INFORMATION, NULL, NULL, - NULL, NULL); - ::CloseHandle(test_handle); - if (result != ERROR_ACCESS_DENIED) { - return SBOX_TEST_SUCCEEDED; - } - } else if (::GetLastError() != ERROR_ACCESS_DENIED) { - return SBOX_TEST_SUCCEEDED; - } - } - - // Open by name with WRITE_DAC. - if ((desktop = ::OpenDesktop(desktop_name, 0, FALSE, WRITE_DAC)) || - ::GetLastError() != ERROR_ACCESS_DENIED) { - ::CloseDesktop(desktop); - return SBOX_TEST_SUCCEEDED; - } - - return SBOX_TEST_DENIED; -} - -BOOL CALLBACK DesktopTestEnumProc(LPTSTR desktop_name, LPARAM result) { - return TRUE; -} - -SBOX_TESTS_COMMAND int EnumAlternateWinsta(int, wchar_t **) { - return TestEnumAlternateWinsta(); -} - -int TestEnumAlternateWinsta() { - int result = SBOX_TEST_DENIED; - // Try to enumerate the destops on the alternate windowstation. - if (::EnumDesktopsW(NULL, DesktopTestEnumProc, 0)) { + if (::SwitchDesktop(sbox_desk)) { return SBOX_TEST_SUCCEEDED; } return SBOX_TEST_DENIED; diff --git a/sandbox/win/tests/validation_tests/commands.h b/sandbox/win/tests/validation_tests/commands.h index f9b61990ed9e34..3a6a0f5f0dc2b9 100644 --- a/sandbox/win/tests/validation_tests/commands.h +++ b/sandbox/win/tests/validation_tests/commands.h @@ -36,13 +36,6 @@ int TestOpenInputDesktop(); // Tries to switch the interactive desktop. Returns a SboxTestResult. int TestSwitchDesktop(); -// Tries to open the alternate desktop. Returns a SboxTestResult. -int TestOpenAlternateDesktop(wchar_t *desktop_name); - -// Tries to enumerate desktops on the alternate windowstation. -// Returns a SboxTestResult. -int TestEnumAlternateWinsta(); - } // namespace sandbox #endif // SANDBOX_TESTS_VALIDATION_TESTS_COMMANDS_H__ diff --git a/sandbox/win/tests/validation_tests/suite.cc b/sandbox/win/tests/validation_tests/suite.cc index e622cc3d67c8c6..95209b7afe00de 100644 --- a/sandbox/win/tests/validation_tests/suite.cc +++ b/sandbox/win/tests/validation_tests/suite.cc @@ -111,26 +111,11 @@ TEST(ValidationSuite, TestRegistry) { // to get to the interactive desktop or to make the sbox desktop interactive. TEST(ValidationSuite, TestDesktop) { TestRunner runner; - runner.GetPolicy()->SetAlternateDesktop(true); + runner.GetPolicy()->SetAlternateDesktop(false); EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"OpenInteractiveDesktop NULL")); 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) { - wchar_t command[1024] = {0}; - TestRunner runner; - runner.SetTimeout(3600000); - runner.GetPolicy()->SetAlternateDesktop(true); - runner.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_UNTRUSTED); - base::string16 desktop_name = runner.GetPolicy()->GetAlternateDesktop(); - desktop_name = desktop_name.substr(desktop_name.find('\\') + 1); - wsprintf(command, L"OpenAlternateDesktop %lS", desktop_name.c_str()); - EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(command)); - EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"EnumAlternateWinsta NULL")); -} - // Tests if the windows are correctly protected by the sandbox. TEST(ValidationSuite, TestWindows) { TestRunner runner;