Ensure values of _s are encoded

Fixes gh-80
This commit is contained in:
Rob Winch
2014-12-17 17:37:29 -06:00
parent 375db97f1c
commit ddff1f4c0d
3 changed files with 84 additions and 5 deletions

View File

@@ -41,7 +41,10 @@
<li class="divider"></li>
</c:if>
<c:forEach items="${accounts}" var="account">
<li><a id="switchAccount${account.username}" href="${account.switchAccountUrl}"><c:out value="${account.username}"/></a></li>
<c:set var="encodedUsername">
<c:out value="${account.username}"/>
</c:set>
<li><a id="switchAccount${encodedUsername}" href="${account.switchAccountUrl}"><c:out value="${account.username}"/></a></li>
</c:forEach>
</ul>
</li>

View File

@@ -15,9 +15,12 @@
*/
package org.springframework.session.web.http;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.StringTokenizer;
import java.util.regex.Pattern;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
@@ -151,6 +154,8 @@ public final class CookieHttpSessionStrategy implements MultiHttpSessionStrategy
static final String DEFAULT_SESSION_ALIAS_PARAM_NAME = "_s";
private Pattern ALIAS_PATTERN = Pattern.compile("^[\\w-]{1,50}$");
private String cookieName = "SESSION";
private String sessionParam = DEFAULT_SESSION_ALIAS_PARAM_NAME;
@@ -169,6 +174,9 @@ public final class CookieHttpSessionStrategy implements MultiHttpSessionStrategy
if(u == null) {
return DEFAULT_ALIAS;
}
if(!ALIAS_PATTERN.matcher(u).matches()) {
return DEFAULT_ALIAS;
}
return u;
}
@@ -324,23 +332,32 @@ public final class CookieHttpSessionStrategy implements MultiHttpSessionStrategy
}
public String encodeURL(String url, String sessionAlias) {
String encodedSessionAlias = urlEncode(sessionAlias);
int queryStart = url.indexOf("?");
boolean isDefaultAlias = DEFAULT_ALIAS.equals(sessionAlias);
boolean isDefaultAlias = DEFAULT_ALIAS.equals(encodedSessionAlias);
if(queryStart < 0) {
return isDefaultAlias ? url : url + "?" + sessionParam + "=" + sessionAlias;
return isDefaultAlias ? url : url + "?" + sessionParam + "=" + encodedSessionAlias;
}
String path = url.substring(0, queryStart);
String query = url.substring(queryStart + 1, url.length());
String replacement = isDefaultAlias ? "" : "$1"+sessionAlias;
String replacement = isDefaultAlias ? "" : "$1"+encodedSessionAlias;
query = query.replaceFirst( "((^|&)" + sessionParam + "=)([^&]+)?", replacement);
if(!isDefaultAlias && url.endsWith(query)) {
// no existing alias
if(!(query.endsWith("&") || query.length() == 0)) {
query += "&";
}
query += sessionParam + "=" + sessionAlias;
query += sessionParam + "=" + encodedSessionAlias;
}
return path + "?" + query;
}
private String urlEncode(String value) {
try {
return URLEncoder.encode(value, "UTF-8");
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
}
}
}

View File

@@ -210,6 +210,11 @@ public class CookieHttpSessionStrategyTests {
assertThat(strategy.encodeURL("/url?a=b&_s=1", "0")).isEqualTo("/url?a=b");
}
@Test
public void encodeURLMaliciousAlias() {
assertThat(strategy.encodeURL("/url?a=b&_s=1", "\"> <script>alert('hi')</script>")).isEqualTo("/url?a=b&_s=%22%3E+%3Cscript%3Ealert%28%27hi%27%29%3C%2Fscript%3E");
}
// --- getCurrentSessionAlias
@Test
@@ -225,6 +230,60 @@ public class CookieHttpSessionStrategyTests {
assertThat(strategy.getCurrentSessionAlias(request)).isEqualTo(CookieHttpSessionStrategy.DEFAULT_ALIAS);
}
// protect against malicious users
@Test
public void getCurrentSessionAliasContainsQuote() {
request.setParameter(CookieHttpSessionStrategy.DEFAULT_SESSION_ALIAS_PARAM_NAME, "here\"this");
assertThat(strategy.getCurrentSessionAlias(request)).isEqualTo(CookieHttpSessionStrategy.DEFAULT_ALIAS);
}
@Test
public void getCurrentSessionAliasContainsSingleQuote() {
request.setParameter(CookieHttpSessionStrategy.DEFAULT_SESSION_ALIAS_PARAM_NAME, "here'this");
assertThat(strategy.getCurrentSessionAlias(request)).isEqualTo(CookieHttpSessionStrategy.DEFAULT_ALIAS);
}
@Test
public void getCurrentSessionAliasContainsSpace() {
request.setParameter(CookieHttpSessionStrategy.DEFAULT_SESSION_ALIAS_PARAM_NAME, "here this");
assertThat(strategy.getCurrentSessionAlias(request)).isEqualTo(CookieHttpSessionStrategy.DEFAULT_ALIAS);
}
@Test
public void getCurrentSessionAliasContainsLt() {
request.setParameter(CookieHttpSessionStrategy.DEFAULT_SESSION_ALIAS_PARAM_NAME, "here<this");
assertThat(strategy.getCurrentSessionAlias(request)).isEqualTo(CookieHttpSessionStrategy.DEFAULT_ALIAS);
}
@Test
public void getCurrentSessionAliasContainsGt() {
strategy.setSessionAliasParamName(null);
request.setParameter(CookieHttpSessionStrategy.DEFAULT_SESSION_ALIAS_PARAM_NAME, "here>this");
assertThat(strategy.getCurrentSessionAlias(request)).isEqualTo(CookieHttpSessionStrategy.DEFAULT_ALIAS);
}
@Test
public void getCurrentSessionAliasTooLong() {
request.setParameter(CookieHttpSessionStrategy.DEFAULT_SESSION_ALIAS_PARAM_NAME, "012345678901234567890123456789012345678901234567890");
assertThat(strategy.getCurrentSessionAlias(request)).isEqualTo(CookieHttpSessionStrategy.DEFAULT_ALIAS);
}
// We want some sort of length restrictions, but want to ensure some sort of length Technically no hard limit, but chose 50
@Test
public void getCurrentSessionAliasAllows50() {
request.setParameter(CookieHttpSessionStrategy.DEFAULT_SESSION_ALIAS_PARAM_NAME, "01234567890123456789012345678901234567890123456789");
assertThat(strategy.getCurrentSessionAlias(request)).isEqualTo("01234567890123456789012345678901234567890123456789");
}
@Test
public void getCurrentSession() {
String expectedAlias = "1";