Skip to content

Commit 3475e86

Browse files
jeremiahjstaceykwwall
jeremiahjstacey
authored and
kwwall
committed
#476 DefaultValidator.getValidInput uses canonicalize method argument (#477)
Close issue #476 * DefaultValidator.*ValidInput uses canonicalize arg Exposing behavior in the StringValidationRule to allow an implementor to opt-out of encoder canonicalization behavior. Tests added to verify intended behavior. DefaultValidator getValidInput has been updated to use the existing canonicalize argument to configure the StringValidationRule delegate for canonicalization behavior before validating the input reference. Tests updated to validate configuration occurs in workflow. Additional test updates in the DefaultValidatorInputStringAPITest to remove unnecessary mock configuration expectations, and to verify that all expected mock interactions have occurred. * Logging in StringValidationRule Adding recommended logging if canonicalization is set to false.
1 parent 8ffe783 commit 3475e86

File tree

4 files changed

+92
-8
lines changed

4 files changed

+92
-8
lines changed

‎src/main/java/org/owasp/esapi/reference/DefaultValidator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ public String getValidInput(String context, String input, String type, int maxLe
216216
}
217217
rvr.setMaximumLength(maxLength);
218218
rvr.setAllowNull(allowNull);
219+
rvr.setCanonicalize(canonicalize);
219220
return rvr.getValid(context, input);
220221
}
221222

‎src/main/java/org/owasp/esapi/reference/validation/StringValidationRule.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
import java.util.regex.Pattern;
2121
import java.util.regex.PatternSyntaxException;
2222

23+
import org.owasp.esapi.ESAPI;
2324
import org.owasp.esapi.Encoder;
2425
import org.owasp.esapi.EncoderConstants;
26+
import org.owasp.esapi.Logger;
2527
import org.owasp.esapi.StringUtilities;
2628
import org.owasp.esapi.errors.ValidationException;
2729
import org.owasp.esapi.util.NullSafe;
@@ -39,11 +41,12 @@
3941
* http://en.wikipedia.org/wiki/Whitelist
4042
*/
4143
public class StringValidationRule extends BaseValidationRule {
42-
44+
private static final Logger LOGGER = ESAPI.getLogger(StringValidationRule.class);
4345
protected List<Pattern> whitelistPatterns = new ArrayList<Pattern>();
4446
protected List<Pattern> blacklistPatterns = new ArrayList<Pattern>();
4547
protected int minLength = 0;
4648
protected int maxLength = Integer.MAX_VALUE;
49+
private boolean canonicalizeInput = true;
4750

4851
public StringValidationRule( String typeName ) {
4952
super( typeName );
@@ -115,6 +118,10 @@ public void setMaximumLength( int length ) {
115118
maxLength = length;
116119
}
117120

121+
public void setCanonicalize(boolean canonicalize) {
122+
this.canonicalizeInput = canonicalize;
123+
}
124+
118125
/**
119126
* checks input against whitelists.
120127
* @param context The context to include in exception messages
@@ -262,9 +269,15 @@ public String getValid( String context, String input ) throws ValidationExceptio
262269

263270
// check length
264271
checkLength(context, input);
265-
272+
266273
// canonicalize
267-
data = encoder.canonicalize( input );
274+
if (canonicalizeInput) {
275+
data = encoder.canonicalize(input);
276+
} else {
277+
String message = String.format("Input validaiton excludes canonicalization. Context: %s Input: %s", context, input);
278+
LOGGER.warning(Logger.SECURITY_AUDIT, message);
279+
data = input;
280+
}
268281

269282
// check whitelist patterns
270283
checkWhitelist(context, input);
@@ -284,5 +297,6 @@ public String sanitize( String context, String input ) {
284297
return whitelist( input, EncoderConstants.CHAR_ALPHANUMERICS );
285298
}
286299

300+
287301
}
288302

‎src/test/java/org/owasp/esapi/reference/DefaultValidatorInputStringAPITest.java‎

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.util.regex.Pattern;
1818

1919
import org.hamcrest.core.Is;
20+
import org.junit.After;
2021
import org.junit.Before;
2122
import org.junit.Rule;
2223
import org.junit.Test;
@@ -66,10 +67,6 @@ public class DefaultValidatorInputStringAPITest {
6667

6768
@Before
6869
public void setup() throws Exception {
69-
//Is intrusion detection disabled? A: Yes, it is off.
70-
//This logic is confusing: True, the value is False...
71-
when(mockSecConfig.getDisableIntrusionDetection()).thenReturn(true);
72-
7370
contextStr = testName.getMethodName();
7471
testValidatorType = testName.getMethodName() + "_validator_type";
7572
validatorResultString = testName.getMethodName() + "_validator_result";
@@ -101,6 +98,13 @@ public void setup() throws Exception {
10198

10299
}
103100

101+
@After
102+
public void verifyDelegateCalls() {
103+
verify(mockSecConfig, times(1)).getValidationPattern(testValidatorType);
104+
105+
PowerMockito.verifyNoMoreInteractions(spyStringRule, mockSecConfig, mockEncoder);
106+
}
107+
104108
@Test
105109
public void getValidInputNullAllowedPassthrough() throws Exception {
106110
String safeValue = uit.getValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, true);
@@ -109,6 +113,7 @@ public void getValidInputNullAllowedPassthrough() throws Exception {
109113
verify(spyStringRule, times(1)).setAllowNull(true);
110114
verify(spyStringRule, times(0)).setAllowNull(false);
111115
verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength);
116+
verify(spyStringRule, times(1)).setCanonicalize(true);
112117
verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName());
113118
}
114119

@@ -120,6 +125,7 @@ public void getValidInputNullNotAllowedPassthrough() throws Exception {
120125
verify(spyStringRule, times(0)).setAllowNull(true);
121126
verify(spyStringRule, times(1)).setAllowNull(false);
122127
verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength);
128+
verify(spyStringRule, times(1)).setCanonicalize(true);
123129
verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName());
124130
}
125131

@@ -137,7 +143,16 @@ public void getValidInputValidationExceptionPropagates() throws Exception {
137143
exEx.expect(Is.is(validationEx));
138144

139145
doThrow(validationEx).when(spyStringRule).getValid(ArgumentMatchers.anyString(), ArgumentMatchers.anyString());
140-
uit.getValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, true);
146+
try {
147+
uit.getValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, true);
148+
} finally {
149+
verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN);
150+
verify(spyStringRule, times(1)).setAllowNull(true);
151+
verify(spyStringRule, times(0)).setAllowNull(false);
152+
verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength);
153+
verify(spyStringRule, times(1)).setCanonicalize(true);
154+
verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName());
155+
}
141156
}
142157

143158
@Test
@@ -149,6 +164,12 @@ public void getValidInputValidationExceptionErrorList() throws Exception {
149164
assertTrue(result.isEmpty());
150165
assertEquals(1, errorList.size());
151166
assertEquals(validationEx, errorList.getError(contextStr));
167+
verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN);
168+
verify(spyStringRule, times(1)).setAllowNull(true);
169+
verify(spyStringRule, times(0)).setAllowNull(false);
170+
verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength);
171+
verify(spyStringRule, times(1)).setCanonicalize(true);
172+
verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName());
152173
}
153174

154175

@@ -161,6 +182,7 @@ public void isValidInputNullAllowedPassthrough() throws Exception {
161182
verify(spyStringRule, times(1)).setAllowNull(true);
162183
verify(spyStringRule, times(0)).setAllowNull(false);
163184
verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength);
185+
verify(spyStringRule, times(1)).setCanonicalize(true);
164186
verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName());
165187
}
166188

@@ -169,6 +191,12 @@ public void isValidInputValidationExceptionReturnsFalse() throws Exception {
169191
doThrow(validationEx).when(spyStringRule).getValid(ArgumentMatchers.anyString(), ArgumentMatchers.anyString());
170192
boolean result = uit.isValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, true);
171193
assertFalse(result);
194+
verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN);
195+
verify(spyStringRule, times(1)).setAllowNull(true);
196+
verify(spyStringRule, times(0)).setAllowNull(false);
197+
verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength);
198+
verify(spyStringRule, times(1)).setCanonicalize(true);
199+
verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName());
172200
}
173201

174202
@Test
@@ -180,5 +208,24 @@ public void isValidInputValidationExceptionErrorListReturnsFalse() throws Except
180208
assertFalse(result);
181209
assertEquals(1, errorList.size());
182210
assertEquals(validationEx, errorList.getError(contextStr));
211+
verify(errors, times(1)).addError(contextStr, validationEx);
212+
verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN);
213+
verify(spyStringRule, times(1)).setAllowNull(true);
214+
verify(spyStringRule, times(0)).setAllowNull(false);
215+
verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength);
216+
verify(spyStringRule, times(1)).setCanonicalize(true);
217+
verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName());
218+
}
219+
220+
@Test
221+
public void canonicalizeSettingPassedThrough() throws Exception {
222+
String safeValue = uit.getValidInput(contextStr, testName.getMethodName(), testValidatorType, testMaximumLength, false,false);
223+
assertEquals(validatorResultString, safeValue);
224+
verify(spyStringRule, times(1)).addWhitelistPattern(TEST_PATTERN);
225+
verify(spyStringRule, times(0)).setAllowNull(true);
226+
verify(spyStringRule, times(1)).setAllowNull(false);
227+
verify(spyStringRule, times(1)).setMaximumLength(testMaximumLength);
228+
verify(spyStringRule, times(1)).setCanonicalize(false);
229+
verify(spyStringRule, times(1)).getValid(contextStr, testName.getMethodName());
183230
}
184231
}

‎src/test/java/org/owasp/esapi/reference/validation/StringValidationRuleTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
import junit.framework.Assert;
44

55
import org.junit.Test;
6+
import org.mockito.ArgumentMatchers;
7+
import org.mockito.Mockito;
8+
import org.owasp.esapi.Encoder;
69
import org.owasp.esapi.ValidationErrorList;
710
import org.owasp.esapi.errors.ValidationException;
811

@@ -155,4 +158,23 @@ public void testAllowNull() throws ValidationException {
155158
Assert.assertTrue(validationRule.isValid("", null));
156159
}
157160

161+
@Test
162+
public void testSetCanonicalize() throws ValidationException {
163+
String context = "test-scope";
164+
String inputString = "SomeInputValue";
165+
String encoderReturn = "MockReturnValue";
166+
Encoder mockEncoder = Mockito.mock(Encoder.class);
167+
Mockito.when(mockEncoder.canonicalize(inputString)).thenReturn(encoderReturn);
168+
StringValidationRule testRule = new StringValidationRule(context, mockEncoder);
169+
String valid = testRule.getValid(context, inputString);
170+
Assert.assertEquals(encoderReturn, valid);
171+
Mockito.verify(mockEncoder, Mockito.times(1)).canonicalize(inputString);
172+
Mockito.reset(mockEncoder);
173+
174+
testRule.setCanonicalize(false);
175+
valid = testRule.getValid(context, inputString);
176+
Assert.assertEquals(inputString, valid);
177+
Mockito.verify(mockEncoder, Mockito.times(0)).canonicalize(inputString);
178+
}
179+
158180
}

0 commit comments

Comments
 (0)