CVE-2022-29577: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: OWASP AntiSamy before 1.6.7 allows XSS via HTML tag smuggling on STYLE content with crafted input. The output serializer does not properly encode the supposed Cascading Style Sheets (CSS) content. NOTE: this issue exists because of an incomplete fix for CVE-2022-28367.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2022-04-21T23:15Z
Repository: https://github.com/nahsra/antisamy
Language: Java
Files changed: src/main/java/org/owasp/validator/html/scan/AntiSamyDOMScanner.java, src/test/java/org/owasp/validator/html/test/AntiSamyTest.java
Commit date: 2022-04-09 18:47:07 -0300
Commit message: Fix child node removal on style tag processing

Diff: diff --git a/src/main/java/org/owasp/validator/html/scan/AntiSamyDOMScanner.java b/src/main/java/org/owasp/validator/html/scan/AntiSamyDOMScanner.java
index dc991c4..53747cb 100644
--- a/src/main/java/org/owasp/validator/html/scan/AntiSamyDOMScanner.java
+++ b/src/main/java/org/owasp/validator/html/scan/AntiSamyDOMScanner.java
@@ -405,11 +405,12 @@ public class AntiSamyDOMScanner extends AbstractAntiSamyScanner {
          * Invoke the css parser on this element.
          */
         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++) {
                     Node childNode = ele.getChildNodes().item(i);
                     if (toScan.length() > 0) {
@@ -426,33 +427,30 @@ public class AntiSamyDOMScanner extends AbstractAntiSamyScanner {
                  * break all CSS on the page. I wish I was kidding. So,
                  * if after validation no CSS properties are left, we
                  * 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;
         }
         return false;
diff --git a/src/test/java/org/owasp/validator/html/test/AntiSamyTest.java b/src/test/java/org/owasp/validator/html/test/AntiSamyTest.java
index e251acb..d8b9cfd 100644
--- a/src/test/java/org/owasp/validator/html/test/AntiSamyTest.java
+++ b/src/test/java/org/owasp/validator/html/test/AntiSamyTest.java
@@ -1711,14 +1711,18 @@ static final String test33 = "<html>\n"
         // If that happens, they should be treated as text content and not as children nodes.
 
         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)
     public void testMalformedPIScan() {
         // Certain malformed input including a malformed processing instruction may lead the parser to an internal memory error.

Vulnerable code:             if (ele.getChildNodes().getLength() > 0) {

                for (int i = 1; i < ele.getChildNodes().getLength(); i++) {



Fixed code:             int childNodesCount = ele.getChildNodes().getLength();
            if (childNodesCount > 0) {
                 * Starting from the end due to list updating on the fly.
                for (int i = childNodesCount - 1; i >= 1; i--) {
        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")));
        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")));