author | Henri Sivonen <hsivonen@iki.fi> |
Tue, 02 Aug 2011 20:45:38 +0300 | |
changeset 73679 | 987e42035f38c6df919f0884617ac085842695f9 |
parent 73678 | 8a54bd8aa5c88f9aa6cdacaa088ea90193361c67 |
child 73680 | 78efa76773a68ec89e026e1ad31296205f0e5520 |
push id | 916 |
push user | hsivonen@iki.fi |
push date | Tue, 02 Aug 2011 17:46:35 +0000 |
treeherder | mozilla-inbound@8e4e181a9ce9 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | bzbarsky |
bugs | 675916 |
milestone | 8.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
|
content/base/public/nsTreeSanitizer.h | file | annotate | diff | comparison | revisions | |
content/base/src/nsTreeSanitizer.cpp | file | annotate | diff | comparison | revisions |
--- a/content/base/public/nsTreeSanitizer.h +++ b/content/base/public/nsTreeSanitizer.h @@ -132,20 +132,21 @@ class NS_STACK_CLASS nsTreeSanitizer { /** * Remove the named URL attribute from the element if the URL fails a * security check. * * @param aElement the element whose attribute to possibly modify * @param aNamespace the namespace of the URL attribute * @param aLocalName the local name of the URL attribute + * @return true if the attribute was removed and false otherwise */ - void SanitizeURL(mozilla::dom::Element* aElement, - PRInt32 aNamespace, - nsIAtom* aLocalName); + PRBool SanitizeURL(mozilla::dom::Element* aElement, + PRInt32 aNamespace, + nsIAtom* aLocalName); /** * Checks a style rule for the presence of the 'binding' CSS property and * removes that property from the rule and reserializes in case the * property was found. * * @param aRule The style rule to check * @param aRuleText the serialized mutated rule if the method returns true
--- a/content/base/src/nsTreeSanitizer.cpp +++ b/content/base/src/nsTreeSanitizer.cpp @@ -1222,17 +1222,22 @@ nsTreeSanitizer::SanitizeAttributes(mozi } } continue; } if (aAllowDangerousSrc && nsGkAtoms::src == attrLocal) { continue; } if (IsURL(aURLs, attrLocal)) { - SanitizeURL(aElement, attrNs, attrLocal); + if (SanitizeURL(aElement, attrNs, attrLocal)) { + // in case the attribute removal shuffled the attribute order, start + // the loop again. + --ac; + i = ac; // i will be decremented immediately thanks to the for loop + } continue; } if (aAllowed->GetEntry(attrLocal) && !(attrLocal == nsGkAtoms::rel && aElement->IsHTML(nsGkAtoms::link)) && !(attrLocal == nsGkAtoms::name && aElement->IsHTML(nsGkAtoms::meta))) { // name="" and rel="" are whitelisted, but treat them as blacklisted @@ -1247,26 +1252,36 @@ nsTreeSanitizer::SanitizeAttributes(mozi if (*localStr == '_' || (attrLocal->GetLength() > 5 && localStr[0] == 'd' && localStr[1] == 'a' && localStr[2] == 't' && localStr[3] == 'a' && localStr[4] == '-')) { continue; } // else not allowed } else if (kNameSpaceID_XML == attrNs) { if (nsGkAtoms::base == attrLocal) { - SanitizeURL(aElement, attrNs, attrLocal); + if (SanitizeURL(aElement, attrNs, attrLocal)) { + // in case the attribute removal shuffled the attribute order, start + // the loop again. + --ac; + i = ac; // i will be decremented immediately thanks to the for loop + } continue; } if (nsGkAtoms::lang == attrLocal || nsGkAtoms::space == attrLocal) { continue; } // else not allowed } else if (aAllowXLink && kNameSpaceID_XLink == attrNs) { if (nsGkAtoms::href == attrLocal) { - SanitizeURL(aElement, attrNs, attrLocal); + if (SanitizeURL(aElement, attrNs, attrLocal)) { + // in case the attribute removal shuffled the attribute order, start + // the loop again. + --ac; + i = ac; // i will be decremented immediately thanks to the for loop + } continue; } if (nsGkAtoms::type == attrLocal || nsGkAtoms::title == attrLocal || nsGkAtoms::show == attrLocal || nsGkAtoms::actuate == attrLocal) { continue; } // else not allowed } @@ -1283,17 +1298,17 @@ nsTreeSanitizer::SanitizeAttributes(mozi aElement->IsHTML(nsGkAtoms::audio)) { aElement->SetAttr(kNameSpaceID_None, nsGkAtoms::controls, EmptyString(), PR_FALSE); } } -void +PRBool nsTreeSanitizer::SanitizeURL(mozilla::dom::Element* aElement, PRInt32 aNamespace, nsIAtom* aLocalName) { nsAutoString value; aElement->GetAttr(aNamespace, aLocalName, value); // Get value and remove mandatory quotes @@ -1307,17 +1322,19 @@ nsTreeSanitizer::SanitizeURL(mozilla::do nsCOMPtr<nsIURI> baseURI = aElement->GetBaseURI(); nsCOMPtr<nsIURI> attrURI; nsresult rv = NS_NewURI(getter_AddRefs(attrURI), v, nsnull, baseURI); if (NS_SUCCEEDED(rv)) { rv = secMan->CheckLoadURIWithPrincipal(sNullPrincipal, attrURI, flags); } if (NS_FAILED(rv)) { aElement->UnsetAttr(aNamespace, aLocalName, PR_FALSE); + return PR_TRUE; } + return PR_FALSE; } void nsTreeSanitizer::Sanitize(nsIContent* aFragment) { // If you want to relax these preconditions, be sure to check the code in // here that notifies / does not notify or that fires mutation events if // in tree. NS_PRECONDITION(aFragment->IsNodeOfType(nsINode::eDOCUMENT_FRAGMENT),