Skip to content

Commit 872f8db

Browse files
Issue 536 (#537)
* Variable Scope Increase Reducing duplication by updating common strings to package-protected. * SecurityWrapperResponse setHeader updates Updating implementation to independently test the header and the value method params. Additional Logging has been added to identify each unique case of ValidationException offense. Tests have been updated and extended to verify the setHeader method. * SecurityWrapperResponse addHeader updates Updating addHeader behavior and testing to be inline with setHeader updates requested in issue. Tests have been updated and extended to verify the addHeader method. * SecurityWrapperRepsonse addHeader log update Copy/Pasta "set" context -- now corrected to "add" * SecurityWrapperResponse Header Log Improvements Appending the input header name to log messages when ValidationExceptions occur.
1 parent e071f27 commit 872f8db

File tree

3 files changed

+658
-63
lines changed

3 files changed

+658
-63
lines changed

src/main/java/org/owasp/esapi/filters/SecurityWrapperResponse.java

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -173,16 +173,28 @@ public void addDateHeader(String name, long date) {
173173
* @param value
174174
*/
175175
public void addHeader(String name, String value) {
176+
SecurityConfiguration sc = ESAPI.securityConfiguration();
177+
String strippedName = StringUtilities.stripControls(name);
178+
String strippedValue = StringUtilities.stripControls(value);
179+
String safeName = null;
180+
String safeValue = null;
176181
try {
177-
// TODO: make stripping a global config
178-
SecurityConfiguration sc = ESAPI.securityConfiguration();
179-
String strippedName = StringUtilities.stripControls(name);
180-
String strippedValue = StringUtilities.stripControls(value);
181-
String safeName = ESAPI.validator().getValidInput("addHeader", strippedName, "HTTPHeaderName", sc.getIntProp("HttpUtilities.MaxHeaderNameSize"), false);
182-
String safeValue = ESAPI.validator().getValidInput("addHeader", strippedValue, "HTTPHeaderValue", sc.getIntProp("HttpUtilities.MaxHeaderValueSize"), false);
183-
getHttpServletResponse().addHeader(safeName, safeValue);
182+
safeName = ESAPI.validator().getValidInput("addHeader", strippedName, "HTTPHeaderName", sc.getIntProp("HttpUtilities.MaxHeaderNameSize"), false);
184183
} catch (ValidationException e) {
185-
logger.warning(Logger.SECURITY_FAILURE, "Attempt to add invalid header denied", e);
184+
logger.warning(Logger.SECURITY_FAILURE, "Attempt to add invalid header NAME denied: HTTPHeaderName:"+ name, e);
185+
}
186+
187+
try {
188+
safeValue = ESAPI.validator().getValidInput("addHeader", strippedValue, "HTTPHeaderValue", sc.getIntProp("HttpUtilities.MaxHeaderValueSize"), false);
189+
} catch (ValidationException e) {
190+
logger.warning(Logger.SECURITY_FAILURE, "Attempt to add invalid header VALUE denied: HTTPHeaderName:"+ name, e);
191+
}
192+
193+
boolean validName = StringUtilities.notNullOrEmpty(safeName, true);
194+
boolean validValue = StringUtilities.notNullOrEmpty(safeValue, true);
195+
196+
if (validName && validValue) {
197+
getHttpServletResponse().addHeader(safeName, safeValue);
186198
}
187199
}
188200

@@ -483,15 +495,28 @@ public void setDateHeader(String name, long date) {
483495
* @param value
484496
*/
485497
public void setHeader(String name, String value) {
498+
SecurityConfiguration sc = ESAPI.securityConfiguration();
499+
String strippedName = StringUtilities.stripControls(name);
500+
String strippedValue = StringUtilities.stripControls(value);
501+
String safeName = null;
502+
String safeValue = null;
486503
try {
487-
String strippedName = StringUtilities.stripControls(name);
488-
String strippedValue = StringUtilities.stripControls(value);
489-
SecurityConfiguration sc = ESAPI.securityConfiguration();
490-
String safeName = ESAPI.validator().getValidInput("setHeader", strippedName, "HTTPHeaderName", sc.getIntProp("HttpUtilities.MaxHeaderNameSize"), false);
491-
String safeValue = ESAPI.validator().getValidInput("setHeader", strippedValue, "HTTPHeaderValue", sc.getIntProp("HttpUtilities.MaxHeaderValueSize"), false);
492-
getHttpServletResponse().setHeader(safeName, safeValue);
504+
safeName = ESAPI.validator().getValidInput("setHeader", strippedName, "HTTPHeaderName", sc.getIntProp("HttpUtilities.MaxHeaderNameSize"), false);
493505
} catch (ValidationException e) {
494-
logger.warning(Logger.SECURITY_FAILURE, "Attempt to set invalid header denied", e);
506+
logger.warning(Logger.SECURITY_FAILURE, "Attempt to set invalid header NAME denied: HTTPHeaderName:"+ name, e);
507+
}
508+
509+
try {
510+
safeValue = ESAPI.validator().getValidInput("setHeader", strippedValue, "HTTPHeaderValue", sc.getIntProp("HttpUtilities.MaxHeaderValueSize"), false);
511+
} catch (ValidationException e) {
512+
logger.warning(Logger.SECURITY_FAILURE, "Attempt to set invalid header VALUE denied: HTTPHeaderName:"+ name, e);
513+
}
514+
515+
boolean validName = StringUtilities.notNullOrEmpty(safeName, true);
516+
boolean validValue = StringUtilities.notNullOrEmpty(safeValue, true);
517+
518+
if (validName && validValue) {
519+
getHttpServletResponse().setHeader(safeName, safeValue);
495520
}
496521
}
497522

src/test/java/org/owasp/esapi/filters/SecurityWrapperRequestTest.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818
import static org.junit.Assert.assertNotEquals;
1919
import static org.junit.Assert.assertNull;
2020
import static org.junit.Assert.assertTrue;
21-
import static org.mockito.ArgumentMatchers.anyInt;
22-
import static org.mockito.ArgumentMatchers.*;
21+
import static org.mockito.ArgumentMatchers.anyString;
22+
import static org.mockito.ArgumentMatchers.eq;
2323
import static org.mockito.Mockito.times;
2424
import static org.mockito.Mockito.verify;
2525
// A hack for now; eventually, I plan to move this into a new org.owasp.esapi.PropNames class. -kww
@@ -41,7 +41,6 @@
4141
import org.mockito.stubbing.Answer;
4242
import org.owasp.esapi.ESAPI;
4343
import org.owasp.esapi.Logger;
44-
import org.owasp.esapi.Logger.EventType;
4544
import org.owasp.esapi.SecurityConfiguration;
4645
import org.owasp.esapi.Validator;
4746
import org.owasp.esapi.errors.ValidationException;
@@ -61,15 +60,15 @@
6160
@RunWith(PowerMockRunner.class)
6261
@PrepareForTest(ESAPI.class)
6362
public class SecurityWrapperRequestTest {
64-
private static final String ESAPI_VALIDATOR_GETTER_METHOD_NAME = "validator";
65-
private static final String ESAPI_GET_LOGGER_METHOD_NAME = "getLogger";
66-
private static final String ESAPY_SECURITY_CONFIGURATION_GETTER_METHOD_NAME = "securityConfiguration";
67-
private static final String SECURITY_CONFIGURATION_QUERY_STRING_LENGTH_KEY_NAME = "HttpUtilities.URILENGTH";
68-
private static final String SECURITY_CONFIGURATION_PARAMETER_STRING_LENGTH_KEY_NAME = "HttpUtilities.httpQueryParamValueLength";
63+
protected static final String ESAPI_VALIDATOR_GETTER_METHOD_NAME = "validator";
64+
protected static final String ESAPI_GET_LOGGER_METHOD_NAME = "getLogger";
65+
protected static final String ESAPY_SECURITY_CONFIGURATION_GETTER_METHOD_NAME = "securityConfiguration";
66+
protected static final String SECURITY_CONFIGURATION_QUERY_STRING_LENGTH_KEY_NAME = "HttpUtilities.URILENGTH";
67+
protected static final String SECURITY_CONFIGURATION_PARAMETER_STRING_LENGTH_KEY_NAME = "HttpUtilities.httpQueryParamValueLength";
6968
private static final String SECURITY_CONFIGURATION_HEADER_NAME_LENGTH_KEY_NAME = "HttpUtilities.MaxHeaderNameSize";
7069
private static final String SECURITY_CONFIGURATION_HEADER_VALUE_LENGTH_KEY_NAME = "HttpUtilities.MaxHeaderValueSize";
7170

72-
private static final int SECURITY_CONFIGURATION_TEST_LENGTH = 255;
71+
protected static final int SECURITY_CONFIGURATION_TEST_LENGTH = 255;
7372

7473
private static final String QUERY_STRING_CANONCALIZE_TYPE_KEY = "HTTPQueryString";
7574
private static final String PARAMETER_STRING_CANONCALIZE_TYPE_KEY = "HTTPParameterValue";

0 commit comments

Comments
 (0)