Add an internal security-check-less method for adding rules to stylesheets to

fix bug 386939.  r+sr=dbaron

layout/base/nsPresShell.cpp

@@ -1108,7 +1108,8 @@ protected:
   nsresult DoFlushPendingNotifications(mozFlushType aType,
                                        PRBool aInterruptibleReflow);
 
-  nsICSSStyleSheet*         mPrefStyleSheet; // mStyleSet owns it but we maintain a ref, may be null
+  nsCOMPtr<nsICSSStyleSheet> mPrefStyleSheet; // mStyleSet owns it but we
+                                              // maintain a ref, may be null
 #ifdef DEBUG
   PRUint32                  mUpdateCount;
 #endif
@@ -1845,7 +1846,7 @@ nsresult PresShell::ClearPreferenceStyleRules(void)
       printf("PrefStyleSheet removed\n");
 #endif
       // clear the sheet pointer: it is strictly historical now
-      NS_RELEASE(mPrefStyleSheet);
+      mPrefStyleSheet = nsnull;
     }
   }
   return result;
@@ -1854,7 +1855,8 @@ nsresult PresShell::ClearPreferenceStyleRules(void)
 nsresult PresShell::CreatePreferenceStyleSheet(void)
 {
   NS_ASSERTION(!mPrefStyleSheet, "prefStyleSheet already exists");
-  nsresult result = CallCreateInstance(kCSSStyleSheetCID, &mPrefStyleSheet);
+  nsresult result;
+  mPrefStyleSheet = do_CreateInstance(kCSSStyleSheetCID, &result);
   if (NS_SUCCEEDED(result)) {
     NS_ASSERTION(mPrefStyleSheet, "null but no error");
     nsCOMPtr<nsIURI> uri;
@@ -1864,24 +1866,25 @@ nsresult PresShell::CreatePreferenceStyleSheet(void)
       result = mPrefStyleSheet->SetURIs(uri, nsnull, uri);
       if (NS_SUCCEEDED(result)) {
         mPrefStyleSheet->SetComplete();
-        nsCOMPtr<nsIDOMCSSStyleSheet> sheet(do_QueryInterface(mPrefStyleSheet));
+        PRUint32 index;
-        if (sheet) {
+        result =
-          PRUint32 index;
+          mPrefStyleSheet->InsertRuleInternal(NS_LITERAL_STRING("@namespace url(http://www.w3.org/1999/xhtml);"),
-          result = sheet->InsertRule(NS_LITERAL_STRING("@namespace url(http://www.w3.org/1999/xhtml);"),
+                                              0, &index);
-                                     0, &index);
+        if (NS_SUCCEEDED(result)) {
-          NS_ENSURE_SUCCESS(result, result);
+          mStyleSet->AppendStyleSheet(nsStyleSet::eUserSheet, mPrefStyleSheet);
         }
-        mStyleSet->AppendStyleSheet(nsStyleSet::eUserSheet, mPrefStyleSheet);
       }
     }
-  } else {
-    result = NS_ERROR_OUT_OF_MEMORY;
   }
 
 #ifdef DEBUG_attinasi
   printf("CreatePrefStyleSheet completed: error=%ld\n",(long)result);
 #endif
 
+  if (NS_FAILED(result)) {
+    mPrefStyleSheet = nsnull;
+  }
+
   return result;
 }
 
@@ -1908,12 +1911,11 @@ PresShell::SetPrefNoScriptRule()
       rv = CreatePreferenceStyleSheet();
       NS_ENSURE_SUCCESS(rv, rv);
     }
-    // get the DOM interface to the stylesheet
+
-    nsCOMPtr<nsIDOMCSSStyleSheet> sheet(do_QueryInterface(mPrefStyleSheet, &rv));
-    NS_ENSURE_SUCCESS(rv, rv);
     PRUint32 index = 0;
-    rv = sheet->InsertRule(NS_LITERAL_STRING("noscript{display:none!important}"),
+    mPrefStyleSheet->
-                           sInsertPrefSheetRulesAt, &index);
+      InsertRuleInternal(NS_LITERAL_STRING("noscript{display:none!important}"),
+                         sInsertPrefSheetRulesAt, &index);
   }
 
   return rv;
@@ -1935,10 +1937,6 @@ nsresult PresShell::SetPrefNoFramesRule(void)
   
   NS_ASSERTION(mPrefStyleSheet, "prefstylesheet should not be null");
   
-  // get the DOM interface to the stylesheet
-  nsCOMPtr<nsIDOMCSSStyleSheet> sheet(do_QueryInterface(mPrefStyleSheet, &rv));
-  NS_ENSURE_SUCCESS(rv, rv);
-
   PRBool allowSubframes = PR_TRUE;
   nsCOMPtr<nsISupports> container = mPresContext->GetContainer();     
   nsCOMPtr<nsIDocShell> docShell(do_QueryInterface(container));
@@ -1947,11 +1945,13 @@ nsresult PresShell::SetPrefNoFramesRule(void)
   }
   if (!allowSubframes) {
     PRUint32 index = 0;
-    rv = sheet->InsertRule(NS_LITERAL_STRING("noframes{display:block}"),
+    rv = mPrefStyleSheet->
-                           sInsertPrefSheetRulesAt, &index);
+      InsertRuleInternal(NS_LITERAL_STRING("noframes{display:block}"),
+                         sInsertPrefSheetRulesAt, &index);
     NS_ENSURE_SUCCESS(rv, rv);
-    rv = sheet->InsertRule(NS_LITERAL_STRING("frame, frameset, iframe {display:none!important}"),
+    rv = mPrefStyleSheet->
-                           sInsertPrefSheetRulesAt, &index);
+      InsertRuleInternal(NS_LITERAL_STRING("frame, frameset, iframe {display:none!important}"),
+                         sInsertPrefSheetRulesAt, &index);
   }
   return rv;
 }
@@ -1972,10 +1972,6 @@ nsresult PresShell::SetPrefLinkRules(void)
   
   NS_ASSERTION(mPrefStyleSheet, "prefstylesheet should not be null");
   
-  // get the DOM interface to the stylesheet
-  nsCOMPtr<nsIDOMCSSStyleSheet> sheet(do_QueryInterface(mPrefStyleSheet, &rv));
-  NS_ENSURE_SUCCESS(rv, rv);
-  
   // support default link colors: 
   //   this means the link colors need to be overridable, 
   //   which they are if we put them in the agent stylesheet,
@@ -1992,23 +1988,26 @@ nsresult PresShell::SetPrefLinkRules(void)
 
   // insert a rule to color links: '*|*:link {color: #RRGGBB [!important];}'
   ColorToString(linkColor, strColor);
-  rv = sheet->InsertRule(NS_LITERAL_STRING("*|*:link{color:") +
+  rv = mPrefStyleSheet->
-                         strColor + ruleClose,
+    InsertRuleInternal(NS_LITERAL_STRING("*|*:link{color:") +
-                         sInsertPrefSheetRulesAt, &index);
+                       strColor + ruleClose,
+                       sInsertPrefSheetRulesAt, &index);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // - visited links: '*|*:visited {color: #RRGGBB [!important];}'
   ColorToString(visitedColor, strColor);
-  rv = sheet->InsertRule(NS_LITERAL_STRING("*|*:visited{color:") +
+  rv = mPrefStyleSheet->
-                         strColor + ruleClose,
+    InsertRuleInternal(NS_LITERAL_STRING("*|*:visited{color:") +
-                         sInsertPrefSheetRulesAt, &index);
+                       strColor + ruleClose,
+                       sInsertPrefSheetRulesAt, &index);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // - active links: '*|*:-moz-any-link:active {color: #RRGGBB [!important];}'
   ColorToString(activeColor, strColor);
-  rv = sheet->InsertRule(NS_LITERAL_STRING("*|*:-moz-any-link:active{color:") +
+  rv = mPrefStyleSheet->
-                         strColor + ruleClose,
+    InsertRuleInternal(NS_LITERAL_STRING("*|*:-moz-any-link:active{color:") +
-                         sInsertPrefSheetRulesAt, &index);
+                       strColor + ruleClose,
+                       sInsertPrefSheetRulesAt, &index);
   NS_ENSURE_SUCCESS(rv, rv);
 
   PRBool underlineLinks =
@@ -2020,11 +2019,13 @@ nsresult PresShell::SetPrefLinkRules(void)
     // no need for important, we want these to be overridable
     // NOTE: these must go in the agent stylesheet or they cannot be
     //       overridden by authors
-    rv = sheet->InsertRule(NS_LITERAL_STRING("*|*:-moz-any-link{text-decoration:underline}"),
+    rv = mPrefStyleSheet->
-                           sInsertPrefSheetRulesAt, &index);
+      InsertRuleInternal(NS_LITERAL_STRING("*|*:-moz-any-link{text-decoration:underline}"),
+                         sInsertPrefSheetRulesAt, &index);
   } else {
-    rv = sheet->InsertRule(NS_LITERAL_STRING("*|*:-moz-any-link{text-decoration:none}"),
+    rv = mPrefStyleSheet->
-                           sInsertPrefSheetRulesAt, &index);
+      InsertRuleInternal(NS_LITERAL_STRING("*|*:-moz-any-link{text-decoration:none}"),
+                         sInsertPrefSheetRulesAt, &index);
   }
 
   return rv;          
@@ -2044,58 +2045,58 @@ nsresult PresShell::SetPrefFocusRules(void)
   if (NS_SUCCEEDED(result)) {
     NS_ASSERTION(mPrefStyleSheet, "prefstylesheet should not be null");
 
-    // get the DOM interface to the stylesheet
+    if (mPresContext->GetUseFocusColors()) {
-    nsCOMPtr<nsIDOMCSSStyleSheet> sheet(do_QueryInterface(mPrefStyleSheet,&result));
+      nscolor focusBackground(mPresContext->FocusBackgroundColor());
-    if (NS_SUCCEEDED(result)) {
+      nscolor focusText(mPresContext->FocusTextColor());
-      if (mPresContext->GetUseFocusColors()) {
-        nscolor focusBackground(mPresContext->FocusBackgroundColor());
-        nscolor focusText(mPresContext->FocusTextColor());
 
-        // insert a rule to make focus the preferred color
+      // insert a rule to make focus the preferred color
-        PRUint32 index = 0;
+      PRUint32 index = 0;
-        nsAutoString strRule, strColor;
+      nsAutoString strRule, strColor;
 
-        ///////////////////////////////////////////////////////////////
+      ///////////////////////////////////////////////////////////////
-        // - focus: '*:focus
+      // - focus: '*:focus
-        ColorToString(focusText,strColor);
+      ColorToString(focusText,strColor);
-        strRule.AppendLiteral("*:focus,*:focus>font {color: ");
+      strRule.AppendLiteral("*:focus,*:focus>font {color: ");
-        strRule.Append(strColor);
+      strRule.Append(strColor);
-        strRule.AppendLiteral(" !important; background-color: ");
+      strRule.AppendLiteral(" !important; background-color: ");
-        ColorToString(focusBackground,strColor);
+      ColorToString(focusBackground,strColor);
-        strRule.Append(strColor);
+      strRule.Append(strColor);
-        strRule.AppendLiteral(" !important; } ");
+      strRule.AppendLiteral(" !important; } ");
-        // insert the rules
+      // insert the rules
-        result = sheet->InsertRule(strRule, sInsertPrefSheetRulesAt, &index);
+      result = mPrefStyleSheet->
-      }
+        InsertRuleInternal(strRule, sInsertPrefSheetRulesAt, &index);
-      PRUint8 focusRingWidth = mPresContext->FocusRingWidth();
+    }
-      PRBool focusRingOnAnything = mPresContext->GetFocusRingOnAnything();
+    PRUint8 focusRingWidth = mPresContext->FocusRingWidth();
+    PRBool focusRingOnAnything = mPresContext->GetFocusRingOnAnything();
 
-      if ((NS_SUCCEEDED(result) && focusRingWidth != 1 && focusRingWidth <= 4 ) || focusRingOnAnything) {
+    if ((NS_SUCCEEDED(result) && focusRingWidth != 1 && focusRingWidth <= 4 ) || focusRingOnAnything) {
-        PRUint32 index = 0;
+      PRUint32 index = 0;
-        nsAutoString strRule;
+      nsAutoString strRule;
-        if (!focusRingOnAnything)
+      if (!focusRingOnAnything)
-          strRule.AppendLiteral("*|*:link:focus, *|*:visited");    // If we only want focus rings on the normal things like links
+        strRule.AppendLiteral("*|*:link:focus, *|*:visited");    // If we only want focus rings on the normal things like links
-        strRule.AppendLiteral(":focus {outline: ");     // For example 3px dotted WindowText (maximum 4)
+      strRule.AppendLiteral(":focus {outline: ");     // For example 3px dotted WindowText (maximum 4)
+      strRule.AppendInt(focusRingWidth);
+      strRule.AppendLiteral("px dotted WindowText !important; } ");     // For example 3px dotted WindowText
+      // insert the rules
+      result = mPrefStyleSheet->
+        InsertRuleInternal(strRule, sInsertPrefSheetRulesAt, &index);
+      NS_ENSURE_SUCCESS(result, result);
+      if (focusRingWidth != 1) {
+        // If the focus ring width is different from the default, fix buttons with rings
+        strRule.AssignLiteral("button::-moz-focus-inner, input[type=\"reset\"]::-moz-focus-inner,");
+        strRule.AppendLiteral("input[type=\"button\"]::-moz-focus-inner, ");
+        strRule.AppendLiteral("input[type=\"submit\"]::-moz-focus-inner { padding: 1px 2px 1px 2px; border: ");
         strRule.AppendInt(focusRingWidth);
-        strRule.AppendLiteral("px dotted WindowText !important; } ");     // For example 3px dotted WindowText
+        strRule.AppendLiteral("px dotted transparent !important; } ");
-        // insert the rules
+        result = mPrefStyleSheet->
-        result = sheet->InsertRule(strRule, sInsertPrefSheetRulesAt, &index);
+          InsertRuleInternal(strRule, sInsertPrefSheetRulesAt, &index);
         NS_ENSURE_SUCCESS(result, result);
-        if (focusRingWidth != 1) {
-          // If the focus ring width is different from the default, fix buttons with rings
-          strRule.AssignLiteral("button::-moz-focus-inner, input[type=\"reset\"]::-moz-focus-inner,");
-          strRule.AppendLiteral("input[type=\"button\"]::-moz-focus-inner, ");
-          strRule.AppendLiteral("input[type=\"submit\"]::-moz-focus-inner { padding: 1px 2px 1px 2px; border: ");
-          strRule.AppendInt(focusRingWidth);
-          strRule.AppendLiteral("px dotted transparent !important; } ");
-          result = sheet->InsertRule(strRule, sInsertPrefSheetRulesAt, &index);
-          NS_ENSURE_SUCCESS(result, result);
           
-          strRule.AssignLiteral("button:focus::-moz-focus-inner, input[type=\"reset\"]:focus::-moz-focus-inner,");
+        strRule.AssignLiteral("button:focus::-moz-focus-inner, input[type=\"reset\"]:focus::-moz-focus-inner,");
-          strRule.AppendLiteral("input[type=\"button\"]:focus::-moz-focus-inner, input[type=\"submit\"]:focus::-moz-focus-inner {");
+        strRule.AppendLiteral("input[type=\"button\"]:focus::-moz-focus-inner, input[type=\"submit\"]:focus::-moz-focus-inner {");
-          strRule.AppendLiteral("border-color: ButtonText !important; }");
+        strRule.AppendLiteral("border-color: ButtonText !important; }");
-          result = sheet->InsertRule(strRule, sInsertPrefSheetRulesAt, &index);
+        result = mPrefStyleSheet->
-        }
+          InsertRuleInternal(strRule, sInsertPrefSheetRulesAt, &index);
       }
     }
   }

layout/style/nsCSSStyleSheet.cpp

@@ -1334,7 +1334,7 @@ nsCSSStyleSheet::SubjectSubsumesInnerPrincipal() const
   securityManager->GetSubjectPrincipal(getter_AddRefs(subjectPrincipal));
 
   if (!subjectPrincipal) {
-    return NS_OK;
+    return NS_ERROR_DOM_SECURITY_ERR;
   }
 
   PRBool subsumes;
@@ -1506,6 +1506,19 @@ NS_IMETHODIMP
 nsCSSStyleSheet::InsertRule(const nsAString& aRule, 
                             PRUint32 aIndex, 
                             PRUint32* aReturn)
+{
+  //-- Security check: Only scripts whose principal subsumes that of the
+  //   style sheet can modify rule collections.
+  nsresult rv = SubjectSubsumesInnerPrincipal();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return InsertRuleInternal(aRule, aIndex, aReturn);
+}
+
+NS_IMETHODIMP
+nsCSSStyleSheet::InsertRuleInternal(const nsAString& aRule, 
+                                    PRUint32 aIndex, 
+                                    PRUint32* aReturn)
 {
   // No doing this if the sheet is not complete!
   PRBool complete;
@@ -1514,11 +1527,6 @@ nsCSSStyleSheet::InsertRule(const nsAString& aRule,
     return NS_ERROR_DOM_INVALID_ACCESS_ERR;
   }
 
-  //-- Security check: Only scripts whose principal subsumes that of the
-  //   style sheet can modify rule collections.
-  nsresult rv = SubjectSubsumesInnerPrincipal();
-  NS_ENSURE_SUCCESS(rv, rv);
-
   if (aRule.IsEmpty()) {
     // Nothing to do here
     return NS_OK;

layout/style/nsCSSStyleSheet.h

@@ -156,6 +156,8 @@ public:
   NS_IMETHOD SetModified(PRBool aModified);
   NS_IMETHOD AddRuleProcessor(nsCSSRuleProcessor* aProcessor);
   NS_IMETHOD DropRuleProcessor(nsCSSRuleProcessor* aProcessor);
+  NS_IMETHOD InsertRuleInternal(const nsAString& aRule,
+                                PRUint32 aIndex, PRUint32* aReturn);  
 
   // nsICSSLoaderObserver interface
   NS_IMETHOD StyleSheetLoaded(nsICSSStyleSheet* aSheet, PRBool aWasAlternate,

layout/style/nsICSSStyleSheet.h

@@ -53,10 +53,10 @@ class nsICSSImportRule;
 class nsIPrincipal;
 
 // IID for the nsICSSStyleSheet interface
-// 74fa10f3-fab7-425a-a7dd-e2afd1ba7a07
+// 363c1c5f-81ec-4d83-ad8a-b48d48f1398d
 #define NS_ICSS_STYLE_SHEET_IID     \
-{ 0x74fa10f3, 0xfab7, 0x425a, \
+{ 0x363c1c5f, 0x81ec, 0x4d83, \
- { 0xa7, 0xdd, 0xe2, 0xaf, 0xd1, 0xba, 0x7a, 0x07 } }
+ { 0xad, 0x8a, 0xb4, 0x8d, 0x48, 0xf1, 0x39, 0x8d } }
 
 class nsICSSStyleSheet : public nsIStyleSheet {
 public:
@@ -121,6 +121,12 @@ public:
 
   NS_IMETHOD  AddRuleProcessor(nsCSSRuleProcessor* aProcessor) = 0;
   NS_IMETHOD  DropRuleProcessor(nsCSSRuleProcessor* aProcessor) = 0;
+
+  /**
+   * Like the DOM insertRule() method, but doesn't do any security checks
+   */
+  NS_IMETHOD InsertRuleInternal(const nsAString& aRule,
+                                PRUint32 aIndex, PRUint32* aReturn) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsICSSStyleSheet, NS_ICSS_STYLE_SHEET_IID)