From af5fd59293a0b778bada40b9e506945b87558c87 Mon Sep 17 00:00:00 2001 From: "bzbarsky%mit.edu" Date: Fri, 18 Jan 2008 05:23:44 +0000 Subject: [PATCH] Add an internal security-check-less method for adding rules to stylesheets to fix bug 386939. r+sr=dbaron --- layout/base/nsPresShell.cpp | 177 ++++++++++++++++--------------- layout/style/nsCSSStyleSheet.cpp | 20 ++-- layout/style/nsCSSStyleSheet.h | 2 + layout/style/nsICSSStyleSheet.h | 12 ++- 4 files changed, 114 insertions(+), 97 deletions(-) diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index b9ed7f0b704..1cccac8ce94 100644 --- a/layout/base/nsPresShell.cpp +++ b/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 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 uri; @@ -1864,24 +1866,25 @@ nsresult PresShell::CreatePreferenceStyleSheet(void) result = mPrefStyleSheet->SetURIs(uri, nsnull, uri); if (NS_SUCCEEDED(result)) { mPrefStyleSheet->SetComplete(); - nsCOMPtr sheet(do_QueryInterface(mPrefStyleSheet)); - if (sheet) { - PRUint32 index; - result = sheet->InsertRule(NS_LITERAL_STRING("@namespace url(http://www.w3.org/1999/xhtml);"), - 0, &index); - NS_ENSURE_SUCCESS(result, result); + PRUint32 index; + result = + mPrefStyleSheet->InsertRuleInternal(NS_LITERAL_STRING("@namespace url(http://www.w3.org/1999/xhtml);"), + 0, &index); + if (NS_SUCCEEDED(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 sheet(do_QueryInterface(mPrefStyleSheet, &rv)); - NS_ENSURE_SUCCESS(rv, rv); + PRUint32 index = 0; - rv = sheet->InsertRule(NS_LITERAL_STRING("noscript{display:none!important}"), - sInsertPrefSheetRulesAt, &index); + mPrefStyleSheet-> + 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 sheet(do_QueryInterface(mPrefStyleSheet, &rv)); - NS_ENSURE_SUCCESS(rv, rv); - PRBool allowSubframes = PR_TRUE; nsCOMPtr container = mPresContext->GetContainer(); nsCOMPtr 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}"), - sInsertPrefSheetRulesAt, &index); + rv = mPrefStyleSheet-> + 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}"), - sInsertPrefSheetRulesAt, &index); + rv = mPrefStyleSheet-> + 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 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:") + - strColor + ruleClose, - sInsertPrefSheetRulesAt, &index); + rv = mPrefStyleSheet-> + InsertRuleInternal(NS_LITERAL_STRING("*|*:link{color:") + + 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:") + - strColor + ruleClose, - sInsertPrefSheetRulesAt, &index); + rv = mPrefStyleSheet-> + InsertRuleInternal(NS_LITERAL_STRING("*|*:visited{color:") + + 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:") + - strColor + ruleClose, - sInsertPrefSheetRulesAt, &index); + rv = mPrefStyleSheet-> + InsertRuleInternal(NS_LITERAL_STRING("*|*:-moz-any-link:active{color:") + + 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}"), - sInsertPrefSheetRulesAt, &index); + rv = mPrefStyleSheet-> + 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}"), - sInsertPrefSheetRulesAt, &index); + rv = mPrefStyleSheet-> + 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 - nsCOMPtr sheet(do_QueryInterface(mPrefStyleSheet,&result)); - if (NS_SUCCEEDED(result)) { - if (mPresContext->GetUseFocusColors()) { - nscolor focusBackground(mPresContext->FocusBackgroundColor()); - nscolor focusText(mPresContext->FocusTextColor()); + if (mPresContext->GetUseFocusColors()) { + nscolor focusBackground(mPresContext->FocusBackgroundColor()); + nscolor focusText(mPresContext->FocusTextColor()); - // insert a rule to make focus the preferred color - PRUint32 index = 0; - nsAutoString strRule, strColor; + // insert a rule to make focus the preferred color + PRUint32 index = 0; + nsAutoString strRule, strColor; - /////////////////////////////////////////////////////////////// - // - focus: '*:focus - ColorToString(focusText,strColor); - strRule.AppendLiteral("*:focus,*:focus>font {color: "); - strRule.Append(strColor); - strRule.AppendLiteral(" !important; background-color: "); - ColorToString(focusBackground,strColor); - strRule.Append(strColor); - strRule.AppendLiteral(" !important; } "); - // insert the rules - result = sheet->InsertRule(strRule, sInsertPrefSheetRulesAt, &index); - } - PRUint8 focusRingWidth = mPresContext->FocusRingWidth(); - PRBool focusRingOnAnything = mPresContext->GetFocusRingOnAnything(); + /////////////////////////////////////////////////////////////// + // - focus: '*:focus + ColorToString(focusText,strColor); + strRule.AppendLiteral("*:focus,*:focus>font {color: "); + strRule.Append(strColor); + strRule.AppendLiteral(" !important; background-color: "); + ColorToString(focusBackground,strColor); + strRule.Append(strColor); + strRule.AppendLiteral(" !important; } "); + // insert the rules + result = mPrefStyleSheet-> + InsertRuleInternal(strRule, sInsertPrefSheetRulesAt, &index); + } + PRUint8 focusRingWidth = mPresContext->FocusRingWidth(); + PRBool focusRingOnAnything = mPresContext->GetFocusRingOnAnything(); - if ((NS_SUCCEEDED(result) && focusRingWidth != 1 && focusRingWidth <= 4 ) || focusRingOnAnything) { - PRUint32 index = 0; - nsAutoString strRule; - if (!focusRingOnAnything) - 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) + if ((NS_SUCCEEDED(result) && focusRingWidth != 1 && focusRingWidth <= 4 ) || focusRingOnAnything) { + PRUint32 index = 0; + nsAutoString strRule; + if (!focusRingOnAnything) + 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.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 - // insert the rules - result = sheet->InsertRule(strRule, sInsertPrefSheetRulesAt, &index); + strRule.AppendLiteral("px dotted transparent !important; } "); + 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 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.AppendLiteral("input[type=\"button\"]:focus::-moz-focus-inner, input[type=\"submit\"]:focus::-moz-focus-inner {"); - strRule.AppendLiteral("border-color: ButtonText !important; }"); - result = sheet->InsertRule(strRule, sInsertPrefSheetRulesAt, &index); - } + 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("border-color: ButtonText !important; }"); + result = mPrefStyleSheet-> + InsertRuleInternal(strRule, sInsertPrefSheetRulesAt, &index); } } } diff --git a/layout/style/nsCSSStyleSheet.cpp b/layout/style/nsCSSStyleSheet.cpp index 2dc36f89d5a..2158f2b348d 100644 --- a/layout/style/nsCSSStyleSheet.cpp +++ b/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; diff --git a/layout/style/nsCSSStyleSheet.h b/layout/style/nsCSSStyleSheet.h index 9b72f99a6cc..e9b30bab5a9 100644 --- a/layout/style/nsCSSStyleSheet.h +++ b/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, diff --git a/layout/style/nsICSSStyleSheet.h b/layout/style/nsICSSStyleSheet.h index c38b8367a86..529608bbd23 100644 --- a/layout/style/nsICSSStyleSheet.h +++ b/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, \ - { 0xa7, 0xdd, 0xe2, 0xaf, 0xd1, 0xba, 0x7a, 0x07 } } +{ 0x363c1c5f, 0x81ec, 0x4d83, \ + { 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)