diff --git a/configuration/esapi/ESAPI.properties b/configuration/esapi/ESAPI.properties index 7ac0fb9d3..bafe1e38c 100644 --- a/configuration/esapi/ESAPI.properties +++ b/configuration/esapi/ESAPI.properties @@ -535,3 +535,10 @@ Validator.AcceptLenientDates=false # #Validator.HtmlValidationAction=clean Validator.HtmlValidationAction=throw + +# With the fix for #310 to enable loading antisamy-esapi.xml from the classpath +# also an enhancement was made to be able to use a different filename for the configuration. +# You don't have to configure the filename here, but in that case the code will keep looking for antisamy-esapi.xml. +# This is the default behaviour of ESAPI. +# +#Validator.HtmlValidationConfigurationFile=antisamy-esapi.xml diff --git a/src/main/java/org/owasp/esapi/SecurityConfiguration.java b/src/main/java/org/owasp/esapi/SecurityConfiguration.java index ed326d6ae..57bfcca38 100644 --- a/src/main/java/org/owasp/esapi/SecurityConfiguration.java +++ b/src/main/java/org/owasp/esapi/SecurityConfiguration.java @@ -640,7 +640,6 @@ public interface SecurityConfiguration extends EsapiPropertyLoader { */ InputStream getResourceStream( String filename ) throws IOException; - /** * Sets the ESAPI resource directory. * diff --git a/src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java b/src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java index d578850ba..24337e14a 100644 --- a/src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java +++ b/src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java @@ -159,6 +159,7 @@ public static SecurityConfiguration getInstance() { public static final String VALIDATION_PROPERTIES_MULTIVALUED = "Validator.ConfigurationFile.MultiValued"; public static final String ACCEPT_LENIENT_DATES = "Validator.AcceptLenientDates"; public static final String VALIDATOR_HTML_VALIDATION_ACTION = "Validator.HtmlValidationAction"; + public static final String VALIDATOR_HTML_VALIDATION_CONFIGURATION_FILE = "Validator.HtmlValidationConfigurationFile"; /** * Special {@code System} property that, if set to {@code true}, will diff --git a/src/main/java/org/owasp/esapi/reference/validation/HTMLValidationRule.java b/src/main/java/org/owasp/esapi/reference/validation/HTMLValidationRule.java index 0670860d9..f68253cd1 100644 --- a/src/main/java/org/owasp/esapi/reference/validation/HTMLValidationRule.java +++ b/src/main/java/org/owasp/esapi/reference/validation/HTMLValidationRule.java @@ -30,6 +30,7 @@ import org.owasp.validator.html.Policy; import org.owasp.validator.html.PolicyException; import org.owasp.validator.html.ScanException; +import org.owasp.esapi.reference.DefaultSecurityConfiguration; /** @@ -46,22 +47,113 @@ public class HTMLValidationRule extends StringValidationRule { /** OWASP AntiSamy markup verification policy */ private static Policy antiSamyPolicy = null; private static final Logger LOGGER = ESAPI.getLogger( "HTMLValidationRule" ); + private static final String ANTISAMYPOLICY_FILENAME = "antisamy-esapi.xml"; + + /** + * Used to load antisamy-esapi.xml from a variety of different classpath locations. + * The classpath locations are the same classpath locations as used to load esapi.properties. + * See DefaultSecurityConfiguration.DefaultSearchPath. + * + * @param fileName The resource file filename. + */ + private static InputStream getResourceStreamFromClasspath(String fileName) { + InputStream resourceStream = null; + + ClassLoader[] loaders = new ClassLoader[] { + Thread.currentThread().getContextClassLoader(), + ClassLoader.getSystemClassLoader(), + ESAPI.securityConfiguration().getClass().getClassLoader() + /* can't use just getClass.getClassLoader() in a static context, so using the DefaultSecurityConfiguration class. */ + }; + + String[] classLoaderNames = { + "current thread context class loader", + "system class loader", + "class loader for DefaultSecurityConfiguration class" + }; + + int i = 0; + for (ClassLoader loader : loaders) { + // try root + String currentClasspathSearchLocation = "/ (root)"; + resourceStream = loader.getResourceAsStream(DefaultSecurityConfiguration.DefaultSearchPath.ROOT.value() + fileName); + + // try resourceDirectory folder + if (resourceStream == null){ + currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.RESOURCE_DIRECTORY.value(); + resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName); + } + + // try .esapi folder. Look here first for backward compatibility. + if (resourceStream == null){ + currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.DOT_ESAPI.value(); + resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName); + } + + // try esapi folder (new directory) + if (resourceStream == null){ + currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.ESAPI.value(); + resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName); + } + + // try resources folder + if (resourceStream == null){ + currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.RESOURCES.value(); + resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName); + } + + // try src/main/resources folder + if (resourceStream == null){ + currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.SRC_MAIN_RESOURCES.value(); + resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName); + } + + if (resourceStream != null) { + LOGGER.info(Logger.EVENT_FAILURE, "SUCCESSFULLY LOADED " + fileName + " via the CLASSPATH from '" + + currentClasspathSearchLocation + "' using " + classLoaderNames[i] + "!"); + break; // Outta here since we've found and loaded it. + } + + i++; + } + + return resourceStream; + } static { InputStream resourceStream = null; + String antisamyPolicyFilename = null; + + try { + antisamyPolicyFilename = ESAPI.securityConfiguration().getStringProp( + // Future: This will be moved to a new PropNames class + org.owasp.esapi.reference.DefaultSecurityConfiguration.VALIDATOR_HTML_VALIDATION_CONFIGURATION_FILE ); + } catch (ConfigurationException cex) { + + LOGGER.info(Logger.EVENT_FAILURE, "ESAPI property " + + org.owasp.esapi.reference.DefaultSecurityConfiguration.VALIDATOR_HTML_VALIDATION_CONFIGURATION_FILE + + " not set, using default value: " + ANTISAMYPOLICY_FILENAME); + antisamyPolicyFilename = ANTISAMYPOLICY_FILENAME; + } try { - resourceStream = ESAPI.securityConfiguration().getResourceStream("antisamy-esapi.xml"); + resourceStream = ESAPI.securityConfiguration().getResourceStream(antisamyPolicyFilename); } catch (IOException e) { - throw new ConfigurationException("Couldn't find antisamy-esapi.xml", e); - } + + LOGGER.info(Logger.EVENT_FAILURE, "Loading " + antisamyPolicyFilename + " from classpaths"); + + resourceStream = getResourceStreamFromClasspath(antisamyPolicyFilename); + } if (resourceStream != null) { try { antiSamyPolicy = Policy.getInstance(resourceStream); } catch (PolicyException e) { - throw new ConfigurationException("Couldn't parse antisamy policy", e); - } - } + throw new ConfigurationException("Couldn't parse " + antisamyPolicyFilename, e); + } } + else { + throw new ConfigurationException("Couldn't find " + antisamyPolicyFilename); + } + } public HTMLValidationRule( String typeName ) { super( typeName ); diff --git a/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleClasspathTest.java b/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleClasspathTest.java new file mode 100644 index 000000000..f032059d1 --- /dev/null +++ b/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleClasspathTest.java @@ -0,0 +1,171 @@ +/** + * OWASP Enterprise Security API (ESAPI) + * + * This file is part of the Open Web Application Security Project (OWASP) + * Enterprise Security API (ESAPI) project. For details, please see + * http://www.owasp.org/index.php/ESAPI. + * + * Copyright (c) 2019 - The OWASP Foundation + * + * The ESAPI is published by OWASP under the BSD license. You should read and accept the + * LICENSE before you use, modify, and/or redistribute this software. + * + * @author kevin.w.wall@gmail.com + * @since 2019 + */ +package org.owasp.esapi.reference.validation; + +import org.owasp.esapi.ESAPI; +import org.owasp.esapi.SecurityConfiguration; +import org.owasp.esapi.SecurityConfigurationWrapper; +import org.owasp.esapi.ValidationErrorList; +import org.owasp.esapi.ValidationRule; +import org.owasp.esapi.Validator; +import org.owasp.esapi.errors.ValidationException; +import org.owasp.esapi.reference.validation.HTMLValidationRule; + +import org.junit.Test; +import org.junit.Before; +import org.junit.After; +import org.junit.Rule; +import org.junit.rules.ExpectedException; +import static org.junit.Assert.*; + +/** + * The Class HTMLValidationRuleThrowsTest. + * + * Based on original test cases, testGetValidSafeHTML() and + * testIsValidSafeHTML() from ValidatorTest by + * Mike Fauzy (mike.fauzy@aspectsecurity.com) and + * Jeff Williams (jeff.williams@aspectsecurity.com) + * that were originally part of src/test/java/org/owasp/esapi/reference/ValidatorTest.java. + * + * This class tests the cases where the new ESAPI.property + * Validator.HtmlValidationAction + * is set to "throw", which causes certain calls to + * ESAPI.validator().getValidSafeHTML() or ESAPI.validator().isValidSafeHTML() + * to throw a ValidationException rather than simply logging a warning and returning + * the cleansed (sanitizied) output when certain unsafe input is encountered. + */ +public class HTMLValidationRuleClasspathTest { + private static class ConfOverride extends SecurityConfigurationWrapper { + private String desiredReturnAction = "clean"; + private String desiredReturnConfigurationFile = "antisamy-esapi.xml"; + + ConfOverride(SecurityConfiguration orig, String desiredReturnAction, String desiredReturnConfigurationFile) { + super(orig); + this.desiredReturnAction = desiredReturnAction; + this.desiredReturnConfigurationFile = desiredReturnConfigurationFile; + } + + @Override + public String getStringProp(String propName) { + // Would it be better making this file a static import? + if ( propName.equals( org.owasp.esapi.reference.DefaultSecurityConfiguration.VALIDATOR_HTML_VALIDATION_ACTION ) ) { + return desiredReturnAction; + } else if ( propName.equals( org.owasp.esapi.reference.DefaultSecurityConfiguration.VALIDATOR_HTML_VALIDATION_CONFIGURATION_FILE ) ) { + return desiredReturnConfigurationFile; + } else { + return super.getStringProp( propName ); + } + } + } + + // Must be public! + @Rule + public ExpectedException thrownEx = ExpectedException.none(); + + @After + public void tearDown() throws Exception { + ESAPI.override(null); + thrownEx = ExpectedException.none(); + } + + @Before + public void setUp() throws Exception { + ESAPI.override( + new ConfOverride( ESAPI.securityConfiguration(), "throw", "antisamy-esapi-CP.xml" ) + ); + + } + + @Test + public void testGetValid() throws Exception { + System.out.println("getValidCP"); + Validator instance = ESAPI.validator(); + HTMLValidationRule rule = new HTMLValidationRule("testCP"); + ESAPI.validator().addRule(rule); + + thrownEx.expect(ValidationException.class); + thrownEx.expectMessage("test: Invalid HTML input"); + + instance.getRule("testCP").getValid("test", "Test. "); + } + + @Test + public void testGetValidSafeHTML() throws Exception { + System.out.println("getValidSafeHTML"); + Validator instance = ESAPI.validator(); + + HTMLValidationRule rule = new HTMLValidationRule("test"); + ESAPI.validator().addRule(rule); + + String[] testInput = { + // These first two don't cause AntiSamy to throw. + // "Test. Aspect Security", + // "Test. <
load=alert()", + "Test. ", + "Test. ", + "Test.
b
", + "Test. alert(document.cookie)", + "Test. alert(document.cookie)", + "Test. alert(document.cookie)" + }; + + int errors = 0; + for( int i = 0; i < testInput.length; i++ ) { + try { + String result = instance.getValidSafeHTML("test", testInput[i], 100, false); + errors++; + System.out.println("testGetValidSafeHTML(): testInput '" + testInput[i] + "' failed to throw."); + } + catch( ValidationException vex ) { + System.out.println("testGetValidSafeHTML(): testInput '" + testInput[i] + "' returned:"); + System.out.println("\t" + i + ": logMsg =" + vex.getLogMessage()); + assertEquals( vex.getUserMessage(), "test: Invalid HTML input"); + } + catch( Exception ex ) { + errors++; + System.out.println("testGetValidSafeHTML(): testInput '" + testInput[i] + + "' threw wrong exception type: " + ex.getClass().getName() ); + } + } + + if ( errors > 0 ) { + fail("testGetValidSafeHTML() encountered " + errors + " failures."); + } + } + + @Test + public void testIsValidSafeHTML() { + System.out.println("isValidSafeHTML"); + Validator instance = ESAPI.validator(); + thrownEx = ExpectedException.none(); // Not expecting any exceptions here. + + assertTrue(instance.isValidSafeHTML("test", "Jeff", 100, false)); + assertTrue(instance.isValidSafeHTML("test", "Aspect Security", 100, false)); + assertFalse(instance.isValidSafeHTML("test", "Test. ", 100, false)); + assertFalse(instance.isValidSafeHTML("test", "Test.
", 100, false)); + assertFalse(instance.isValidSafeHTML("test", "Test. alert(document.cookie)", 100, false)); + assertFalse(instance.isValidSafeHTML("test", "Test. alert(document.cookie)", 100, false)); + assertFalse(instance.isValidSafeHTML("test", "Test. alert(document.cookie)", 100, false)); + + ValidationErrorList errors = new ValidationErrorList(); + assertFalse(instance.isValidSafeHTML("test1", "Test. ", 100, false, errors)); + assertFalse(instance.isValidSafeHTML("test2", "Test.
", 100, false, errors)); + assertFalse(instance.isValidSafeHTML("test3", "Test. alert(document.cookie)", 100, false, errors)); + assertFalse(instance.isValidSafeHTML("test4", "Test. alert(document.cookie)", 100, false, errors)); + assertFalse(instance.isValidSafeHTML("test5", "Test. alert(document.cookie)", 100, false, errors)); + assertTrue( errors.size() == 5 ); + } +} diff --git a/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleCleanTest.java b/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleCleanTest.java index dfed45607..8bbb5f756 100644 --- a/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleCleanTest.java +++ b/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleCleanTest.java @@ -13,7 +13,7 @@ * @author kevin.w.wall@gmail.com * @since 2019 */ -package org.owasp.esapi.reference; +package org.owasp.esapi.reference.validation; import org.owasp.esapi.ESAPI; import org.owasp.esapi.EncoderConstants; diff --git a/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleThrowsTest.java b/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleThrowsTest.java index 6726ef56f..7605462aa 100644 --- a/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleThrowsTest.java +++ b/src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleThrowsTest.java @@ -13,7 +13,7 @@ * @author kevin.w.wall@gmail.com * @since 2019 */ -package org.owasp.esapi.reference; +package org.owasp.esapi.reference.validation; import org.owasp.esapi.ESAPI; import org.owasp.esapi.SecurityConfiguration; diff --git a/src/test/resources/antisamy-esapi-CP.xml b/src/test/resources/antisamy-esapi-CP.xml new file mode 100644 index 000000000..14880d0b5 --- /dev/null +++ b/src/test/resources/antisamy-esapi-CP.xml @@ -0,0 +1,492 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +