diff --git a/openid-connect-common/pom.xml b/openid-connect-common/pom.xml
index bc910d1cd..79123a854 100644
--- a/openid-connect-common/pom.xml
+++ b/openid-connect-common/pom.xml
@@ -87,6 +87,19 @@
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.jsoup</groupId>
+ <artifactId>jsoup</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-all</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.spockframework</groupId>
+ <artifactId>spock-core</artifactId>
+ </dependency>
</dependencies>
<packaging>jar</packaging>
@@ -101,6 +114,37 @@
<target>${java-version}</target>
</configuration>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.gmavenplus</groupId>
+ <artifactId>gmavenplus-plugin</artifactId>
+ <version>1.7.0</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>addTestStubSources</goal>
+ <goal>compileTests</goal>
+ <goal>removeTestStubs</goal>
+ </goals>
+ </execution>
+ </executions>
+ <configuration>
+ <stubsOutputDirectory>${project.build.directory}/generated-groovy-stubs</stubsOutputDirectory>
+ <testStubsOutputDirectory>${project.build.directory}/generated-groovy-test-stubs</testStubsOutputDirectory>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>2.18.1</version>
+ <configuration>
+ <excludedGroups combine.self="override"/>
+ <testClassesDirectory>${project.build.testOutputDirectory}</testClassesDirectory>
+ <includes>
+ <include>**/*Test.java</include>
+ <include>**/*Spec.java</include>
+ </includes>
+ </configuration>
+ </plugin>
<!--<!– BUILD SOURCE FILES –>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
diff --git a/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java b/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java
index 584b435ff..43d9c8591 100644
--- a/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java
+++ b/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java
@@ -25,6 +25,9 @@ import java.lang.reflect.Type;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.jsoup.Jsoup;
+import org.jsoup.safety.Whitelist;
+import org.mitre.openid.connect.model.Address;
import org.mitre.openid.connect.model.OIDCAuthenticationToken;
import org.mitre.openid.connect.model.UserInfo;
import org.mitre.openid.connect.service.UserInfoService;
@@ -78,9 +81,11 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter {
if (auth instanceof OIDCAuthenticationToken) {
// if they're logging into this server from a remote OIDC server, pass through their user info
OIDCAuthenticationToken oidc = (OIDCAuthenticationToken) auth;
- if (oidc.getUserInfo() != null) {
- request.setAttribute("userInfo", oidc.getUserInfo());
- request.setAttribute("userInfoJson", oidc.getUserInfo().toJson());
+ UserInfo userInfo = oidc.getUserInfo();
+ if (userInfo != null) {
+ santiseUserInfo(userInfo);
+ request.setAttribute("userInfo", userInfo);
+ request.setAttribute("userInfoJson", userInfo.toJson());
} else {
request.setAttribute("userInfo", null);
request.setAttribute("userInfoJson", "null");
@@ -94,6 +99,7 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter {
// if we have one, inject it so views can use it
if (user != null) {
+ santiseUserInfo(user);
request.setAttribute("userInfo", user);
request.setAttribute("userInfoJson", user.toJson());
}
@@ -104,4 +110,47 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter {
return true;
}
+ private UserInfo santiseUserInfo(final UserInfo userInfo) {
+ userInfo.setSub(nullCheckClean(userInfo.getSub()));
+ userInfo.setPreferredUsername(nullCheckClean(userInfo.getPreferredUsername()));
+ userInfo.setName(nullCheckClean(userInfo.getName()));
+ userInfo.setGivenName(nullCheckClean(userInfo.getGivenName()));
+ userInfo.setFamilyName(nullCheckClean(userInfo.getFamilyName()));
+ userInfo.setMiddleName(nullCheckClean(userInfo.getMiddleName()));
+ userInfo.setNickname(nullCheckClean(userInfo.getNickname()));
+ userInfo.setProfile(nullCheckClean(userInfo.getProfile()));
+ userInfo.setPicture(nullCheckClean(userInfo.getPicture()));
+ userInfo.setWebsite(nullCheckClean(userInfo.getWebsite()));
+ userInfo.setEmail(nullCheckClean(userInfo.getEmail()));
+ userInfo.setGender(nullCheckClean(userInfo.getGender()));
+ userInfo.setLocale(nullCheckClean(userInfo.getLocale()));
+ userInfo.setPhoneNumber(nullCheckClean(userInfo.getPhoneNumber()));
+ userInfo.setUpdatedTime(nullCheckClean(userInfo.getUpdatedTime()));
+ userInfo.setBirthdate(nullCheckClean(userInfo.getBirthdate()));
+
+ Address userInfoAddress = userInfo.getAddress();
+ if (userInfoAddress != null) {
+ userInfoAddress.setFormatted(nullCheckClean(userInfoAddress.getFormatted()));
+ userInfoAddress.setStreetAddress(nullCheckClean(userInfoAddress.getStreetAddress()));
+ userInfoAddress.setLocality(nullCheckClean(userInfoAddress.getLocality()));
+ userInfoAddress.setRegion(nullCheckClean(userInfoAddress.getRegion()));
+ userInfoAddress.setPostalCode(nullCheckClean(userInfoAddress.getPostalCode()));
+ userInfoAddress.setCountry(nullCheckClean(userInfoAddress.getCountry()));
+ userInfo.setAddress(userInfoAddress);
+ }
+
+ return userInfo;
+ }
+
+ private String nullCheckClean(String elementToClean) {
+ final Whitelist whitelist = Whitelist.relaxed()
+ .removeTags("a")
+ .removeProtocols("img", "src", "http", "https");
+
+ if (elementToClean != null) {
+ return Jsoup.clean(elementToClean, whitelist);
+ }
+ return null;
+ }
+
}
diff --git a/openid-connect-common/src/test/groovy/org/mitre/openid/connect/web/UserInfoInterceptorSpec.groovy b/openid-connect-common/src/test/groovy/org/mitre/openid/connect/web/UserInfoInterceptorSpec.groovy
new file mode 100644
index 000000000..9bea3e7c0
--- /dev/null
+++ b/openid-connect-common/src/test/groovy/org/mitre/openid/connect/web/UserInfoInterceptorSpec.groovy
@@ -0,0 +1,41 @@
+package org.mitre.openid.connect.web
+
+import org.mitre.openid.connect.model.DefaultUserInfo
+import org.mitre.openid.connect.model.UserInfo
+import spock.lang.Specification
+
+class UserInfoInterceptorSpec extends Specification {
+
+ private def userInfoInterceptor = new UserInfoInterceptor()
+
+ // CVE-2020-5497 -> https://github.com/mitreid-connect/OpenID-Connect-Java-Spring-Server/issues/1521
+ def 'User Info is sanitised before making it back to the webpage'() {
+ given: 'A user name with a malicious payload'
+
+ UserInfo userInfo = new DefaultUserInfo()
+ userInfo.setSub('12318767')
+ userInfo.setName("Test</script><script>alert(1)</script> Test")
+ userInfo.setPreferredUsername('Test')
+ userInfo.setGivenName("Test</script><script>alert(1)</script>")
+ userInfo.setFamilyName('Test')
+ userInfo.setEmail('test@test.com')
+ userInfo.setEmailVerified(true)
+
+ when: 'The user info object is passed through the sanitise method'
+
+ UserInfo sanitisedUserInfo = userInfoInterceptor.santiseUserInfo(userInfo)
+
+ then: 'The malicious names have been sanitised'
+
+ sanitisedUserInfo.getName() == 'Test Test'
+ sanitisedUserInfo.getGivenName() == 'Test'
+
+ and: 'The non malicious elements have been unaffected'
+
+ sanitisedUserInfo.getSub() == '12318767'
+ sanitisedUserInfo.getPreferredUsername() == 'Test'
+ sanitisedUserInfo.getFamilyName() == 'Test'
+ sanitisedUserInfo.getEmail() == 'test@test.com'
+ }
+
+}
diff --git a/pom.xml b/pom.xml
index 1347b72e2..a11af7e5c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -520,6 +520,24 @@
<version>1.9.5</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-all</artifactId>
+ <version>2.4.13</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.spockframework</groupId>
+ <artifactId>spock-core</artifactId>
+ <version>1.1-groovy-2.4</version>
+ <scope>test</scope>
+ <exclusions>
+ <exclusion>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-all</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
<!-- MITREid Connect components -->
<dependency>
<groupId>org.mitre</groupId>
@@ -607,6 +625,11 @@
<artifactId>wro4j-extensions</artifactId>
<version>1.8.0</version>
</dependency>
+ <dependency>
+ <groupId>org.jsoup</groupId>
+ <artifactId>jsoup</artifactId>
+ <version>1.10.3</version>
+ </dependency>
</dependencies>
</dependencyManagement>