Added impersonation of the anonymous token around CloseClipboard

ui/base/clipboard/clipboard_win.cc

 // Copyright (c) 2012 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.
 
 // Many of these functions are based on those found in
 // webkit/port/platform/PasteboardWin.cpp
 
 #include "ui/base/clipboard/clipboard_win.h"
 
 #include <shellapi.h>
@@ skipping 17 matching lines @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/clipboard/clipboard_util_win.h"
 #include "ui/base/clipboard/custom_data_helper.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/size.h"
 
 namespace ui {
 
 namespace {
 
+// A scoper to impersonate the anonymous token and revert when leaving scope
+class AnonymousImpersonator {
+ public:
+  AnonymousImpersonator() {
+    must_revert_ = ::ImpersonateAnonymousToken(::GetCurrentThread());

[Wez, 2014/12/15 17:07:11]

Under what circumstances can ImpersonateAnonymousToken fail?

Should we instead be CHECK()ing this, since if it fails, we're not impersonating
the anonymous token? 

[forshaw, 2014/12/16 09:07:30]

It can fail if we're running under a restricted token, other than that it
shouldn't fail. However if we're in a restricted token it isn't as big an issue
if the token is captured. I don't think it needs a CHECK though as we don't want
to crash if it fails as it only makes a difference in a very limited set of
scenarios. 

+  }
+
+  ~AnonymousImpersonator() {
+    if (must_revert_)
+      ::RevertToSelf();
+  }
+
+ private:
+  BOOL must_revert_;
+  DISALLOW_COPY_AND_ASSIGN(AnonymousImpersonator);

[dcheng, 2014/12/15 16:58:20]

Note: I usually see a newline between DISALLOW_COPY_AND_ASSIGN and the rest of
the class definition.

[forshaw, 2014/12/16 09:07:30]

Acknowledged.

+};
+
 // A scoper to manage acquiring and automatically releasing the clipboard.
 class ScopedClipboard {
  public:
   ScopedClipboard() : opened_(false) { }
 
   ~ScopedClipboard() {
     if (opened_)
       Release();
   }
 
@@ skipping 29 matching lines @@
         return true;
       }
     }
 
     // We failed to acquire the clipboard.
     return false;
   }
 
   void Release() {
     if (opened_) {
+      // Impersonate the anonymous token during the call to CloseClipboard
+      // This prevents Windows 8+ capturing the broker's access token which
+      // could be accessed by lower-privileges chrome processes leading to
+      // a risk of EoP

[Wez, 2014/12/15 17:07:11]

nit: punctuation

Impersonating the anonymous token presumably impacts the behaviour of e.g.
putting files onto the clipboard - any application which expects to be able to
use GetClipboardAuthToken() to ensure that it has access to the files is going
to fail. We should document that likely impact, at least.

[forshaw, 2014/12/16 09:07:30]

The only thing I think this should impact is anyone calling the
GetClipboardAccessToken system call which as far as I can tell is limited to a
few internal DLLs. It isn't documented from what I can determine or clear what
it's actual purpose is. It shouldn't affect the processing of files transferred
over the clipboard but it's always a possibility that something won't work
correctly afterwards. 

+      AnonymousImpersonator impersonator;
       ::CloseClipboard();
       opened_ = false;
     } else {
       NOTREACHED();
     }
   }
 
  private:
   bool opened_;
 };
@@ skipping 756 matching lines @@
   if (!clipboard_owner_)
     return NULL;
 
   if (clipboard_owner_->hwnd() == NULL)
     clipboard_owner_->Create(base::Bind(&ClipboardOwnerWndProc));
 
   return clipboard_owner_->hwnd();
 }
 
 }  // namespace ui