HV-1739 Fix CVE-2019-10219 Security issue with @SafeHtml

hibernate · Oct 18, 2019 · 124b7dd · 124b7dd
1 parent 2687d33
commit 124b7dd
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 5 deletions.
diff --git a/...main/java/org/hibernate/validator/internal/constraintvalidators/hv/SafeHtmlValidator.java b/...main/java/org/hibernate/validator/internal/constraintvalidators/hv/SafeHtmlValidator.java
@@ -6,15 +6,15 @@
  */
 package org.hibernate.validator.internal.constraintvalidators.hv;
 
-import java.util.Iterator;
+import java.util.List;
 
 import javax.validation.ConstraintValidator;
 import javax.validation.ConstraintValidatorContext;
 
 import org.hibernate.validator.constraints.SafeHtml;
 import org.jsoup.Jsoup;
 import org.jsoup.nodes.Document;
-import org.jsoup.nodes.Element;
+import org.jsoup.nodes.Node;
 import org.jsoup.parser.Parser;
 import org.jsoup.safety.Cleaner;
 import org.jsoup.safety.Whitelist;
@@ -91,9 +91,9 @@ private Document getFragmentAsDocument(CharSequence value) {
 		Document document = Document.createShell( baseURI );
 
 		// add the fragment's nodes to the body of resulting document
-		Iterator<Element> nodes = fragment.children().iterator();
-		while ( nodes.hasNext() ) {
-			document.body().appendChild( nodes.next() );
+		List<Node> childNodes = fragment.childNodes();
+		for ( Node node : childNodes ) {
+			document.body().appendChild( node.clone() );
 		}
 
 		return document;

diff --git a/.../org/hibernate/validator/test/internal/constraintvalidators/hv/SafeHtmlValidatorTest.java b/.../org/hibernate/validator/test/internal/constraintvalidators/hv/SafeHtmlValidatorTest.java
@@ -57,6 +57,44 @@ public void testInvalidScriptTagIncluded() throws Exception {
 		assertFalse( getSafeHtmlValidator().isValid( "Hello<script>alert('Doh')</script>World !", null ) );
 	}
 
+	@Test
+	// A "downlevel revealed" conditional 'comment' is not an (X)HTML comment at all,
+	// despite the misleading name, it is default Microsoft syntax.
+	// The tag is unrecognized by therefore executed
+	public void testDownlevelRevealedConditionalComment() throws Exception {
+		descriptorBuilder.setAttribute( "whitelistType", WhiteListType.BASIC );
+
+		assertFalse( getSafeHtmlValidator().isValid( "<![if !IE]>\n<SCRIPT>alert{'XSS'};</SCRIPT>\n<![endif]>", null ) );
+	}
+
+	@Test
+	public void testDownlevelHiddenConditionalComment() throws Exception {
+		descriptorBuilder.setAttribute( "whitelistType", WhiteListType.BASIC );
+
+		assertFalse( getSafeHtmlValidator().isValid( "<!--[if gte IE 4]>\n<SCRIPT>alert{'XSS'};</SCRIPT>\n<![endif]-->", null ) );
+	}
+
+	@Test
+	public void testSimpleComment() throws Exception {
+		descriptorBuilder.setAttribute( "whitelistType", WhiteListType.BASIC );
+
+		assertFalse( getSafeHtmlValidator().isValid( "<!-- Just a comment -->", null ) );
+	}
+
+	@Test
+	public void testServerSideIncludesSSI() throws Exception {
+		descriptorBuilder.setAttribute( "whitelistType", WhiteListType.BASIC );
+
+		assertFalse( getSafeHtmlValidator().isValid( "<? echo{'<SCR}'; echo{'IPT>alert{\"XSS\"}</SCRIPT>'}; ?>", null ) );
+	}
+
+	@Test
+	public void testPHPScript() throws Exception {
+		descriptorBuilder.setAttribute( "whitelistType", WhiteListType.BASIC );
+
+		assertFalse( getSafeHtmlValidator().isValid( "<? echo{'<SCR}'; echo{'IPT>alert{\"XSS\"}</SCRIPT>'}; ?>", null ) );
+	}
+
 	@Test
 	public void testInvalidIncompleteImgTagWithScriptIncluded() {
 		descriptorBuilder.setAttribute( "whitelistType", WhiteListType.BASIC );