Merge pull request #168 from nahsra/1.6.7

Merge all changes for 1.6.7 into main
nahsra · Apr 10, 2022 · a42865c · a42865c
2 parents b6e76de + a0ec25d
commit a42865c
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 8 deletions.
diff --git a/pom.xml b/pom.xml
@@ -3,7 +3,7 @@
     <groupId>org.owasp.antisamy</groupId>
     <artifactId>antisamy</artifactId>
     <packaging>jar</packaging>
-    <version>1.6.6.1</version>
+    <version>1.6.7</version>
     <distributionManagement>
         <snapshotRepository>
             <id>ossrh</id>
@@ -43,7 +43,7 @@
     <properties>
         <gpg.skip>true</gpg.skip><!-- by default skip gpg -->
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-        <project.build.outputTimestamp>2022-04-07T19:23:45Z</project.build.outputTimestamp>
+        <project.build.outputTimestamp>2022-04-10T18:12:34Z</project.build.outputTimestamp>
         <project.java.target>1.7</project.java.target>
         <version.findsecbugs>1.12.0</version.findsecbugs>
         <version.io>2.11.0</version.io>

diff --git a/src/main/java/org/owasp/validator/html/scan/AntiSamyDOMScanner.java b/src/main/java/org/owasp/validator/html/scan/AntiSamyDOMScanner.java
@@ -407,7 +407,8 @@ private boolean processStyleTag(Element ele, Node parentNode) {
         CssScanner styleScanner = new CssScanner(policy, messages, policy.isEmbedStyleSheets());
 
         try {
-            if (ele.getChildNodes().getLength() > 0) {
+            int childNodesCount = ele.getChildNodes().getLength();
+            if (childNodesCount > 0) {
                 StringBuffer toScan = new StringBuffer();
 
                 for (int i = 0; i < ele.getChildNodes().getLength(); i++) {
@@ -428,29 +429,26 @@ private boolean processStyleTag(Element ele, Node parentNode) {
                  * would normally be left with an empty style tag and
                  * break all CSS. To prevent that, we have this check.
                  */
-
                 String cleanHTML = cr.getCleanHTML();
                 cleanHTML = cleanHTML == null || cleanHTML.equals("") ? "/* */" : cleanHTML;
 
                 ele.getFirstChild().setNodeValue(cleanHTML);
                 /*
                  * Remove every other node after cleaning CSS, there will
                  * be only one node in the end, as it always should have.
+                 * Starting from the end due to list updating on the fly.
                  */
-                for (int i = 1; i < ele.getChildNodes().getLength(); i++) {
+                for (int i = childNodesCount - 1; i >= 1; i--) {
                     Node childNode = ele.getChildNodes().item(i);
                     ele.removeChild(childNode);
                 }
             }
-
         } catch (DOMException | ScanException | ParseException | NumberFormatException e) {
-
             /*
              * ParseException shouldn't be possible anymore, but we'll leave it
              * here because I (Arshan) am hilariously dumb sometimes.
              * Batik can throw NumberFormatExceptions (see bug #48).
              */
-
             addError(ErrorMessageUtil.ERROR_CSS_TAG_MALFORMED, new Object[]{HTMLEntityEncoder.htmlEntityEncode(ele.getFirstChild().getNodeValue())});
             parentNode.removeChild(ele);
             return true;

diff --git a/src/test/java/org/owasp/validator/html/test/AntiSamyTest.java b/src/test/java/org/owasp/validator/html/test/AntiSamyTest.java
@@ -1713,10 +1713,14 @@ public void testSmuggledTagsInStyleContent() throws ScanException, PolicyExcepti
         Policy revised = policy.cloneWithDirective(Policy.USE_XHTML,"true");
         assertThat(as.scan("<style/>b<![cdata[</style><a href=javascript:alert(1)>test", revised, AntiSamy.DOM).getCleanHTML(), not(containsString("javascript")));
         assertThat(as.scan("<style/>b<![cdata[</style><a href=javascript:alert(1)>test", revised, AntiSamy.SAX).getCleanHTML(), not(containsString("javascript")));
+        assertThat(as.scan("<select<style/>k<input<</>input/onfocus=alert(1)>", revised, AntiSamy.DOM).getCleanHTML(), not(containsString("input")));
+        assertThat(as.scan("<select<style/>k<input<</>input/onfocus=alert(1)>", revised, AntiSamy.SAX).getCleanHTML(), not(containsString("input")));
 
         Policy revised2 = policy.cloneWithDirective(Policy.USE_XHTML,"false");
         assertThat(as.scan("<select<style/>W<xmp<script>alert(1)</script>", revised2, AntiSamy.DOM).getCleanHTML(), not(containsString("script")));
         assertThat(as.scan("<select<style/>W<xmp<script>alert(1)</script>", revised2, AntiSamy.SAX).getCleanHTML(), not(containsString("script")));
+        assertThat(as.scan("<select<style/>k<input<</>input/onfocus=alert(1)>", revised2, AntiSamy.DOM).getCleanHTML(), not(containsString("input")));
+        assertThat(as.scan("<select<style/>k<input<</>input/onfocus=alert(1)>", revised2, AntiSamy.SAX).getCleanHTML(), not(containsString("input")));
     }
 
     @Test(timeout = 3000)