diff --git a/src/main/java/org/owasp/esapi/reference/DefaultValidator.java b/src/main/java/org/owasp/esapi/reference/DefaultValidator.java index 1a05534c0..abe6c93a5 100644 --- a/src/main/java/org/owasp/esapi/reference/DefaultValidator.java +++ b/src/main/java/org/owasp/esapi/reference/DefaultValidator.java @@ -216,6 +216,7 @@ public String getValidInput(String context, String input, String type, int maxLe } rvr.setMaximumLength(maxLength); rvr.setAllowNull(allowNull); + rvr.setCanonicalize(canonicalize); return rvr.getValid(context, input); } diff --git a/src/main/java/org/owasp/esapi/reference/validation/StringValidationRule.java b/src/main/java/org/owasp/esapi/reference/validation/StringValidationRule.java index 3163c532d..8d0c88cf0 100644 --- a/src/main/java/org/owasp/esapi/reference/validation/StringValidationRule.java +++ b/src/main/java/org/owasp/esapi/reference/validation/StringValidationRule.java @@ -20,8 +20,10 @@ import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; +import org.owasp.esapi.ESAPI; import org.owasp.esapi.Encoder; import org.owasp.esapi.EncoderConstants; +import org.owasp.esapi.Logger; import org.owasp.esapi.StringUtilities; import org.owasp.esapi.errors.ValidationException; import org.owasp.esapi.util.NullSafe; @@ -39,11 +41,12 @@ * http://en.wikipedia.org/wiki/Whitelist */ public class StringValidationRule extends BaseValidationRule { - + private static final Logger LOGGER = ESAPI.getLogger(StringValidationRule.class); protected List whitelistPatterns = new ArrayList(); protected List blacklistPatterns = new ArrayList(); protected int minLength = 0; protected int maxLength = Integer.MAX_VALUE; + private boolean canonicalizeInput = true; public StringValidationRule( String typeName ) { super( typeName ); @@ -115,6 +118,10 @@ public void setMaximumLength( int length ) { maxLength = length; } + public void setCanonicalize(boolean canonicalize) { + this.canonicalizeInput = canonicalize; + } + /** * checks input against whitelists. * @param context The context to include in exception messages @@ -262,9 +269,15 @@ public String getValid( String context, String input ) throws ValidationExceptio // check length checkLength(context, input); - + // canonicalize - data = encoder.canonicalize( input ); + if (canonicalizeInput) { + data = encoder.canonicalize(input); + } else { + String message = String.format("Input validaiton excludes canonicalization. Context: %s Input: %s", context, input); + LOGGER.warning(Logger.SECURITY_AUDIT, message); + data = input; + } // check whitelist patterns checkWhitelist(context, input); @@ -284,5 +297,6 @@ public String sanitize( String context, String input ) { return whitelist( input, EncoderConstants.CHAR_ALPHANUMERICS ); } + } diff --git a/src/test/java/org/owasp/esapi/reference/DefaultValidatorInputStringAPITest.java b/src/test/java/org/owasp/esapi/reference/DefaultValidatorInputStringAPITest.java index 525584b6a..99e25c493 100644 --- a/src/test/java/org/owasp/esapi/reference/DefaultValidatorInputStringAPITest.java +++ b/src/test/java/org/owasp/esapi/reference/DefaultValidatorInputStringAPITest.java @@ -17,6 +17,7 @@ import java.util.regex.Pattern; import org.hamcrest.core.Is; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -66,10 +67,6 @@ public class DefaultValidatorInputStringAPITest { @Before public void setup() throws Exception { - //Is intrusion detection disabled? A: Yes, it is off. - //This logic is confusing: True, the value is False... - when(mockSecConfig.getDisableIntrusionDetection()).thenReturn(true); - contextStr = testName.getMethodName(); testValidatorType = testName.getMethodName() + "_validator_type"; validatorResultString = testName.getMethodName() + "_validator_result"; @@ -101,6 +98,13 @@ public void setup() throws Exception { } + @After + public void verifyDelegateCalls() { + verify(mockSecConfig, times(1)).getValidationPattern(testValidatorType); + + PowerMockito.verifyNoMoreInteractions(spyStringRule, mockSecConfig, mockEncoder); + } + @Test public void getValidInputNullAllowedPassthrough() throws Exception { String safeValue = uit.getValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, true); @@ -109,6 +113,7 @@ public void getValidInputNullAllowedPassthrough() throws Exception { verify(spyStringRule, times(1)).setAllowNull(true); verify(spyStringRule, times(0)).setAllowNull(false); verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength); + verify(spyStringRule, times(1)).setCanonicalize(true); verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName()); } @@ -120,6 +125,7 @@ public void getValidInputNullNotAllowedPassthrough() throws Exception { verify(spyStringRule, times(0)).setAllowNull(true); verify(spyStringRule, times(1)).setAllowNull(false); verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength); + verify(spyStringRule, times(1)).setCanonicalize(true); verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName()); } @@ -137,7 +143,16 @@ public void getValidInputValidationExceptionPropagates() throws Exception { exEx.expect(Is.is(validationEx)); doThrow(validationEx).when(spyStringRule).getValid(ArgumentMatchers.anyString(), ArgumentMatchers.anyString()); - uit.getValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, true); + try { + uit.getValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, true); + } finally { + verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN); + verify(spyStringRule, times(1)).setAllowNull(true); + verify(spyStringRule, times(0)).setAllowNull(false); + verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength); + verify(spyStringRule, times(1)).setCanonicalize(true); + verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName()); + } } @Test @@ -149,6 +164,12 @@ public void getValidInputValidationExceptionErrorList() throws Exception { assertTrue(result.isEmpty()); assertEquals(1, errorList.size()); assertEquals(validationEx, errorList.getError(contextStr)); + verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN); + verify(spyStringRule, times(1)).setAllowNull(true); + verify(spyStringRule, times(0)).setAllowNull(false); + verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength); + verify(spyStringRule, times(1)).setCanonicalize(true); + verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName()); } @@ -161,6 +182,7 @@ public void isValidInputNullAllowedPassthrough() throws Exception { verify(spyStringRule, times(1)).setAllowNull(true); verify(spyStringRule, times(0)).setAllowNull(false); verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength); + verify(spyStringRule, times(1)).setCanonicalize(true); verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName()); } @@ -169,6 +191,12 @@ public void isValidInputValidationExceptionReturnsFalse() throws Exception { doThrow(validationEx).when(spyStringRule).getValid(ArgumentMatchers.anyString(), ArgumentMatchers.anyString()); boolean result = uit.isValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, true); assertFalse(result); + verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN); + verify(spyStringRule, times(1)).setAllowNull(true); + verify(spyStringRule, times(0)).setAllowNull(false); + verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength); + verify(spyStringRule, times(1)).setCanonicalize(true); + verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName()); } @Test @@ -180,5 +208,24 @@ public void isValidInputValidationExceptionErrorListReturnsFalse() throws Except assertFalse(result); assertEquals(1, errorList.size()); assertEquals(validationEx, errorList.getError(contextStr)); + verify(errors, times(1)).addError(contextStr, validationEx); + verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN); + verify(spyStringRule, times(1)).setAllowNull(true); + verify(spyStringRule, times(0)).setAllowNull(false); + verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength); + verify(spyStringRule, times(1)).setCanonicalize(true); + verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName()); + } + + @Test + public void canonicalizeSettingPassedThrough() throws Exception { + String safeValue = uit.getValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, false,false); + assertEquals(validatorResultString, safeValue); + verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN); + verify(spyStringRule, times(0)).setAllowNull(true); + verify(spyStringRule, times(1)).setAllowNull(false); + verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength); + verify(spyStringRule, times(1)).setCanonicalize(false); + verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName()); } } diff --git a/src/test/java/org/owasp/esapi/reference/validation/StringValidationRuleTest.java b/src/test/java/org/owasp/esapi/reference/validation/StringValidationRuleTest.java index d580d4cc7..e172e2ebb 100644 --- a/src/test/java/org/owasp/esapi/reference/validation/StringValidationRuleTest.java +++ b/src/test/java/org/owasp/esapi/reference/validation/StringValidationRuleTest.java @@ -3,6 +3,9 @@ import junit.framework.Assert; import org.junit.Test; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; +import org.owasp.esapi.Encoder; import org.owasp.esapi.ValidationErrorList; import org.owasp.esapi.errors.ValidationException; @@ -155,4 +158,23 @@ public void testAllowNull() throws ValidationException { Assert.assertTrue(validationRule.isValid("", null)); } + @Test + public void testSetCanonicalize() throws ValidationException { + String context = "test-scope"; + String inputString = "SomeInputValue"; + String encoderReturn = "MockReturnValue"; + Encoder mockEncoder = Mockito.mock(Encoder.class); + Mockito.when(mockEncoder.canonicalize(inputString)).thenReturn(encoderReturn); + StringValidationRule testRule = new StringValidationRule(context, mockEncoder); + String valid = testRule.getValid(context, inputString); + Assert.assertEquals(encoderReturn, valid); + Mockito.verify(mockEncoder, Mockito.times(1)).canonicalize(inputString); + Mockito.reset(mockEncoder); + + testRule.setCanonicalize(false); + valid = testRule.getValid(context, inputString); + Assert.assertEquals(inputString, valid); + Mockito.verify(mockEncoder, Mockito.times(0)).canonicalize(inputString); + } + }