Bug 1484980 - Add selective canvas tainting for content scripts r=bzbarsky
authorTomislav Jovanovic <tomica@gmail.com>
Wed, 26 Sep 2018 20:29:36 +0200
changeset 438914 3039bfc0e2707fa57523756d29de73703baccd59
parent 438913 d3fd96af0ca264b2637b88107420070ea9400700
child 438915 be070bf5d08a5fa7efb83751b7b27d5f1269e5da
push id34742
push userccoroiu@mozilla.com
push dateSun, 30 Sep 2018 09:46:37 +0000
treeherdermozilla-central@7e9ab0e7b608 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1484980
milestone64.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 1484980 - Add selective canvas tainting for content scripts r=bzbarsky Reviewers: bzbarsky Bug #: 1484980 Differential Revision: https://phabricator.services.mozilla.com/D6999
dom/canvas/CanvasRenderingContext2D.cpp
dom/canvas/CanvasUtils.cpp
dom/html/HTMLCanvasElement.cpp
dom/html/HTMLCanvasElement.h
toolkit/components/extensions/test/xpcshell/data/pixel_green.gif
toolkit/components/extensions/test/xpcshell/data/pixel_red.gif
toolkit/components/extensions/test/xpcshell/test_ext_contentscript_canvas_tainting.js
toolkit/components/extensions/test/xpcshell/xpcshell-content.ini
--- a/dom/canvas/CanvasRenderingContext2D.cpp
+++ b/dom/canvas/CanvasRenderingContext2D.cpp
@@ -5434,23 +5434,17 @@ CanvasRenderingContext2D::GetImageData(J
   if (!mCanvasElement && !mDocShell) {
     NS_ERROR("No canvas element and no docshell in GetImageData!!!");
     aError.Throw(NS_ERROR_DOM_SECURITY_ERR);
     return nullptr;
   }
 
   // Check only if we have a canvas element; if we were created with a docshell,
   // then it's special internal use.
-  if (mCanvasElement && mCanvasElement->IsWriteOnly() &&
-      // We could ask bindings for the caller type, but they already hand us a
-      // JSContext, and we're at least _somewhat_ perf-sensitive (so may not
-      // want to compute the caller type in the common non-write-only case), so
-      // let's just use what we have.
-      !nsContentUtils::CallerHasPermission(aCx, nsGkAtoms::all_urlsPermission))
-  {
+  if (mCanvasElement && !mCanvasElement->CallerCanRead(aCx)) {
     // XXX ERRMSG we need to report an error to developers here! (bug 329026)
     aError.Throw(NS_ERROR_DOM_SECURITY_ERR);
     return nullptr;
   }
 
   if (!IsFinite(aSx) || !IsFinite(aSy) ||
       !IsFinite(aSw) || !IsFinite(aSh)) {
     aError.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
--- a/dom/canvas/CanvasUtils.cpp
+++ b/dom/canvas/CanvasUtils.cpp
@@ -228,18 +228,19 @@ DoDrawImageSecurityCheck(dom::HTMLCanvas
                          bool CORSUsed)
 {
     // Callers should ensure that mCanvasElement is non-null before calling this
     if (!aCanvasElement) {
         NS_WARNING("DoDrawImageSecurityCheck called without canvas element!");
         return;
     }
 
-    if (aCanvasElement->IsWriteOnly())
+    if (aCanvasElement->IsWriteOnly() && !aCanvasElement->mExpandedReader) {
         return;
+    }
 
     // If we explicitly set WriteOnly just do it and get out
     if (forceWriteOnly) {
         aCanvasElement->SetWriteOnly();
         return;
     }
 
     // No need to do a security check if the image used CORS for the load
@@ -248,16 +249,35 @@ DoDrawImageSecurityCheck(dom::HTMLCanvas
 
     MOZ_ASSERT(aPrincipal, "Must have a principal here");
 
     if (aCanvasElement->NodePrincipal()->Subsumes(aPrincipal)) {
         // This canvas has access to that image anyway
         return;
     }
 
+    if (BasePrincipal::Cast(aPrincipal)->AddonPolicy()) {
+        // This is a resource from an extension content script principal.
+
+        if (aCanvasElement->mExpandedReader &&
+            aCanvasElement->mExpandedReader->Subsumes(aPrincipal)) {
+            // This canvas already allows reading from this principal.
+            return;
+        }
+
+        if (!aCanvasElement->mExpandedReader) {
+            // Allow future reads from this same princial only.
+            aCanvasElement->SetWriteOnly(aPrincipal);
+            return;
+        }
+
+        // If we got here, this must be the *second* extension tainting
+        // the canvas.  Fall through to mark it WriteOnly for everyone.
+    }
+
     aCanvasElement->SetWriteOnly();
 }
 
 bool
 CoerceDouble(const JS::Value& v, double* d)
 {
     if (v.isDouble()) {
         *d = v.toDouble();
--- a/dom/html/HTMLCanvasElement.cpp
+++ b/dom/html/HTMLCanvasElement.cpp
@@ -665,19 +665,18 @@ HTMLCanvasElement::ParseAttribute(int32_
 
 void
 HTMLCanvasElement::ToDataURL(JSContext* aCx, const nsAString& aType,
                              JS::Handle<JS::Value> aParams,
                              nsAString& aDataURL,
                              nsIPrincipal& aSubjectPrincipal,
                              ErrorResult& aRv)
 {
-  // do a trust check if this is a write-only canvas
-  if (mWriteOnly &&
-      !nsContentUtils::CallerHasPermission(aCx, nsGkAtoms::all_urlsPermission)) {
+  // mWriteOnly check is redundant, but optimizes for the common case.
+  if (mWriteOnly && !CallerCanRead(aCx)) {
     aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
     return;
   }
 
   aRv = ToDataURLImpl(aCx, aSubjectPrincipal, aType, aParams, aDataURL);
 }
 
 void
@@ -876,19 +875,18 @@ HTMLCanvasElement::ToDataURLImpl(JSConte
 void
 HTMLCanvasElement::ToBlob(JSContext* aCx,
                           BlobCallback& aCallback,
                           const nsAString& aType,
                           JS::Handle<JS::Value> aParams,
                           nsIPrincipal& aSubjectPrincipal,
                           ErrorResult& aRv)
 {
-  // do a trust check if this is a write-only canvas
-  if (mWriteOnly &&
-      !nsContentUtils::CallerHasPermission(aCx, nsGkAtoms::all_urlsPermission)) {
+  // mWriteOnly check is redundant, but optimizes for the common case.
+  if (mWriteOnly && !CallerCanRead(aCx)) {
     aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
     return;
   }
 
   nsCOMPtr<nsIGlobalObject> global = OwnerDoc()->GetScopeObject();
   MOZ_ASSERT(global);
 
   nsIntSize elemSize = GetWidthHeight();
@@ -1088,20 +1086,47 @@ bool
 HTMLCanvasElement::IsWriteOnly()
 {
   return mWriteOnly;
 }
 
 void
 HTMLCanvasElement::SetWriteOnly()
 {
+  mExpandedReader = nullptr;
   mWriteOnly = true;
 }
 
 void
+HTMLCanvasElement::SetWriteOnly(nsIPrincipal* aExpandedReader)
+{
+  mExpandedReader = aExpandedReader;
+  mWriteOnly = true;
+}
+
+bool
+HTMLCanvasElement::CallerCanRead(JSContext* aCx)
+{
+  if (!mWriteOnly) {
+    return true;
+  }
+
+  nsIPrincipal* prin = nsContentUtils::SubjectPrincipal(aCx);
+
+  // If mExpandedReader is set, this canvas was tainted only by
+  // mExpandedReader's resources. So allow reading if the subject
+  // principal subsumes mExpandedReader.
+  if (mExpandedReader && prin->Subsumes(mExpandedReader)) {
+    return true;
+  }
+
+  return nsContentUtils::PrincipalHasPermission(prin, nsGkAtoms::all_urlsPermission);
+}
+
+void
 HTMLCanvasElement::InvalidateCanvasContent(const gfx::Rect* damageRect)
 {
   // We don't need to flush anything here; if there's no frame or if
   // we plan to reframe we don't need to invalidate it anyway.
   nsIFrame *frame = GetPrimaryFrame();
   if (!frame)
     return;
 
--- a/dom/html/HTMLCanvasElement.h
+++ b/dom/html/HTMLCanvasElement.h
@@ -226,16 +226,22 @@ public:
   bool IsWriteOnly();
 
   /**
    * Force the canvas to be write-only.
    */
   void SetWriteOnly();
 
   /**
+   * Force the canvas to be write-only, except for readers from
+   * a specific extension's content script expanded principal.
+   */
+  void SetWriteOnly(nsIPrincipal* aExpandedReader);
+
+  /**
    * Notify that some canvas content has changed and the window may
    * need to be updated. aDamageRect is in canvas coordinates.
    */
   void InvalidateCanvasContent(const mozilla::gfx::Rect* aDamageRect);
   /*
    * Notify that we need to repaint the entire canvas, including updating of
    * the layer tree.
    */
@@ -390,18 +396,25 @@ protected:
   RefPtr<OffscreenCanvas> mOffscreenCanvas;
   RefPtr<HTMLCanvasElementObserver> mContextObserver;
 
 public:
   // Record whether this canvas should be write-only or not.
   // We set this when script paints an image from a different origin.
   // We also transitively set it when script paints a canvas which
   // is itself write-only.
-  bool                     mWriteOnly;
+  bool mWriteOnly;
 
+  // When this canvas is (only) tainted by an image from an extension
+  // content script, allow reads from the same extension afterwards.
+  RefPtr<nsIPrincipal> mExpandedReader;
+
+  // Determines if the caller should be able to read the content.
+  bool CallerCanRead(JSContext* aCx);
+  
   bool IsPrintCallbackDone();
 
   void HandlePrintCallback(nsPresContext::nsPresContextType aType);
 
   nsresult DispatchPrintCallback(nsITimerCallback* aCallback);
 
   void ResetPrintCallback();
 
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..baf8166dae9868aadc93f29ab487a9813fdab311
GIT binary patch
literal 35
kc${<hbhEHbWMp7uXkcVu_|E_YIv@fh!obAj!pL9^09(id-2eap
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..48f97f74bd47cfa51c13c579c8ecb0a01bf2202f
GIT binary patch
literal 35
jc${<hbhEHbWMp7uXkcXc&j12CAOa-9z{KRj$Y2csT*m|5
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_canvas_tainting.js
@@ -0,0 +1,86 @@
+"use strict";
+
+const server = createHttpServer({hosts: ["green.example.com", "red.example.com"]});
+
+server.registerDirectory("/data/", do_get_file("data"));
+
+server.registerPathHandler("/pixel.html", (request, response) => {
+  response.setStatusLine(request.httpVersion, 200, "OK");
+  response.setHeader("Content-Type", "text/html", false);
+  response.write(`<!DOCTYPE html>
+    <script>
+      function readByWeb() {
+        let ctx = document.querySelector("canvas").getContext("2d");
+        let {data} = ctx.getImageData(0, 0, 1, 1);
+        return data.slice(0, 3).join();
+      }
+    </script>
+  `);
+});
+
+add_task(async function test_contentscript_canvas_tainting() {
+  async function contentScript() {
+    let canvas = document.createElement("canvas");
+    let ctx = canvas.getContext("2d");
+    document.body.appendChild(canvas);
+
+    function draw(url) {
+      return new Promise(resolve => {
+        let img = document.createElement("img");
+        img.onload = () => {
+          ctx.drawImage(img, 0, 0, 1, 1);
+          resolve();
+        };
+        img.src = url;
+      });
+    }
+
+    function readByExt() {
+      let {data} = ctx.getImageData(0, 0, 1, 1);
+      return data.slice(0, 3).join();
+    }
+
+    let readByWeb = window.wrappedJSObject.readByWeb;
+
+    // Test reading after drawing an image from the same origin as the web page.
+    await draw("http://green.example.com/data/pixel_green.gif");
+    browser.test.assertEq(readByWeb(), "0,255,0", "Content can read same-origin image");
+    browser.test.assertEq(readByExt(), "0,255,0", "Extension can read same-origin image");
+
+    // Test reading after drawing a blue pixel data URI from extension content script.
+    await draw("data:image/gif;base64,R0lGODlhAQABAIABAAAA/wAAACwAAAAAAQABAAACAkQBADs=");
+    browser.test.assertThrows(readByWeb, /operation is insecure/, "Content can't read extension's image");
+    browser.test.assertEq(readByExt(), "0,0,255", "Extension can read its own image");
+
+    // Test after tainting the canvas with an image from a third party domain.
+    await draw("http://red.example.com/data/pixel_red.gif");
+    browser.test.assertThrows(readByWeb, /operation is insecure/, "Content can't read third party image");
+    browser.test.assertThrows(readByExt, /operation is insecure/, "Extension can't read fully tainted");
+
+    // Test canvas is still fully tainted after drawing extension's data: image again.
+    await draw("data:image/gif;base64,R0lGODlhAQABAIABAAAA/wAAACwAAAAAAQABAAACAkQBADs=");
+    browser.test.assertThrows(readByWeb, /operation is insecure/, "Canvas still fully tainted for content");
+    browser.test.assertThrows(readByExt, /operation is insecure/, "Canvas still fully tainted for extension");
+
+    browser.test.sendMessage("done");
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      content_scripts: [{
+        "matches": ["http://green.example.com/pixel.html"],
+        "js": ["cs.js"],
+      }],
+    },
+    files: {
+      "cs.js": contentScript,
+    },
+  });
+
+  await extension.startup();
+  let contentPage = await ExtensionTestUtils.loadContentPage("http://green.example.com/pixel.html");
+  await extension.awaitMessage("done");
+
+  await contentPage.close();
+  await extension.unload();
+});
--- a/toolkit/components/extensions/test/xpcshell/xpcshell-content.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell-content.ini
@@ -1,13 +1,14 @@
 [test_ext_i18n.js]
 skip-if = os == "android" || (os == "win" && debug) || (os == "linux")
 [test_ext_i18n_css.js]
 [test_ext_contentscript.js]
 [test_ext_contentscript_about_blank_start.js]
+[test_ext_contentscript_canvas_tainting.js]
 [test_ext_contentscript_scriptCreated.js]
 [test_ext_contentscript_triggeringPrincipal.js]
 skip-if = (os == "android" && debug) || (os == "win" && debug) # Windows: Bug 1438796
 [test_ext_contentscript_xrays.js]
 [test_ext_contentScripts_register.js]
 [test_ext_contexts_gc.js]
 [test_ext_adoption_with_xrays.js]
 [test_ext_shadowdom.js]