Don't apply the SandboxPlugins flag until we know a plugin will be used

third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp

 /**
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Stefan Schimanski (1Stein@gmx.de)
  * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 20 matching lines @@
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/events/Event.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
 #include "core/frame/csp/ContentSecurityPolicy.h"
 #include "core/html/HTMLContentElement.h"
 #include "core/html/HTMLImageLoader.h"
 #include "core/html/PluginDocument.h"
 #include "core/input/EventHandler.h"
+#include "core/inspector/ConsoleMessage.h"
 #include "core/layout/LayoutBlockFlow.h"
 #include "core/layout/LayoutEmbeddedObject.h"
 #include "core/layout/LayoutImage.h"
 #include "core/layout/LayoutPart.h"
 #include "core/loader/FrameLoaderClient.h"
 #include "core/loader/MixedContentChecker.h"
 #include "core/page/Page.h"
 #include "core/page/scrolling/ScrollingCoordinator.h"
 #include "core/plugins/PluginView.h"
 #include "platform/Logging.h"
@@ skipping 416 matching lines @@
 
 bool HTMLPlugInElement::requestObject(const String& url, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues)
 {
     if (url.isEmpty() && mimeType.isEmpty())
         return false;
 
     if (protocolIsJavaScript(url))
         return false;
 
     KURL completedURL = url.isEmpty() ? KURL() : document().completeURL(url);
-    if (!pluginIsLoadable(completedURL, mimeType))
+    if (!objectIsLoadable(completedURL, mimeType))
         return false;
 
     bool useFallback;
-    if (shouldUsePlugin(completedURL, mimeType, hasFallbackContent(), useFallback))
+    if (shouldUsePlugin(completedURL, mimeType, hasFallbackContent(), useFallback)) {
+        if (document().isSandboxed(SandboxPlugins)) {
+            document().addConsoleMessage(ConsoleMessage::create(SecurityMessageSource, ErrorMessageLevel,
+                "Failed to load '" + completedURL.elidedString() + "' as a plugin, because the frame into which the plugin is loading is sandboxed."));
+            return false;
+        }
         return loadPlugin(completedURL, mimeType, paramNames, paramValues, useFallback, true);

[pdr., 2016/01/29 22:14:54]

There is a scary side-codepath
(requestPluginCreationWithoutLayoutObjectIfPossible). Can this code be
structured so loadPlugin can ASSERT_WITH_SECURITY_IMPLICATION(objectIsLoadable
&& shouldUsePlugin)?

[fs, 2016/01/30 00:03:09]

Yeah, I noticed that - and was equally frightened. Looking closer though it
appeared to be a (poorly marked) escape-hatch for tests (test_runner embedder
impl. is the only thing that doesn't block it/return false.) I guess that
doesn't necessarily stop us from pushing the sandbox-check down into loadPlugin
- hopefully no test actually relies on the escape-hatch. Alternatively just push
the requireLayoutObject bool through requestObject too and then call that in
requestPluginCreationWithoutLayoutObjectIfPossible.

+    }
 
     // If the plugin element already contains a subframe,
     // loadOrRedirectSubframe will re-use it. Otherwise, it will create a new
     // frame and set it as the LayoutPart's widget, causing what was previously
     // in the widget to be torn down.
     return loadOrRedirectSubframe(completedURL, getNameAttribute(), true);
 }
 
 bool HTMLPlugInElement::loadPlugin(const KURL& url, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues, bool useFallback, bool requireLayoutObject)
 {
@@ skipping 41 matching lines @@
 }
 
 bool HTMLPlugInElement::shouldUsePlugin(const KURL& url, const String& mimeType, bool hasFallback, bool& useFallback)
 {
     // Allow other plugins to win over QuickTime because if the user has
     // installed a plugin that can handle TIFF (which QuickTime can also
     // handle) they probably intended to override QT.
     if (document().frame()->page() && (mimeType == "image/tiff" || mimeType == "image/tif" || mimeType == "image/x-tiff")) {
         const PluginData* pluginData = document().frame()->page()->pluginData();
         String pluginName = pluginData ? pluginData->pluginNameForMimeType(mimeType) : String();
-        if (!pluginName.isEmpty() && !pluginName.contains("QuickTime", TextCaseInsensitive))
+        if (!pluginName.isEmpty() && !pluginName.contains("QuickTime", TextCaseInsensitive)) {
+            useFallback = false;
             return true;
+        }
     }
 
     ObjectContentType objectType = document().frame()->loader().client()->objectContentType(url, mimeType, shouldPreferPlugInsForImages());
     // If an object's content can't be handled and it has no fallback, let
     // it be handled as a plugin to show the broken plugin icon.
     useFallback = objectType == ObjectContentNone && hasFallback;
     return objectType == ObjectContentNone || objectType == ObjectContentNetscapePlugin || objectType == ObjectContentOtherPlugin;
 
 }
 
 void HTMLPlugInElement::dispatchErrorEvent()
 {
     if (document().isPluginDocument() && document().ownerElement())
         document().ownerElement()->dispatchEvent(Event::create(EventTypeNames::error));
     else
         dispatchEvent(Event::create(EventTypeNames::error));
 }
 
-bool HTMLPlugInElement::pluginIsLoadable(const KURL& url, const String& mimeType)
+bool HTMLPlugInElement::objectIsLoadable(const KURL& url, const String& mimeType)
 {
     if (url.isEmpty() && mimeType.isEmpty())
         return false;
 
     LocalFrame* frame = document().frame();
     Settings* settings = frame->settings();
     if (!settings)
         return false;
 
     if (MIMETypeRegistry::isJavaAppletMIMEType(mimeType))
         return false;
 
-    if (document().isSandboxed(SandboxPlugins))

[pdr., 2016/01/29 22:14:54]

It seems odd to me that sandboxing and the plugin csp checks below result in
different behavior. Is that intended?

[fs, 2016/01/30 00:03:09]

It's intended (for now at least) - as in better to tread with caution. AIUI,
this means you can still block an image/svg+xml object via a CSP directive.
Arguably it should move to within shouldUsePlugin though.

[fs, 2016/02/01 09:21:06]

I shuffled the code a bit and the plugin-CSP check is now grouped with the
sandbox check.

[fs, 2016/02/01 11:52:34]

Doing that added a new failure:

http/tests/security/contentSecurityPolicy/1.1/plugintypes-notype-url.html

I guess this makes us take the mimeType.isEmpty() path in
FrameLoaderClientImpl::objectContentType and end up returning
ObjectContentFrame.

-        return false;
-
     if (!document().securityOrigin()->canDisplay(url)) {
         FrameLoader::reportLocalLoadFailed(frame, url.string());
         return false;
     }
 
     AtomicString declaredMimeType = document().isPluginDocument() && document().ownerElement() ?
         document().ownerElement()->fastGetAttribute(HTMLNames::typeAttr) :
         fastGetAttribute(HTMLNames::typeAttr);
     if (!document().contentSecurityPolicy()->allowObjectFromSource(url)
         || !document().contentSecurityPolicy()->allowPluginTypeForDocument(document(), mimeType, declaredMimeType, url)) {
@@ skipping 24 matching lines @@
     return openShadowRoot();
 }
 
 void HTMLPlugInElement::lazyReattachIfNeeded()
 {
     if (!useFallbackContent() && needsWidgetUpdate() && layoutObject() && !isImageType())
         lazyReattachIfAttached();
 }
 
 } // namespace blink