From 124b7dd6d9a4ad24d4d49f74701f05a13e56ceee Mon Sep 17 00:00:00 2001
From: Davide D'Alto <davide@hibernate.org>
Date: Fri, 18 Oct 2019 16:45:20 +0200
Subject: [PATCH] HV-1739 Fix CVE-2019-10219 Security issue with @SafeHtml

---
 .../hv/SafeHtmlValidator.java                 | 10 ++---
 .../hv/SafeHtmlValidatorTest.java             | 38 +++++++++++++++++++
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/engine/src/main/java/org/hibernate/validator/internal/constraintvalidators/hv/SafeHtmlValidator.java b/engine/src/main/java/org/hibernate/validator/internal/constraintvalidators/hv/SafeHtmlValidator.java
index 7fba356..26e4361 100644
--- a/engine/src/main/java/org/hibernate/validator/internal/constraintvalidators/hv/SafeHtmlValidator.java
+++ b/engine/src/main/java/org/hibernate/validator/internal/constraintvalidators/hv/SafeHtmlValidator.java
@@ -6,13 +6,13 @@
  */
 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.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;
@@ -76,9 +76,9 @@ private Document getFragmentAsDocument(CharSequence value) {
 		Document document = Document.createShell( "" );
 
 		// 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/engine/src/test/java/org/hibernate/validator/test/internal/constraintvalidators/hv/SafeHtmlValidatorTest.java b/engine/src/test/java/org/hibernate/validator/test/internal/constraintvalidators/hv/SafeHtmlValidatorTest.java
index 65a1f8a..c45aad3 100644
--- a/engine/src/test/java/org/hibernate/validator/test/internal/constraintvalidators/hv/SafeHtmlValidatorTest.java
+++ b/engine/src/test/java/org/hibernate/validator/test/internal/constraintvalidators/hv/SafeHtmlValidatorTest.java
@@ -54,6 +54,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() {
 		descriptor.setValue( "whitelistType", WhiteListType.BASIC );
-- 
2.23.0