Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UP-3315] Fix some XSS issue around inputting usernames. The issue was ... #379

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions uportal-war/src/main/java/org/jasig/portal/utils/jsp/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@
package org.jasig.portal.utils.jsp;

import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.util.Collection;
import java.util.Map;

import org.jasig.portal.spring.beans.factory.ObjectMapperFactoryBean;

import com.fasterxml.jackson.core.JsonGenerationException;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.jasig.portal.spring.beans.factory.ObjectMapperFactoryBean;
import org.springframework.web.util.UriUtils;


/**
* JSP Static utility functions
Expand Down Expand Up @@ -74,4 +76,45 @@ public static boolean instanceOf(Object obj, String className) throws ClassNotFo
public static String json(Object obj) throws JsonGenerationException, JsonMappingException, IOException {
return OBJECT_MAPPER.writeValueAsString(obj);
}


/**
* URL encode a path segment. This is just a thin wrapper around UriUtils.encodePathSegment. It is intended
* for the case where you are building URLs in JS. c:url + escapeBody doesn't correctly escape
* the contents (especially "</script>"), and fn:escapeXml incorrectly encodes the URL (it escapes chars
* like '<' as &lt; instead of %3C). It should help avoid XSS attacks when building RESTful
* URLS in js. Example:
*
* Given: ${userId} -> "<script>alert('test')</script>
*
* ...
* <script>
* $.ajax({ url: <c:url value='/users/${up:encodePathSegment(userId)}'/> });
* </script>
*
* Will encode the URL as:
*
* /users/%3Cscript%3Ealert('test%')%3C%2Fscript%3E
*
* IMPORTANT:
* Note that this encodes the '/' in </script> to %2F. Unfortunately, tomcat
* still does not interpret %2F correctly unless you relax some security
* settings (@see tomcat.apache.org/tomcat-7.0-doc/config/systemprops.html#Security,
* the ALLOW_ENCODED_SLASH property). So, while this method does a better job
* at avoiding XSS issues than c:url, it's still not ideal. Unless the
* input is whitelisted to avoid invalid input chars, it's still possible to
* end up with REST URLs that won't work correctly (like the one above), but at
* least this will protect you from XSS attacks on the front end.
*
* @param val the path segment to encode
* @return the encoded path segment
*/
public static String encodePathSegment(String val) {
try {
return UriUtils.encodePathSegment(val, "UTF-8");
} catch (UnsupportedEncodingException e) {
// should be unreachable...
throw new RuntimeException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ up.jQuery(function() {

var pager;
var editUrl = "${editUrl}";
var targetUrl = "<c:url value="/api/assignments/target/${ principalString }.json?includeInherited=true"/>";
var principalUrl = "<c:url value="/api/assignments/principal/${ principalString }.json?includeInherited=true"/>";
var targetUrl = "<c:url value="/api/assignments/target/${ up:encodePathSegment(principalString) }.json?includeInherited=true"/>";
var principalUrl = "<c:url value="/api/assignments/principal/${ up:encodePathSegment(principalString) }.json?includeInherited=true"/>";

var getPermissionAssignments = function(url) {
var rslt;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ PORTLET DEVELOPMENT STANDARDS AND GUIDELINES
var $ = up.jQuery;

var pager;
var targetUrl = "<c:url value="/api/assignments/target/${ personEntity.principalString }.json?includeInherited=true"/>";
var principalUrl = "<c:url value="/api/assignments/principal/${ personEntity.principalString }.json?includeInherited=true"/>";
var targetUrl = "<c:url value="/api/assignments/target/${ up:encodePathSegment(personEntity.principalString) }.json?includeInherited=true"/>";
var principalUrl = "<c:url value="/api/assignments/principal/${ up:encodePathSegment(personEntity.principalString) }.json?includeInherited=true"/>";
var editUrl = "${editUrl}";
var deleteUrl = "${deleteUrl}";

Expand Down
12 changes: 12 additions & 0 deletions uportal-war/src/main/webapp/WEB-INF/tag/uportal.tld
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,16 @@
${up:json(obj)}
</example>
</function>
<function>
<description>
URL encodes a single path segment. Intended for building RESTful URLs where
variable values may contain illegal characters.
</description>
<name>encodePathSegment</name>
<function-class>org.jasig.portal.utils.jsp.Util</function-class>
<function-signature>java.lang.String encodePathSegment(java.lang.String)</function-signature>
<example>
${up:urlEncode("www.example.com/image.png")}
</example>
</function>
</taglib>