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 dc991c46..53747cb2 100644 --- 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,7 +429,6 @@ 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; @@ -436,21 +436,19 @@ private boolean processStyleTag(Element ele, Node parentNode) { /* * 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 index e251acb5..d8b9cfdd 100644 --- 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("test", revised, AntiSamy.DOM).getCleanHTML(), not(containsString("javascript"))); assertThat(as.scan("test", revised, AntiSamy.SAX).getCleanHTML(), not(containsString("javascript"))); + assertThat(as.scan("kinput/onfocus=alert(1)>", revised, AntiSamy.DOM).getCleanHTML(), not(containsString("input"))); + assertThat(as.scan("kinput/onfocus=alert(1)>", revised, AntiSamy.SAX).getCleanHTML(), not(containsString("input"))); Policy revised2 = policy.cloneWithDirective(Policy.USE_XHTML,"false"); assertThat(as.scan("Walert(1)", revised2, AntiSamy.DOM).getCleanHTML(), not(containsString("script"))); assertThat(as.scan("Walert(1)", revised2, AntiSamy.SAX).getCleanHTML(), not(containsString("script"))); + assertThat(as.scan("kinput/onfocus=alert(1)>", revised2, AntiSamy.DOM).getCleanHTML(), not(containsString("input"))); + assertThat(as.scan("kinput/onfocus=alert(1)>", revised2, AntiSamy.SAX).getCleanHTML(), not(containsString("input"))); } @Test(timeout = 3000)