Bug 1303325: Revert changes to policy_target.cc that cause issue with CoInitializeSecurity. draft
authorBob Owen <bobowencode@gmail.com>
Fri, 16 Sep 2016 13:49:53 +0100
changeset 414498 ae0e0229f40c70b3ecfc02782ee63e89be683b16
parent 413272 82d0a583a9a39bf0b0000bccbf6d5c9ec2596bcc
child 531455 f3564726ddf746f1b8da30fbe2f61e854563c649
push id29685
push userbobowencode@gmail.com
push dateFri, 16 Sep 2016 13:10:19 +0000
bugs1303325, 1287426
milestone51.0a1
Bug 1303325: Revert changes to policy_target.cc that cause issue with CoInitializeSecurity. 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