Reduce sandbox permissions granted to alternate desktop

This pass adds the first round of deny ACEs for the Winstation and
Desktop objects. Assuming these stick, I'll get more aggressive in
a follow-up.

BUG=346586
NOTRY=true

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253546 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 134 insertions, 26 deletions

diff --git a/sandbox/win/sandbox_poc/main_ui_window.cc b/sandbox/win/sandbox_poc/main_ui_window.cc
index 243075c..1671424 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) 2006-2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2014 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,8 +553,9 @@ bool MainUIWindow::SpawnTarget() {
     if (INVALID_HANDLE_VALUE == pipe_handle_)
       AddDebugMessage(L"Failed to create pipe. Error %d", ::GetLastError());
 
-    if (!sandbox::AddKnownSidToKernelObject(pipe_handle_, WinWorldSid,
-                                            FILE_ALL_ACCESS))
+    if (!sandbox::AddKnownSidToObject(pipe_handle_, SE_KERNEL_OBJECT,
+                                      WinWorldSid, GRANT_ACCESS,
+                                      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 70d2a8d..0f2936ab 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_MASK access,
-                  ACL** new_dacl) {
+bool AddSidToDacl(const Sid& sid, ACL* old_dacl, ACCESS_MODE access_mode,
+                  ACCESS_MASK access, ACL** new_dacl) {
   EXPLICIT_ACCESS new_access = {0};
-  new_access.grfAccessMode = GRANT_ACCESS;
+  new_access.grfAccessMode = access_mode;
   new_access.grfAccessPermissions = access;
   new_access.grfInheritance = NO_INHERITANCE;
 
@@ -64,7 +64,8 @@ bool AddSidToDefaultDacl(HANDLE token, const Sid& sid, ACCESS_MASK access) {
     return false;
 
   ACL* new_dacl = NULL;
-  if (!AddSidToDacl(sid, default_dacl->DefaultDacl, access, &new_dacl))
+  if (!AddSidToDacl(sid, default_dacl->DefaultDacl, GRANT_ACCESS, access,
+                    &new_dacl))
     return false;
 
   TOKEN_DEFAULT_DACL new_token_dacl = {0};
@@ -90,8 +91,9 @@ bool AddUserSidToDefaultDacl(HANDLE token, ACCESS_MASK access) {
                              access);
 }
 
-bool AddKnownSidToKernelObject(HANDLE object, const Sid& sid,
-                               ACCESS_MASK access) {
+bool AddKnownSidToObject(HANDLE object, SE_OBJECT_TYPE object_type,
+                         const Sid& sid, ACCESS_MODE access_mode,
+                         ACCESS_MASK access) {
   PSECURITY_DESCRIPTOR descriptor = NULL;
   PACL old_dacl = NULL;
   PACL new_dacl = NULL;
@@ -101,12 +103,12 @@ bool AddKnownSidToKernelObject(HANDLE object, const Sid& sid,
                                          &old_dacl, NULL, &descriptor))
     return false;
 
-  if (!AddSidToDacl(sid.GetPSID(), old_dacl, access, &new_dacl)) {
+  if (!AddSidToDacl(sid.GetPSID(), old_dacl, access_mode, access, &new_dacl)) {
     ::LocalFree(descriptor);
     return false;
   }
 
-  DWORD result = ::SetSecurityInfo(object, SE_KERNEL_OBJECT,
+  DWORD result = ::SetSecurityInfo(object, object_type,
                                    DACL_SECURITY_INFORMATION, NULL, NULL,
                                    new_dacl, NULL);
 
diff --git a/sandbox/win/src/acl.h b/sandbox/win/src/acl.h
index 25d5cdb..531259f 100644
--- a/sandbox/win/src/acl.h
+++ b/sandbox/win/src/acl.h
@@ -5,6 +5,7 @@
 #ifndef SANDBOX_SRC_ACL_H_
 #define SANDBOX_SRC_ACL_H_
 
+#include <AccCtrl.h>
 #include <windows.h>
 
 #include "base/memory/scoped_ptr.h"
@@ -16,11 +17,11 @@ namespace sandbox {
 bool GetDefaultDacl(HANDLE token,
                     scoped_ptr_malloc<TOKEN_DEFAULT_DACL>* default_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);
+// 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);
 
 // Adds and ACE represented by |sid| and |access| to the default dacl present
 // in the token.
@@ -30,10 +31,11 @@ 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| and |access| to the dacl of the kernel
-// object referenced by |object|.
-bool AddKnownSidToKernelObject(HANDLE object, const Sid& sid,
-                               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);
 
 }  // namespace sandbox
 
diff --git a/sandbox/win/src/window.cc b/sandbox/win/src/window.cc
index d21858a..32c02b2 100644
--- a/sandbox/win/src/window.cc
+++ b/sandbox/win/src/window.cc
@@ -8,6 +8,8 @@
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "sandbox/win/src/acl.h"
+#include "sandbox/win/src/sid.h"
 
 namespace {
 
@@ -46,8 +48,20 @@ ResultCode CreateAltWindowStation(HWINSTA* winsta) {
   *winsta = ::CreateWindowStationW(NULL, 0, WINSTA_ALL_ACCESS, &attributes);
   LocalFree(attributes.lpSecurityDescriptor);
 
-  if (*winsta)
+  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);
     return SBOX_ALL_OK;
+  }
 
   return SBOX_ERROR_CANNOT_CREATE_WINSTATION;
 }
@@ -94,8 +108,18 @@ ResultCode CreateAltDesktop(HWINSTA winsta, HDESK* desktop) {
     }
   }
 
-  if (*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);
     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 eefc498..e7620c3 100644
--- a/sandbox/win/tests/validation_tests/commands.cc
+++ b/sandbox/win/tests/validation_tests/commands.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <Aclapi.h>
 #include <windows.h>
 #include <string>
 
@@ -240,11 +241,62 @@ SBOX_TESTS_COMMAND int SwitchToSboxDesktop(int, wchar_t **) {
 }
 
 int TestSwitchDesktop() {
-  HDESK sbox_desk = ::GetThreadDesktop(::GetCurrentThreadId());
-  if (NULL == sbox_desk) {
+  HDESK desktop = ::GetThreadDesktop(::GetCurrentThreadId());
+  if (NULL == desktop) {
     return SBOX_TEST_FAILED;
   }
-  if (::SwitchDesktop(sbox_desk)) {
+  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_FAILED;
+    }
+  }
+
+  // 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)) {
     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 3a6a0f5..f9b6199 100644
--- a/sandbox/win/tests/validation_tests/commands.h
+++ b/sandbox/win/tests/validation_tests/commands.h
@@ -36,6 +36,13 @@ 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 95209b7..f85b8da 100644
--- a/sandbox/win/tests/validation_tests/suite.cc
+++ b/sandbox/win/tests/validation_tests/suite.cc
@@ -111,11 +111,31 @@ TEST(ValidationSuite, TestRegistry) {
 // to get to the interactive desktop or to make the sbox desktop interactive.
 TEST(ValidationSuite, TestDesktop) {
   TestRunner runner;
-  runner.GetPolicy()->SetAlternateDesktop(false);
+  runner.GetPolicy()->SetAlternateDesktop(true);
   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) {
+  base::win::Version version = base::win::GetVersion();
+  if (version < base::win::VERSION_WIN7)
+    return;
+
+  TestRunner runner;
+  EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"EnumAlternateWinsta NULL"));
+
+  wchar_t command[1024] = {0};
+  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));
+}
+
 // Tests if the windows are correctly protected by the sandbox.
 TEST(ValidationSuite, TestWindows) {
   TestRunner runner;