xml validation exception introduced fixes #31

Author: bgalek

build.gradle.kts

@@ -14,7 +14,7 @@ repositories {
 }
 
 dependencies {
-    implementation("com.googlecode.owasp-java-html-sanitizer:owasp-java-html-sanitizer:1.1")
+    implementation("com.googlecode.owasp-java-html-sanitizer:owasp-java-html-sanitizer:20220608.1")
     testImplementation("org.junit.jupiter:junit-jupiter:5.9.1")
 }
 

src/main/java/com/github/bgalek/security/svg/InvalidXMLSyntaxException.java

@@ -0,0 +1,7 @@
+package com.github.bgalek.security.svg;
+
+public class InvalidXMLSyntaxException extends RuntimeException {
+    public InvalidXMLSyntaxException(Throwable cause) {
+        super(cause);
+    }
+}

src/main/java/com/github/bgalek/security/svg/SvgSecurityValidator.java

@@ -1,12 +1,11 @@
 package com.github.bgalek.security.svg;
 
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import org.owasp.html.CssSchema;
 import org.owasp.html.HtmlChangeListener;
 import org.owasp.html.HtmlPolicyBuilder;
 import org.owasp.html.PolicyFactory;
 
+import javax.xml.parsers.DocumentBuilder;
+import java.io.ByteArrayInputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
@@ -29,6 +28,7 @@ public class SvgSecurityValidator implements XssDetector {
 
     private final String[] svgElements;
     private final String[] svgAttributes;
+    private final DocumentBuilder xmlParser;
 
     /**
      * Use builder SvgSecurityValidator.builder()
@@ -37,11 +37,13 @@ public class SvgSecurityValidator implements XssDetector {
     public SvgSecurityValidator() {
         this.svgElements = SvgElements.DEFAULT_SVG_ELEMENTS;
         this.svgAttributes = SvgAttributes.DEFAULT_SVG_ATTRIBUTES;
+        this.xmlParser = null;
     }
 
-    SvgSecurityValidator(String[] elements, String[] attributes) {
+    SvgSecurityValidator(String[] elements, String[] attributes, DocumentBuilder xmlParser) {
         this.svgElements = elements;
         this.svgAttributes = attributes;
+        this.xmlParser = xmlParser;
     }
 
     public static SvgSecurityValidatorBuilder builder() {
@@ -57,6 +59,7 @@ public static SvgSecurityValidatorBuilder builder() {
      */
     @Override
     public ValidationResult validate(String input) {
+        if (xmlParser != null) validateXMLSchema(input);
         Set<String> offendingElements = getOffendingElements(input);
         if (offendingElements.isEmpty()) return new NegativeValidationResult();
         return new PositiveValidationResult(offendingElements);
@@ -67,6 +70,15 @@ public ValidationResult validate(byte[] input) {
         return validate(new String(input, StandardCharsets.UTF_8));
     }
 
+    private void validateXMLSchema(String input) {
+        try {
+            assert xmlParser != null;
+            xmlParser.parse(new ByteArrayInputStream(input.getBytes()));
+        } catch (Exception e) {
+            throw new InvalidXMLSyntaxException(e);
+        }
+    }
+
     private Set<String> getOffendingElements(String xml) {
         if (JAVASCRIPT_PROTOCOL_IN_CSS_URL.matcher(xml).find()) return Collections.singleton("style");
         PolicyFactory policy = new HtmlPolicyBuilder()

src/main/java/com/github/bgalek/security/svg/SvgSecurityValidatorBuilder.java

@@ -1,5 +1,8 @@
 package com.github.bgalek.security.svg;
 
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
 import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Stream;
@@ -10,6 +13,7 @@
 public class SvgSecurityValidatorBuilder {
     private String[] elements = DEFAULT_SVG_ELEMENTS;
     private String[] attributes = DEFAULT_SVG_ATTRIBUTES;
+    private DocumentBuilder xmlParser;
 
     SvgSecurityValidatorBuilder() {
     }
@@ -24,7 +28,20 @@ public SvgSecurityValidatorBuilder withAdditionalAttributes(List<String> additio
         return this;
     }
 
+    public SvgSecurityValidatorBuilder withSyntaxValidation() {
+        DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
+        try {
+            documentBuilderFactory.setNamespaceAware(true);
+            documentBuilderFactory.setFeature("http://xml.org/sax/features/namespaces", false);
+            documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            this.xmlParser = documentBuilderFactory.newDocumentBuilder();
+        } catch (ParserConfigurationException e) {
+            throw new RuntimeException(e);
+        }
+        return this;
+    }
+
     public SvgSecurityValidator build() {
-        return new SvgSecurityValidator(elements, attributes);
+        return new SvgSecurityValidator(elements, attributes, xmlParser);
     }
 }

src/test/java/com/github/bgalek/security/SvgSecurityValidatorTest.java

@@ -1,7 +1,9 @@
 package com.github.bgalek.security;
 
+import com.github.bgalek.security.svg.InvalidXMLSyntaxException;
 import com.github.bgalek.security.svg.SvgSecurityValidator;
 import com.github.bgalek.security.svg.ValidationResult;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
@@ -10,7 +12,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -49,12 +50,24 @@ void shouldNotDetectAnythingInValidFilesUsingBytes(String file) {
 
     @MethodSource("brokenUseCases")
     @ParameterizedTest(name = "validate {0} svg")
-    void shouldThrowExceptionWhenInputIsNotValidXml(String file) {
+    void shouldNotThrowExceptionWhenInputIsNotValidXml(String file) {
         ValidationResult detect = SvgSecurityValidator.builder().build().validate(loadFile(file));
         assertEquals(Collections.emptySet(), detect.getOffendingElements());
         assertFalse(detect.hasViolations());
     }
 
+    @MethodSource("brokenUseCases")
+    @ParameterizedTest(name = "validate {0} svg")
+    void shouldThrowExceptionWhenInputIsNotValidXmlAndSyntaxValidationIsEnabled(String file) {
+        InvalidXMLSyntaxException exception = Assertions.assertThrows(InvalidXMLSyntaxException.class, () ->
+                SvgSecurityValidator.builder()
+                        .withSyntaxValidation()
+                        .build()
+                        .validate(loadFile(file)));
+        assertTrue(exception.getMessage().contains("lineNumber:"));
+        assertTrue(exception.getMessage().contains("columnNumber:"));
+    }
+
     @Test
     void shouldNotFailWhenUserDefinedAttributesAreUsed() {
         String testFile = loadFile("custom/custom1.svg");
@@ -120,6 +133,7 @@ private static Stream<Arguments> evilUseCases() {
 
     private static Stream<Arguments> brokenUseCases() {
         return Stream.of(
+                Arguments.of("broken/broken.syntax.svg"),
                 Arguments.of("broken/broken.csv.svg"),
                 Arguments.of("broken/broken.png.svg")
         );