Bug 1600429 - Add ability to strip conditional CSS rules, only. r=hsivonen
authorKai Engert <kaie@kuix.de>
Tue, 03 Dec 2019 07:04:50 +0000
changeset 2512126 53cfede1065cc3eeb57bb1f2561ac830396e84c7
parent 2512125 e70b2faff1dd8a48a1d22c29f55901b0f50e2712
child 2512127 761985bda5a55b5e356ce9c034f7a1e9de5bfa37
child 2512135 3a845d71869cb4b10f6fb5ddffab87eb9eee03f0
child 2512150 27757932773c52a0f94599d626f8022e351cb7b9
child 2512206 8c282b577a354408e649f844bcbb6ef2d08b7af3
child 2512213 d0fcde12e1874c529c10efbda4cf36947d946a07
child 2512222 7d2e582360270c6c7be279496d647a55fd8e8401
child 2512227 1321e1b97df61f52eb9bf42ccfdb61db6cab5d0b
child 2512235 ab18d3acf49bd2b7e5dfa1742e1e3459589124f8
child 2512236 37dc6c09dc62b7d5e1b8ce8e1614bfdb57f937b3
child 2512245 ad728d5c7f48ed2462ce06da748bc7590df34baa
child 2512255 c84b9c4825b7ad7d1ef399374be784c848b90ff4
child 2512258 f11845dcff9151cdbeeb7dbae1cee8cf1fbcbc80
child 2512262 fa273049953b8e56c944202b846419baaac32e20
child 2512264 f02687061eee203aa704d4e6c7c75851582d0fd9
child 2512280 09f036e9a2ae5c5c3f235a294cddd464049b7481
child 2512342 ee374873f15d7921d522ea89f40a41f7813cdbc5
child 2512348 f890c38d3533bd1bf6271c209752c2b006ce5f63
child 2512353 bd7d60bce48c7e03ff9c2b730eaa7b242cd1bc64
child 2512357 da774e9f720dc59d2dc3fec92a8dd3a7843ddf71
child 2512359 86c0cd1df100a6be8ac580d1ecc1d7904177bf38
child 2512361 8c2527c284a215f31a03fb7636de69191c963ec6
child 2512392 d1001fea6e4c66b98bb4983df49c6e47d2db5ceb
child 2512450 f481ee7a77985c1ae28e296e505c5da0ac210d0c
child 2512452 3adc9a884fbc2fde2413bdfd060a1088c1cdb799
child 2512455 1fdbf19a22091afd354f51bf2cb09e98814bf04c
child 2512459 fbd66cbc02d0a2a82684bac15b864a8f875c69b1
child 2512464 ebfe17e0ed5511af6c4e83e80859be489ab55fda
child 2512470 0b74c012c92c28ec4a8b572a3546c0fd94f8504e
child 2512670 a4c7cb8fbab0c900a86316c16994eb07a56c840d
child 2512730 f573f1578ac5a7aaf7b2be9f2aaff36e91ac06f3
child 2512835 036db80f832f234f9d6ae2416e8d52ae38debdb5
child 2512860 5bf8ada9b7c41a704bb70a099d9244b1c18c4bfe
child 2512918 dd1b4608fd65a529d443114992b30dc4d6e22889
child 2512975 efd938b07919ce920264c4d938e08d371858de31
child 2512977 8fb8bf6b7418eed09a694239f8188f04909d9422
child 2513118 5f8829083bfc6cce8f97b6cda3dfa6ad8c38d2a2
child 2513233 04fb0b3f029f12050af255eca49fa5a2a351a3ba
child 2513592 0af8fa806dcdbe716abbbd4d4b586866a57ffcb0
child 2514163 bf6a7a0b1152ff24e47abfe160aadb5798bab04b
child 2514644 4aa6df5832106cc3c64134069bcca26a81d909f4
child 2514736 1deb3d68ea1348cec0ea1eecd527617b308aba91
child 2514776 08f03c3bd139a433197779219b2babbcbfc76b1d
child 2517134 f603402e1869eb97bd87b496d7a7c45ea9852212
child 2517412 8a83728e9f4385f4bfafb2cabbc81e1d674ee9f3
child 2517414 3598aa0ef92ed3e8e87acd1f067e60bd976d592e
child 2517523 6f6bd961592b2516e785453d7e7c04d7035bef04
child 2519238 25af208589a7b036ed719c58b9c2ec94875d4db4
child 2519333 0ef23741c81a33a6ed1d0d8ed2e017b4a33c9bd1
child 2522290 3c581822e9f25c972ddea8c83ec0d1ba9f7f5fce
child 2522292 d15cf14ba0540c6fc51a2bb22c49e67cdd6ecd94
child 2524390 dd89265fa848827b43a44fceb5ded1a9e35cc02e
child 2525737 e7036c006900a9a5b735e01ca5a509c445438657
child 2525838 e2b9dcee320fbb34c0c7597f1aa4a5acb0fc1085
child 2525840 6180ea281fce602244a83ac1aafed732ba155d0f
child 2525843 ef0cf67504ee27cb633303797f4691752cccdea3
child 2525847 100b66109eb3e97d8d3a923694a678c712e85450
child 2536077 0b31e3b75f72e562a1d67341520ea6a671e439bf
push id459265
push userreviewbot
push dateTue, 03 Dec 2019 16:25:49 +0000
treeherdertry@d52a0c09d105 [default view] [failures only]
reviewershsivonen
bugs1600429
milestone73.0a1
Bug 1600429 - Add ability to strip conditional CSS rules, only. r=hsivonen Differential Revision: https://phabricator.services.mozilla.com/D55350
dom/base/nsTreeSanitizer.cpp
dom/base/nsTreeSanitizer.h
parser/html/nsIParserUtils.idl
--- a/dom/base/nsTreeSanitizer.cpp
+++ b/dom/base/nsTreeSanitizer.cpp
@@ -946,26 +946,34 @@ nsTreeSanitizer::nsTreeSanitizer(uint32_
     : mAllowStyles(aFlags & nsIParserUtils::SanitizerAllowStyle),
       mAllowComments(aFlags & nsIParserUtils::SanitizerAllowComments),
       mDropNonCSSPresentation(aFlags &
                               nsIParserUtils::SanitizerDropNonCSSPresentation),
       mDropForms(aFlags & nsIParserUtils::SanitizerDropForms),
       mCidEmbedsOnly(aFlags & nsIParserUtils::SanitizerCidEmbedsOnly),
       mDropMedia(aFlags & nsIParserUtils::SanitizerDropMedia),
       mFullDocument(false),
-      mLogRemovals(aFlags & nsIParserUtils::SanitizerLogRemovals) {
+      mLogRemovals(aFlags & nsIParserUtils::SanitizerLogRemovals),
+      mOnlyConditionalCSS(aFlags &
+                          nsIParserUtils::SanitizerRemoveOnlyConditionalCSS) {
   if (mCidEmbedsOnly) {
     // Sanitizing styles for external references is not supported.
     mAllowStyles = false;
   }
   if (!sElementsHTML) {
     // Initialize lazily to avoid having to initialize at all if the user
     // doesn't paste HTML or load feeds.
     InitializeStatics();
   }
+  /* Ensure SanitizerRemoveOnlyConditionalCSS isn't combined with any
+   * flags, except SanitizerLogRemovals. */
+  MOZ_ASSERT(!mOnlyConditionalCSS ||
+             0 ==
+                 (aFlags & ~(nsIParserUtils::SanitizerRemoveOnlyConditionalCSS |
+                             nsIParserUtils::SanitizerLogRemovals)));
 }
 
 bool nsTreeSanitizer::MustFlatten(int32_t aNamespace, nsAtom* aLocal) {
   if (aNamespace == kNameSpaceID_XHTML) {
     if (mDropNonCSSPresentation &&
         (nsGkAtoms::font == aLocal || nsGkAtoms::center == aLocal)) {
       return true;
     }
@@ -1097,29 +1105,47 @@ bool nsTreeSanitizer::SanitizeStyleSheet
   err.SuppressException();
   if (!rules) {
     return true;
   }
   uint32_t ruleCount = rules->Length();
   for (uint32_t i = 0; i < ruleCount; ++i) {
     mozilla::css::Rule* rule = rules->Item(i);
     if (!rule) continue;
-    switch (rule->Type()) {
-      default:
-        didSanitize = true;
-        // Ignore these rule types.
-        break;
-      case CSSRule_Binding::STYLE_RULE:
-      case CSSRule_Binding::NAMESPACE_RULE:
-      case CSSRule_Binding::FONT_FACE_RULE: {
-        // Append style, @namespace and @font-face rules verbatim.
-        nsAutoString cssText;
-        rule->GetCssText(cssText);
-        aSanitized.Append(cssText);
-        break;
+    if (!mOnlyConditionalCSS) {
+      switch (rule->Type()) {
+        default:
+          didSanitize = true;
+          // Ignore these rule types.
+          break;
+        case CSSRule_Binding::STYLE_RULE:
+        case CSSRule_Binding::NAMESPACE_RULE:
+        case CSSRule_Binding::FONT_FACE_RULE: {
+          // Append style, @namespace and @font-face rules verbatim.
+          nsAutoString cssText;
+          rule->GetCssText(cssText);
+          aSanitized.Append(cssText);
+          break;
+        }
+      }
+    } else {
+      switch (rule->Type()) {
+        case CSSRule_Binding::MEDIA_RULE:
+        case CSSRule_Binding::SUPPORTS_RULE:
+        case CSSRule_Binding::DOCUMENT_RULE:
+          didSanitize = true;
+          // Ignore (remove) these rule types.
+          break;
+        default: {
+          // Append other rules verbatim.
+          nsAutoString cssText;
+          rule->GetCssText(cssText);
+          aSanitized.Append(cssText);
+          break;
+        }
       }
     }
   }
   if (didSanitize && mLogRemovals) {
     LogMessage("Removed some rules and/or properties from stylesheet.",
                aDocument);
   }
   return didSanitize;
@@ -1320,69 +1346,73 @@ void nsTreeSanitizer::SanitizeChildren(n
   nsIContent* node = aRoot->GetFirstChild();
   while (node) {
     if (node->IsElement()) {
       mozilla::dom::Element* elt = node->AsElement();
       mozilla::dom::NodeInfo* nodeInfo = node->NodeInfo();
       nsAtom* localName = nodeInfo->NameAtom();
       int32_t ns = nodeInfo->NamespaceID();
 
-      if (MustPrune(ns, localName, elt)) {
+      if (!mOnlyConditionalCSS && MustPrune(ns, localName, elt)) {
         if (mLogRemovals) {
           LogMessage("Removing unsafe node.", elt->OwnerDoc(), elt);
         }
         RemoveAllAttributes(elt);
         nsIContent* descendant = node;
         while ((descendant = descendant->GetNextNode(node))) {
           if (descendant->IsElement()) {
             RemoveAllAttributes(descendant->AsElement());
           }
         }
         nsIContent* next = node->GetNextNonChildNode(aRoot);
         node->RemoveFromParent();
         node = next;
         continue;
       }
       if (nsGkAtoms::style == localName) {
+        // If !mOnlyConditionalCSS check the following condition:
         // If styles aren't allowed, style elements got pruned above. Even
         // if styles are allowed, non-HTML, non-SVG style elements got pruned
         // above.
-        NS_ASSERTION(ns == kNameSpaceID_XHTML || ns == kNameSpaceID_SVG,
+        NS_ASSERTION((ns == kNameSpaceID_XHTML || ns == kNameSpaceID_SVG) ||
+                         mOnlyConditionalCSS,
                      "Should have only HTML or SVG here!");
         nsAutoString styleText;
         nsContentUtils::GetNodeTextContent(node, false, styleText);
 
         nsAutoString sanitizedStyle;
         if (SanitizeStyleSheet(styleText, sanitizedStyle, aRoot->OwnerDoc(),
                                node->GetBaseURI())) {
           nsContentUtils::SetNodeTextContent(node, sanitizedStyle, true);
         } else {
           // If the node had non-text child nodes, this operation zaps those.
           // XXXgijs: if we're logging, we should theoretically report about
           // this, but this way of removing those items doesn't allow for that
           // to happen. Seems less likely to be a problem for actual chrome
           // consumers though.
           nsContentUtils::SetNodeTextContent(node, styleText, true);
         }
-        AllowedAttributes allowed;
-        allowed.mStyle = mAllowStyles;
-        if (ns == kNameSpaceID_XHTML) {
-          allowed.mNames = sAttributesHTML;
-          allowed.mURLs = kURLAttributesHTML;
-          SanitizeAttributes(elt, allowed);
-        } else {
-          allowed.mNames = sAttributesSVG;
-          allowed.mURLs = kURLAttributesSVG;
-          allowed.mXLink = true;
-          SanitizeAttributes(elt, allowed);
+        if (!mOnlyConditionalCSS) {
+          AllowedAttributes allowed;
+          allowed.mStyle = mAllowStyles;
+          if (ns == kNameSpaceID_XHTML) {
+            allowed.mNames = sAttributesHTML;
+            allowed.mURLs = kURLAttributesHTML;
+            SanitizeAttributes(elt, allowed);
+          } else {
+            allowed.mNames = sAttributesSVG;
+            allowed.mURLs = kURLAttributesSVG;
+            allowed.mXLink = true;
+            SanitizeAttributes(elt, allowed);
+          }
         }
         node = node->GetNextNonChildNode(aRoot);
         continue;
       }
-      if (MustFlatten(ns, localName)) {
+      if (!mOnlyConditionalCSS && MustFlatten(ns, localName)) {
         if (mLogRemovals) {
           LogMessage("Flattening unsafe node (descendants are preserved).",
                      elt->OwnerDoc(), elt);
         }
         RemoveAllAttributes(elt);
         nsCOMPtr<nsIContent> next = node->GetNextNode(aRoot);
         nsCOMPtr<nsIContent> parent = node->GetParent();
         nsCOMPtr<nsIContent> child;  // Must keep the child alive during move
@@ -1393,44 +1423,47 @@ void nsTreeSanitizer::SanitizeChildren(n
           if (rv.Failed()) {
             break;
           }
         }
         node->RemoveFromParent();
         node = next;
         continue;
       }
-      NS_ASSERTION(ns == kNameSpaceID_XHTML || ns == kNameSpaceID_SVG ||
-                       ns == kNameSpaceID_MathML,
-                   "Should have only HTML, MathML or SVG here!");
-      AllowedAttributes allowed;
-      if (ns == kNameSpaceID_XHTML) {
-        allowed.mNames = sAttributesHTML;
-        allowed.mURLs = kURLAttributesHTML;
-        allowed.mStyle = mAllowStyles;
-        allowed.mDangerousSrc = nsGkAtoms::img == localName && !mCidEmbedsOnly;
-        SanitizeAttributes(elt, allowed);
-      } else if (ns == kNameSpaceID_SVG) {
-        allowed.mNames = sAttributesSVG;
-        allowed.mURLs = kURLAttributesSVG;
-        allowed.mXLink = true;
-        allowed.mStyle = mAllowStyles;
-        SanitizeAttributes(elt, allowed);
-      } else {
-        allowed.mNames = sAttributesMathML;
-        allowed.mURLs = kURLAttributesMathML;
-        allowed.mXLink = true;
-        SanitizeAttributes(elt, allowed);
+      if (!mOnlyConditionalCSS) {
+        NS_ASSERTION(ns == kNameSpaceID_XHTML || ns == kNameSpaceID_SVG ||
+                         ns == kNameSpaceID_MathML,
+                     "Should have only HTML, MathML or SVG here!");
+        AllowedAttributes allowed;
+        if (ns == kNameSpaceID_XHTML) {
+          allowed.mNames = sAttributesHTML;
+          allowed.mURLs = kURLAttributesHTML;
+          allowed.mStyle = mAllowStyles;
+          allowed.mDangerousSrc =
+              nsGkAtoms::img == localName && !mCidEmbedsOnly;
+          SanitizeAttributes(elt, allowed);
+        } else if (ns == kNameSpaceID_SVG) {
+          allowed.mNames = sAttributesSVG;
+          allowed.mURLs = kURLAttributesSVG;
+          allowed.mXLink = true;
+          allowed.mStyle = mAllowStyles;
+          SanitizeAttributes(elt, allowed);
+        } else {
+          allowed.mNames = sAttributesMathML;
+          allowed.mURLs = kURLAttributesMathML;
+          allowed.mXLink = true;
+          SanitizeAttributes(elt, allowed);
+        }
       }
       node = node->GetNextNode(aRoot);
       continue;
     }
     NS_ASSERTION(!node->GetFirstChild(), "How come non-element node had kids?");
     nsIContent* next = node->GetNextNonChildNode(aRoot);
-    if (!mAllowComments && node->IsComment()) {
+    if (!mOnlyConditionalCSS && (!mAllowComments && node->IsComment())) {
       node->RemoveFromParent();
     }
     node = next;
   }
 }
 
 void nsTreeSanitizer::RemoveAllAttributes(Element* aElement) {
   const nsAttrName* attrName;
--- a/dom/base/nsTreeSanitizer.h
+++ b/dom/base/nsTreeSanitizer.h
@@ -78,16 +78,21 @@ class MOZ_STACK_CLASS nsTreeSanitizer {
   bool mFullDocument;
 
   /**
    * Whether we should notify to the console for anything that's stripped.
    */
   bool mLogRemovals;
 
   /**
+   * Whether we should remove CSS conditional rules, no other changes.
+   */
+  bool mOnlyConditionalCSS;
+
+  /**
    * We have various tables of static atoms for elements and attributes.
    */
   class AtomsTable : public nsTHashtable<nsPtrHashKey<const nsStaticAtom>> {
    public:
     explicit AtomsTable(uint32_t aLength)
         : nsTHashtable<nsPtrHashKey<const nsStaticAtom>>(aLength) {}
 
     bool Contains(nsAtom* aAtom) {
--- a/parser/html/nsIParserUtils.idl
+++ b/parser/html/nsIParserUtils.idl
@@ -61,16 +61,23 @@ interface nsIParserUtils : nsISupports
 
   /**
    * Flag for sanitizer: Log messages to the console for everything that gets
    * sanitized
    */
   const unsigned long SanitizerLogRemovals = (1 << 6);
 
   /**
+   * Flag for sanitizer: Only CSS conditional rules will be moved,
+   * nothing else will be changed.
+   * Can be combined with flag SanitizerLogRemovals, only.
+   */
+  const unsigned long SanitizerRemoveOnlyConditionalCSS = (1 << 7);
+
+  /**
    * Parses a string into an HTML document, sanitizes the document and
    * returns the result serialized to a string.
    *
    * The sanitizer is designed to protect against XSS when sanitized content
    * is inserted into a different-origin context without an iframe-equivalent
    * sandboxing mechanism.
    *
    * By default, the sanitizer doesn't try to avoid leaking information that