Bug 1303325: Revert changes to policy_target.cc that cause issue with CoInitializeSecurity. r=aklotz
authorBob Owen <bobowencode@gmail.com>
Fri, 16 Sep 2016 13:49:53 +0100
changeset 355558 f4be1a7f9b3ee81960fa2f81a55c87ea11784bb2
parent 355557 0a60d2f3b7027f9230d07e764fa2a48bf92140ee
child 355559 03f631f0fca7d908f954d3a36faa18d087b28212
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz
bugs1303325, 1287426
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1303325: Revert changes to policy_target.cc that cause issue with CoInitializeSecurity. r=aklotz This also reverts the Bug 1287426 Part 8 patch that turned the USER_NON_ADMIN loken into a restricted token. MozReview-Commit-ID: 9fNeyhAHw55
security/sandbox/chromium/sandbox/win/src/policy_target.cc
security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc
security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
security/sandbox/moz-chromium-commit-status.txt
--- a/security/sandbox/chromium/sandbox/win/src/policy_target.cc
+++ b/security/sandbox/chromium/sandbox/win/src/policy_target.cc
@@ -76,16 +76,26 @@ NTSTATUS WINAPI TargetNtSetInformationTh
     NtSetInformationThreadFunction orig_SetInformationThread, HANDLE thread,
     NT_THREAD_INFORMATION_CLASS thread_info_class, PVOID thread_information,
     ULONG thread_information_bytes) {
   do {
     if (SandboxFactory::GetTargetServices()->GetState()->RevertedToSelf())
       break;
     if (ThreadImpersonationToken != thread_info_class)
       break;
+    if (!thread_information)
+      break;
+    HANDLE token;
+    if (sizeof(token) > thread_information_bytes)
+      break;
+
+    NTSTATUS ret = CopyData(&token, thread_information, sizeof(token));
+    if (!NT_SUCCESS(ret) || NULL != token)
+      break;
+
     // This is a revert to self.
     return STATUS_SUCCESS;
   } while (false);
 
   return orig_SetInformationThread(thread, thread_info_class,
                                    thread_information,
                                    thread_information_bytes);
 }
--- a/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc
+++ b/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc
@@ -48,29 +48,16 @@ DWORD CreateRestrictedToken(TokenLevel s
       break;
     }
     case USER_NON_ADMIN: {
       sid_exceptions.push_back(WinBuiltinUsersSid);
       sid_exceptions.push_back(WinWorldSid);
       sid_exceptions.push_back(WinInteractiveSid);
       sid_exceptions.push_back(WinAuthenticatedUserSid);
       privilege_exceptions.push_back(SE_CHANGE_NOTIFY_NAME);
-      // We need to make USER_NON_ADMIN into a restricted token to work around a
-      // conflict with a call to CoInitializeSecurity (see bug 1287426).
-      // To do this we add the same restricted SIDs as USER_INTERACTIVE, because
-      // USER_NON_ADMIN should have at least the same permissions. We also add
-      // in any that are in the deny only exception list above, which should
-      // give the new USER_NON_ADMIN token the same permissions as the old.
-      restricted_token.AddRestrictingSid(WinBuiltinUsersSid);
-      restricted_token.AddRestrictingSid(WinWorldSid);
-      restricted_token.AddRestrictingSid(WinInteractiveSid);
-      restricted_token.AddRestrictingSid(WinAuthenticatedUserSid);
-      restricted_token.AddRestrictingSid(WinRestrictedCodeSid);
-      restricted_token.AddRestrictingSidCurrentUser();
-      restricted_token.AddRestrictingSidLogonSession();
       break;
     }
     case USER_INTERACTIVE: {
       sid_exceptions.push_back(WinBuiltinUsersSid);
       sid_exceptions.push_back(WinWorldSid);
       sid_exceptions.push_back(WinInteractiveSid);
       sid_exceptions.push_back(WinAuthenticatedUserSid);
       privilege_exceptions.push_back(SE_CHANGE_NOTIFY_NAME);
--- a/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
+++ b/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
@@ -1,9 +1,8 @@
 Please add a link to the bugzilla bug and patch name that should be re-applied.
 Also, please update any existing links to their actual mozilla-central changeset.
 
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part4.patch
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part5.patch
 https://hg.mozilla.org/mozilla-central/rev/7df8d6639971
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part6.patch
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part7.patch
-https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part8.patch
--- a/security/sandbox/moz-chromium-commit-status.txt
+++ b/security/sandbox/moz-chromium-commit-status.txt
@@ -1,3 +1,4 @@
 Chromium Commit                            Directory / File (relative to security/sandbox/)
 ----------------------------------------   ------------------------------------------------
 4ec79b7f2379a60cdc15599e93255c0fa417f1ed   chromium
+611754aea9d1c0ba5c7980fa267fd005dc249b85   chromium/sandbox/win/src/policy_target.cc