Skip to content

Commit 9fb1ff8

Browse files
committed
Address LGTM alerts about failure to use 'secure' cookies.
1 parent 120ff02 commit 9fb1ff8

File tree

1 file changed

+42
-50
lines changed

1 file changed

+42
-50
lines changed

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

Lines changed: 42 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,11 @@ public class SecurityWrapperRequest extends HttpServletRequestWrapper implements
6464
* @param request The {@code HttpServletRequest} we are wrapping.
6565
*/
6666
public SecurityWrapperRequest(HttpServletRequest request) {
67-
super( request );
67+
super( request );
6868
}
6969

7070
private HttpServletRequest getHttpServletRequest() {
71-
return (HttpServletRequest)super.getRequest();
71+
return (HttpServletRequest)super.getRequest();
7272
}
7373

7474
/**
@@ -128,8 +128,8 @@ public String getContentType() {
128128
public String getContextPath() {
129129
String path = getHttpServletRequest().getContextPath();
130130
SecurityConfiguration sc = ESAPI.securityConfiguration();
131-
//Return empty String for the ROOT context
132-
if (path == null || "".equals(path.trim())) return "";
131+
//Return empty String for the ROOT context
132+
if (path == null || "".equals(path.trim())) return "";
133133

134134
String clean = "";
135135
try {
@@ -159,7 +159,7 @@ public Cookie[] getCookies() {
159159
int maxAge = c.getMaxAge();
160160
String domain = c.getDomain();
161161
String path = c.getPath();
162-
162+
163163
Cookie n = new Cookie(name, value);
164164
n.setMaxAge(maxAge);
165165

@@ -347,7 +347,7 @@ public String getParameter(String name) {
347347
* @return The "scrubbed" parameter value.
348348
*/
349349
public String getParameter(String name, boolean allowNull) {
350-
SecurityConfiguration sc = ESAPI.securityConfiguration();
350+
SecurityConfiguration sc = ESAPI.securityConfiguration();
351351
return getParameter(name, allowNull, sc.getIntProp("HttpUtilities.httpQueryParamValueLength"), "HTTPParameterValue");
352352
}
353353

@@ -423,7 +423,7 @@ public Enumeration getParameterNames() {
423423
Enumeration en = getHttpServletRequest().getParameterNames();
424424
while (en.hasMoreElements()) {
425425
try {
426-
SecurityConfiguration sc = ESAPI.securityConfiguration();
426+
SecurityConfiguration sc = ESAPI.securityConfiguration();
427427
String name = (String) en.nextElement();
428428
String clean = ESAPI.validator().getValidInput("HTTP parameter name: " + name, name, "HTTPParameterName", sc.getIntProp("HttpUtilities.httpQueryParamNameLength"), true);
429429
v.add(clean);
@@ -446,8 +446,8 @@ public String[] getParameterValues(String name) {
446446
String[] values = getHttpServletRequest().getParameterValues(name);
447447
List<String> newValues;
448448

449-
if(values == null)
450-
return null;
449+
if(values == null)
450+
return null;
451451
newValues = new ArrayList<String>();
452452
SecurityConfiguration sc = ESAPI.securityConfiguration();
453453
for (String value : values) {
@@ -469,7 +469,7 @@ public String[] getParameterValues(String name) {
469469
*/
470470
public String getPathInfo() {
471471
String path = getHttpServletRequest().getPathInfo();
472-
if (path == null) return null;
472+
if (path == null) return null;
473473
String clean = "";
474474
SecurityConfiguration sc = ESAPI.securityConfiguration();
475475
try {
@@ -681,15 +681,15 @@ public String getServerName() {
681681
* HttpServletRequest after parsing and checking the range 0-65536.
682682
* @return The local server port
683683
*/
684-
public int getServerPort() {
685-
int port = getHttpServletRequest().getServerPort();
686-
if ( port < 0 || port > 0xFFFF ) {
687-
logger.warning( Logger.SECURITY_FAILURE, "HTTP server port out of range: " + port );
688-
port = 0;
689-
}
690-
return port;
691-
}
692-
684+
public int getServerPort() {
685+
int port = getHttpServletRequest().getServerPort();
686+
if ( port < 0 || port > 0xFFFF ) {
687+
logger.warning( Logger.SECURITY_FAILURE, "HTTP server port out of range: " + port );
688+
port = 0;
689+
}
690+
return port;
691+
}
692+
693693

694694
/**
695695
* Returns the server path from the HttpServletRequest after canonicalizing
@@ -710,52 +710,44 @@ public String getServletPath() {
710710

711711
/**
712712
* Returns a session, creating it if necessary, and sets the HttpOnly flag
713-
* on the Session ID cookie.
713+
* on the Session ID cookie. The 'secure' flag is also set if the property
714+
* {@Code HttpUtilities.ForceSecureCookies} is set to {@Code true} in the <b>ESAPI.properties</b> file.
714715
* @return The current session
715716
*/
716717
public HttpSession getSession() {
717-
HttpSession session = getHttpServletRequest().getSession();
718-
719-
// send a new cookie header with HttpOnly on first and second responses
720-
if (ESAPI.securityConfiguration().getBooleanProp("HttpUtilities.ForceHttpOnlySession")) {
721-
if (session.getAttribute("HTTP_ONLY") == null) {
722-
session.setAttribute("HTTP_ONLY", "set");
723-
Cookie cookie = new Cookie(ESAPI.securityConfiguration().getStringProp("HttpUtilities.HttpSessionIdName"), session.getId());
724-
cookie.setPath( getHttpServletRequest().getContextPath() );
725-
cookie.setMaxAge(-1); // session cookie
726-
HttpServletResponse response = ESAPI.currentResponse();
727-
if (response != null) {
728-
ESAPI.currentResponse().addCookie(cookie);
729-
}
730-
}
731-
}
732-
return session;
718+
return getSession(true);
733719
}
734720

735721
/**
736-
* Returns a session, creating it if necessary, and sets the HttpOnly flag
737-
* on the Session ID cookie.
738-
* @param create Create a new session if one doesn't exist
722+
* Returns the current session associated with this request or, if there is no current session and
723+
* {@Code create} is {@Code true}, returns a new session and sets the HttpOnly flag on the session ID cookie.
724+
* The 'secure' flag is also set if the property {@Code HttpUtilities.ForceSecureCookies} is set to
725+
* {@Code true} in the <b>ESAPI.properties</b> file.
726+
* @param create If set to {@Code true}, create a new session if one doesn't exist, otherwise return {@Code null}
739727
* @return The current session
740728
*/
741729
public HttpSession getSession(boolean create) {
742730
HttpSession session = getHttpServletRequest().getSession(create);
731+
743732
if (session == null) {
744733
return null;
745734
}
746735

736+
SecurityConfiguration sc = ESAPI.securityConfiguration();
737+
747738
// send a new cookie header with HttpOnly on first and second responses
748-
if (ESAPI.securityConfiguration().getBooleanProp("HttpUtilities.ForceHttpOnlySession")) {
749-
if (session.getAttribute("HTTP_ONLY") == null) {
750-
session.setAttribute("HTTP_ONLY", "set");
751-
Cookie cookie = new Cookie(ESAPI.securityConfiguration().getStringProp("HttpUtilities.HttpSessionIdName"), session.getId());
752-
cookie.setMaxAge(-1); // session cookie
753-
cookie.setPath( getHttpServletRequest().getContextPath() );
754-
HttpServletResponse response = ESAPI.currentResponse();
755-
if (response != null) {
756-
ESAPI.currentResponse().addCookie(cookie);
757-
}
758-
}
739+
if ( sc.getBooleanProp("HttpUtilities.ForceHttpOnlySession") ) {
740+
if (session.getAttribute("HTTP_ONLY") == null) {
741+
session.setAttribute("HTTP_ONLY", "set");
742+
Cookie cookie = new Cookie( sc.getStringProp("HttpUtilities.HttpSessionIdName"), session.getId() );
743+
cookie.setMaxAge(-1); // session cookie
744+
cookie.setPath( getHttpServletRequest().getContextPath() );
745+
cookie.setSecure( sc.getBooleanProp("HttpUtilities.ForceSecureCookies") );
746+
HttpServletResponse response = ESAPI.currentResponse();
747+
if (response != null) {
748+
ESAPI.currentResponse().addCookie(cookie);
749+
}
750+
}
759751
}
760752
return session;
761753
}

0 commit comments

Comments
 (0)