Bug 675916 - Restart iteration over attributes in the sanitizer when URL check ends up removing an attribute. r=bzbarsky.

content/base/public/nsTreeSanitizer.h

@@ -137,10 +137,11 @@ class NS_STACK_CLASS nsTreeSanitizer {
      * @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

content/base/src/nsTreeSanitizer.cpp

@@ -1227,7 +1227,12 @@ nsTreeSanitizer::SanitizeAttributes(mozilla::dom::Element* aElement,
         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) &&
@@ -1252,7 +1257,12 @@ nsTreeSanitizer::SanitizeAttributes(mozilla::dom::Element* aElement,
       // 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) {
@@ -1261,7 +1271,12 @@ nsTreeSanitizer::SanitizeAttributes(mozilla::dom::Element* aElement,
       // 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
@@ -1288,7 +1303,7 @@ nsTreeSanitizer::SanitizeAttributes(mozilla::dom::Element* aElement,
   }
 }
 
-void
+PRBool
 nsTreeSanitizer::SanitizeURL(mozilla::dom::Element* aElement,
                              PRInt32 aNamespace,
                              nsIAtom* aLocalName)
@@ -1312,7 +1327,9 @@ nsTreeSanitizer::SanitizeURL(mozilla::dom::Element* aElement,
   }
   if (NS_FAILED(rv)) {
     aElement->UnsetAttr(aNamespace, aLocalName, PR_FALSE);
+    return PR_TRUE;
   }
+  return PR_FALSE;
 }
 
 void