diff --git a/src/main/java/org/owasp/validator/html/Policy.java b/src/main/java/org/owasp/validator/html/Policy.java index faab2715..6c2587b9 100644 --- a/src/main/java/org/owasp/validator/html/Policy.java +++ b/src/main/java/org/owasp/validator/html/Policy.java @@ -31,7 +31,6 @@ import java.io.InputStream; import java.net.MalformedURLException; import java.net.URI; -import java.net.URISyntaxException; import java.net.URL; import java.util.ArrayList; import java.util.Collections; @@ -40,6 +39,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.Callable; import java.util.regex.Pattern; import javax.xml.XMLConstants; @@ -120,6 +120,8 @@ public class Policy { * XML Schema for policy validation */ private static Schema schema = null; + private static boolean validateSchema = true; + private static SAXException savedSchemaValidationException; /** * Get the Tag specified by the provided tag name. @@ -160,6 +162,16 @@ public Property getPropertyByName(String propertyName) { return cssRules.get(propertyName.toLowerCase()); } + /** + * This can enable/disable the schema validation against AntiSamy XSD for the instantiated policies. + * It is enabled by default. + * + * @param enable Boolean value to specify if the schema validation should be performed. Use false to disable. + */ + public static void toggleSchemaValidation(boolean enable) { + validateSchema = enable; + } + /** * This retrieves a Policy based on a default location ("resources/antisamy.xml") * @@ -278,9 +290,19 @@ protected static ParseContext getParseContext(Element topLevelElement, URL baseU return parseContext; } + protected static Element getTopLevelElement(final URL baseUrl) throws PolicyException { + final InputSource source = getSourceFromUrl(baseUrl); + return getTopLevelElement(source, new Callable() { + @Override + public InputSource call() throws PolicyException { + return getSourceFromUrl(baseUrl); + } + }); + } + @SuppressFBWarnings(value = "SECURITY", justification="Opening a stream to the provided URL is not " - + "a vulnerability because it points to a local JAR file.") - protected static Element getTopLevelElement(URL baseUrl) throws PolicyException { + + "a vulnerability because it points to a local JAR file.") + protected static InputSource getSourceFromUrl(URL baseUrl) throws PolicyException { try { InputSource source = resolveEntity(baseUrl.toExternalForm(), baseUrl); if (source == null) { @@ -290,7 +312,7 @@ protected static Element getTopLevelElement(URL baseUrl) throws PolicyException source.setSystemId(baseUrl.toExternalForm()); } - return getTopLevelElement( source); + return source; } catch (SAXException | IOException e) { // SAXException can't actually happen. See JavaDoc for resolveEntity(String, URL) throw new PolicyException(e); @@ -298,32 +320,64 @@ protected static Element getTopLevelElement(URL baseUrl) throws PolicyException } private static Element getTopLevelElement(InputStream is) throws PolicyException { - return getTopLevelElement(new InputSource(is)); + final InputSource source = new InputSource(is); + source.getByteStream().mark(0); + return getTopLevelElement(source, new Callable() { + @Override + public InputSource call() throws IOException { + source.getByteStream().reset(); + return source; + } + }); } - protected static Element getTopLevelElement(InputSource source) throws PolicyException { + protected static Element getTopLevelElement(InputSource source, Callable getResetSource) throws PolicyException { try { - getPolicySchema(); + return getDocumentElementFromSource(source, true); + } catch (SAXException e) { + if (!validateSchema) { + try { + savedSchemaValidationException = e; + source = getResetSource.call(); + return getDocumentElementFromSource(source, false); + } catch (Exception e2) { + savedSchemaValidationException = null; + throw new PolicyException(e2); + } + } else { + throw new PolicyException(e); + } + } catch (ParserConfigurationException | IOException e) { + throw new PolicyException(e); + } + } - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + private static Element getDocumentElementFromSource(InputSource source, boolean schemaValidationEnabled) + throws ParserConfigurationException, SAXException, IOException { - /** - * Disable external entities, etc. - */ - dbf.setFeature(EXTERNAL_GENERAL_ENTITIES, false); - dbf.setFeature(EXTERNAL_PARAM_ENTITIES, false); - dbf.setFeature(DISALLOW_DOCTYPE_DECL, true); - dbf.setFeature(LOAD_EXTERNAL_DTD, false); + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + + /** + * Disable external entities, etc. + */ + dbf.setFeature(EXTERNAL_GENERAL_ENTITIES, false); + dbf.setFeature(EXTERNAL_PARAM_ENTITIES, false); + dbf.setFeature(DISALLOW_DOCTYPE_DECL, true); + dbf.setFeature(LOAD_EXTERNAL_DTD, false); + + if (schemaValidationEnabled) { + getPolicySchema(); dbf.setNamespaceAware(true); dbf.setSchema(schema); - DocumentBuilder db = dbf.newDocumentBuilder(); - db.setErrorHandler(new SAXErrorHandler()); - Document dom = db.parse(source); - - return dom.getDocumentElement(); - } catch (SAXException | ParserConfigurationException | IOException | URISyntaxException e) { - throw new PolicyException(e); } + + DocumentBuilder db = dbf.newDocumentBuilder(); + db.setErrorHandler(new SAXErrorHandler()); + Document dom = db.parse(source); + + triggerSchemaValidationWarnings(); + + return dom.getDocumentElement(); } private static void parsePolicy(Element topLevelElement, ParseContext parseContext) @@ -350,74 +404,106 @@ private static void parsePolicy(Element topLevelElement, ParseContext parseConte */ @SuppressFBWarnings(value = "SECURITY", justification="Opening a stream to the provided URL is not " + "a vulnerability because only local file URLs are allowed.") - private static Element getPolicy(String href, URL baseUrl) - throws PolicyException { - + private static Element getPolicy(String href, URL baseUrl) throws PolicyException { try { - InputSource source = null; + return getDocumentElementByUrl(href, baseUrl, true); + } catch (SAXException e) { + if (!validateSchema) { + try { + savedSchemaValidationException = e; + return getDocumentElementByUrl(href, baseUrl, false); + } catch (SAXException | ParserConfigurationException | IOException e2) { + savedSchemaValidationException = null; + throw new PolicyException(e2); + } + } else { + throw new PolicyException(e); + } + } catch (ParserConfigurationException | IOException e) { + throw new PolicyException(e); + } + } - // Can't resolve public id, but might be able to resolve relative - // system id, since we have a base URI. - if (href != null && baseUrl != null) { + private static Element getDocumentElementByUrl(String href, URL baseUrl, boolean schemaValidationEnabled) + throws IOException, ParserConfigurationException, SAXException { - if (!"file".equals(baseUrl.getProtocol())) { - throw new MalformedURLException( - "Only local files can be accessed with the baseURL. Illegal value supplied was: " + baseUrl); - } + InputSource source = null; - URL url; + // Can't resolve public id, but might be able to resolve relative + // system id, since we have a base URI. + if (href != null && baseUrl != null) { + + if (!"file".equals(baseUrl.getProtocol())) { + throw new MalformedURLException( + "Only local files can be accessed with the baseURL. Illegal value supplied was: " + baseUrl); + } + + URL url; + + try { + url = new URL(baseUrl, href); + source = new InputSource(url.openStream()); + source.setSystemId(href); + } catch (MalformedURLException | java.io.FileNotFoundException e) { try { - url = new URL(baseUrl, href); + String absURL = URIUtils.resolveAsString(href, baseUrl.toString()); + url = new URL(absURL); source = new InputSource(url.openStream()); source.setSystemId(href); - } catch (MalformedURLException | java.io.FileNotFoundException e) { - try { - String absURL = URIUtils.resolveAsString(href, baseUrl.toString()); - url = new URL(absURL); - source = new InputSource(url.openStream()); - source.setSystemId(href); - - } catch (MalformedURLException ex2) { - // nothing to do - } + } catch (MalformedURLException ex2) { + // nothing to do } } + } - getPolicySchema(); + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + /** + * Disable external entities, etc. + */ + dbf.setFeature(EXTERNAL_GENERAL_ENTITIES, false); + dbf.setFeature(EXTERNAL_PARAM_ENTITIES, false); + dbf.setFeature(DISALLOW_DOCTYPE_DECL, true); + dbf.setFeature(LOAD_EXTERNAL_DTD, false); - /** - * Disable external entities, etc. - */ - dbf.setFeature(EXTERNAL_GENERAL_ENTITIES, false); - dbf.setFeature(EXTERNAL_PARAM_ENTITIES, false); - dbf.setFeature(DISALLOW_DOCTYPE_DECL, true); - dbf.setFeature(LOAD_EXTERNAL_DTD, false); + if (schemaValidationEnabled) { + getPolicySchema(); dbf.setNamespaceAware(true); dbf.setSchema(schema); - DocumentBuilder db = dbf.newDocumentBuilder(); - db.setErrorHandler(new SAXErrorHandler()); - Document dom; + } + + DocumentBuilder db = dbf.newDocumentBuilder(); + db.setErrorHandler(new SAXErrorHandler()); + Document dom; + + /** + * Load and parse the file. + */ + if (source != null) { + dom = db.parse(source); + + triggerSchemaValidationWarnings(); /** - * Load and parse the file. + * Get the policy information out of it! */ - if (source != null) { - dom = db.parse(source); - /** - * Get the policy information out of it! - */ + return dom.getDocumentElement(); + } - return dom.getDocumentElement(); - } + return null; + } - return null; - } catch (SAXException | ParserConfigurationException | IOException | URISyntaxException e) { - throw new PolicyException(e); + private static void triggerSchemaValidationWarnings() { + if (!validateSchema) { + System.out.println("WARNING: XML schema validation for the policy is disabled, but it should not be."); + } + + if (savedSchemaValidationException != null) { + System.out.println("WARNING: " + savedSchemaValidationException.getMessage()); + savedSchemaValidationException = null; } } @@ -906,7 +992,7 @@ public AntiSamyPattern getCommonRegularExpressions(String name) { return commonRegularExpressions.get(name); } - private static void getPolicySchema() throws URISyntaxException, SAXException { + private static void getPolicySchema() throws SAXException { if (schema == null) { InputStream schemaStream = Policy.class.getClassLoader().getResourceAsStream(POLICY_SCHEMA_URI); Source schemaSource = new StreamSource(schemaStream); diff --git a/src/test/java/org/owasp/validator/html/test/PolicyTest.java b/src/test/java/org/owasp/validator/html/test/PolicyTest.java index 5a0af270..839f02b6 100644 --- a/src/test/java/org/owasp/validator/html/test/PolicyTest.java +++ b/src/test/java/org/owasp/validator/html/test/PolicyTest.java @@ -35,6 +35,7 @@ import org.owasp.validator.html.scan.Constants; import java.io.ByteArrayInputStream; +import java.net.URL; /** * This class tests the Policy functionality to show that we can successfully parse the policy file. @@ -115,7 +116,7 @@ public void testInvalidPolicies() { String policyFile = assembleFile(notSupportedTagsSection); try { policy = Policy.getInstance(new ByteArrayInputStream(policyFile.getBytes())); - fail("Not supported tag on policy, but not PolicyException occurred."); + fail("Not supported tag on policy, but no PolicyException occurred."); } catch (PolicyException e) { assertNotNull(e); } @@ -140,5 +141,72 @@ public void testInvalidPolicies() { assertNotNull(e); } } + + public void testSchemaValidationToggleWithSource() { + String notSupportedTagsSection = "\n" + + "\n"; + String policyFile = assembleFile(notSupportedTagsSection); + + // Disable validation + Policy.toggleSchemaValidation(false); + + try { + policy = Policy.getInstance(new ByteArrayInputStream(policyFile.getBytes())); + assertNotNull(policy); + } catch (PolicyException e) { + fail("Schema validation is disabled, policy creation should not fail."); + } + + // This one should only print a warning on the console because validation is disabled + try { + policy = Policy.getInstance(new ByteArrayInputStream(assembleFile("").getBytes())); + assertNotNull(policy); + } catch (PolicyException e) { + fail("Schema validation is disabled, policy creation should not fail."); + } + + // Enable validation again + Policy.toggleSchemaValidation(true); + + try { + policy = Policy.getInstance(new ByteArrayInputStream(policyFile.getBytes())); + fail("Not supported tag on policy, but no PolicyException occurred."); + } catch (PolicyException e) { + assertNotNull(e); + } + } + + public void testSchemaValidationToggleWithUrl() { + URL urlOfValidPolicy = getClass().getResource("/antisamy.xml"); + URL urlOfInvalidPolicy = getClass().getResource("/invalidPolicy.xml"); + + // Disable validation + Policy.toggleSchemaValidation(false); + + try { + policy = TestPolicy.getInstance(urlOfInvalidPolicy); + assertNotNull(policy); + } catch (PolicyException e) { + fail("Schema validation is disabled, policy creation should not fail."); + } + + // This one should only print a warning on the console because validation is disabled + try { + policy = TestPolicy.getInstance(urlOfValidPolicy); + assertNotNull(policy); + } catch (PolicyException e) { + fail("Schema validation is disabled, policy creation should not fail."); + } + + // Enable validation again + Policy.toggleSchemaValidation(true); + + try { + policy = TestPolicy.getInstance(urlOfInvalidPolicy); + fail("Not supported tag on policy, but no PolicyException occurred."); + } catch (PolicyException e) { + assertNotNull(e); + } + } } diff --git a/src/test/resources/invalidPolicy.xml b/src/test/resources/invalidPolicy.xml new file mode 100644 index 00000000..80756268 --- /dev/null +++ b/src/test/resources/invalidPolicy.xml @@ -0,0 +1,11 @@ + + + + + + + + + + + \ No newline at end of file