honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix TestpageOverlay XSS Vulnerability (#2027) 

* Resolve XSS vulnerability

* Add changelog
This commit is contained in:
James Agnew 2020-08-09 07:06:11 -04:00 committed by GitHub
parent 63ef2ce006
commit 0de0b88aa0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 64 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
example-projects/hapi-fhir-jaxrs-sse/pom.xml
View File

@ -79,7 +79,7 @@
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>21.0</version>
<version>24.1.1</version>
</dependency>
<dependency>
<groupId>com.google.inject.extensions</groupId>

28
hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java
View File

@ -16,10 +16,16 @@ import java.io.UnsupportedEncodingException;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLDecoder;
import java.util.*;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.StringTokenizer;
import static org.apache.commons.lang3.StringUtils.*;
import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
import static org.apache.commons.lang3.StringUtils.defaultString;
import static org.apache.commons.lang3.StringUtils.isBlank;
/*
* #%L
@ -370,7 +376,7 @@ public class UrlUtil {
/**
* This method specifically HTML-encodes the &quot; and
* &lt; characters in order to prevent injection attacks.
*
* <p>
* The following characters are escaped:
* <ul>
* <li>&apos;</li>
@ -379,7 +385,6 @@ public class UrlUtil {
* <li>&gt;</li>
* <li>\n (newline)</li>
* </ul>
*
*/
public static String sanitizeUrlPart(CharSequence theString) {
if (theString == null) {
@ -432,6 +437,21 @@ public class UrlUtil {
return theString.toString();
}
/**
* Applies the same logic as {@link #sanitizeUrlPart(CharSequence)} but against an array, returning an array with the
* same strings as the input but with sanitization applied
*/
public static String[] sanitizeUrlPart(String[] theParameterValues) {
String[] retVal = null;
if (theParameterValues != null) {
retVal = new String[theParameterValues.length];
for (int i = 0; i < theParameterValues.length; i++) {
retVal[i] = sanitizeUrlPart(theParameterValues[i]);
}
}
return retVal;
}
private static Map<String, String[]> toQueryStringMap(HashMap<String, List<String>> map) {
HashMap<String, String[]> retVal = new HashMap<>();
for (Entry<String, List<String>> nextEntry : map.entrySet()) {

8
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/2026-resolve-xss-testpage-vulnerability.yaml Normal file
View File

@ -0,0 +1,8 @@
---
type: security
issue: 2026
title: "An XSS vulnerability was reported in the HAPI FHIR Testpage Overlay module. Thanks to Will Davison of NCC Group (Manchester UK) for disclosing this vulnerability.
Users of the HAPI FHIR Testpage Overlay can use a specially crafted URL to exploit an XSS vulnerability in this module, allowing arbitrary JavaScript to be executed in the user's browser. The impact of this vulnerability is believed to be low, as this module is intended for testing and not believed to be widely used for any production purposes. Nonetheless, we recommend all users of the affected module upgrade immediately.
A complete audit of the affected codebase has been completed in order to detect and resolve any similar issues."

47
hapi-fhir-testpage-overlay/src/main/java/ca/uhn/fhir/to/BaseController.java
View File

@ -41,6 +41,7 @@ import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.util.*;
import static ca.uhn.fhir.util.UrlUtil.sanitizeUrlPart;
import static org.apache.commons.lang3.StringUtils.defaultString;
public class BaseController {
@ -49,7 +50,7 @@ public class BaseController {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseController.class);
@Autowired
protected TesterConfig myConfig;
private Map<FhirVersionEnum, FhirContext> myContexts = new HashMap<FhirVersionEnum, FhirContext>();
private final Map<FhirVersionEnum, FhirContext> myContexts = new HashMap<FhirVersionEnum, FhirContext>();
private List<String> myFilterHeaders;
@Autowired
private ITemplateEngine myTemplateEngine;
@ -78,19 +79,6 @@ public class BaseController {
return loadAndAddConf(theServletRequest, theRequest, theModel);
}
private Header[] applyHeaderFilters(Header[] theAllHeaders) {
if (myFilterHeaders == null || myFilterHeaders.isEmpty()) {
return theAllHeaders;
}
ArrayList<Header> retVal = new ArrayList<Header>();
for (Header next : theAllHeaders) {
if (!myFilterHeaders.contains(next.getName().toLowerCase())) {
retVal.add(next);
}
}
return retVal.toArray(new Header[retVal.size()]);
}
private Header[] applyHeaderFilters(Map<String, List<String>> theAllHeaders) {
ArrayList<Header> retVal = new ArrayList<Header>();
for (String nextKey : theAllHeaders.keySet()) {
@ -274,7 +262,7 @@ public class BaseController {
}
protected RuntimeResourceDefinition getResourceType(HomeRequest theRequest, HttpServletRequest theReq) throws ServletException {
String resourceName = StringUtils.defaultString(theReq.getParameter(PARAM_RESOURCE));
String resourceName = sanitizeUrlPart(defaultString(theReq.getParameter(PARAM_RESOURCE)));
RuntimeResourceDefinition def = getContext(theRequest).getResourceDefinition(resourceName);
if (def == null) {
throw new ServletException("Invalid resourceName: " + resourceName);
@ -317,7 +305,7 @@ public class BaseController {
ca.uhn.fhir.model.dstu2.resource.Conformance conformance;
try {
conformance = (ca.uhn.fhir.model.dstu2.resource.Conformance) client.fetchConformance().ofType(Conformance.class).execute();
conformance = client.fetchConformance().ofType(Conformance.class).execute();
} catch (Exception e) {
ourLog.warn("Failed to load conformance statement, error was: {}", e.toString());
theModel.put("errorMsg", toDisplayError("Failed to load conformance statement, error was: " + e.toString(), e));
@ -326,7 +314,7 @@ public class BaseController {
theModel.put("jsonEncodedConf", getContext(theRequest).newJsonParser().encodeResourceToString(conformance));
Map<String, Number> resourceCounts = new HashMap<String, Number>();
Map<String, Number> resourceCounts = new HashMap<>();
long total = 0;
for (ca.uhn.fhir.model.dstu2.resource.Conformance.Rest nextRest : conformance.getRest()) {
for (ca.uhn.fhir.model.dstu2.resource.Conformance.RestResource nextResource : nextRest.getResource()) {
@ -385,7 +373,7 @@ public class BaseController {
theModel.put("jsonEncodedConf", getContext(theRequest).newJsonParser().encodeResourceToString(capabilityStatement));
Map<String, Number> resourceCounts = new HashMap<String, Number>();
Map<String, Number> resourceCounts = new HashMap<>();
long total = 0;
for (CapabilityStatementRestComponent nextRest : capabilityStatement.getRest()) {
@ -446,7 +434,7 @@ public class BaseController {
theModel.put("jsonEncodedConf", getContext(theRequest).newJsonParser().encodeResourceToString(capabilityStatement));
Map<String, Number> resourceCounts = new HashMap<String, Number>();
Map<String, Number> resourceCounts = new HashMap<>();
long total = 0;
for (org.hl7.fhir.r4.model.CapabilityStatement.CapabilityStatementRestComponent nextRest : capabilityStatement.getRest()) {
@ -507,7 +495,7 @@ public class BaseController {
theModel.put("jsonEncodedConf", getContext(theRequest).newJsonParser().encodeResourceToString(capabilityStatement));
Map<String, Number> resourceCounts = new HashMap<String, Number>();
Map<String, Number> resourceCounts = new HashMap<>();
long total = 0;
for (org.hl7.fhir.r5.model.CapabilityStatement.CapabilityStatementRestComponent nextRest : capabilityStatement.getRest()) {
@ -614,25 +602,6 @@ public class BaseController {
protected void processAndAddLastClientInvocation(GenericClient theClient, ResultType theResultType, ModelMap theModelMap, long theLatency, String outcomeDescription,
CaptureInterceptor theInterceptor, HomeRequest theRequest) {
try {
// ApacheHttpRequest lastRequest = theInterceptor.getLastRequest();
// HttpResponse lastResponse = theInterceptor.getLastResponse();
// String requestBody = null;
// String requestUrl = lastRequest != null ? lastRequest.getApacheRequest().getURI().toASCIIString() : null;
// String action = lastRequest != null ? lastRequest.getApacheRequest().getMethod() : null;
// String resultStatus = lastResponse != null ? lastResponse.getStatusLine().toString() : null;
// String resultBody = StringUtils.defaultString(theInterceptor.getLastResponseBody());
//
// if (lastRequest instanceof HttpEntityEnclosingRequest) {
// HttpEntity entity = ((HttpEntityEnclosingRequest) lastRequest).getEntity();
// if (entity.isRepeatable()) {
// requestBody = IOUtils.toString(entity.getContent());
// }
// }
//
// ContentType ct = lastResponse != null ? ContentType.get(lastResponse.getEntity()) : null;
// String mimeType = ct != null ? ct.getMimeType() : null;
IHttpRequest lastRequest = theInterceptor.getLastRequest();
IHttpResponse lastResponse = theInterceptor.getLastResponse();
String requestBody = null;

39
hapi-fhir-testpage-overlay/src/main/java/ca/uhn/fhir/to/Controller.java
View File

@ -26,6 +26,7 @@ import ca.uhn.fhir.rest.gclient.TokenClientParam;
import ca.uhn.fhir.to.model.HomeRequest;
import ca.uhn.fhir.to.model.ResourceRequest;
import ca.uhn.fhir.to.model.TransactionRequest;
import ca.uhn.fhir.util.UrlUtil;
import com.google.gson.stream.JsonWriter;
import org.apache.commons.lang3.StringUtils;
import org.hl7.fhir.dstu3.model.CapabilityStatement;
@ -49,6 +50,7 @@ import java.util.Collections;
import java.util.List;
import java.util.TreeSet;
import static ca.uhn.fhir.util.UrlUtil.sanitizeUrlPart;
import static org.apache.commons.lang3.StringUtils.defaultIfEmpty;
import static org.apache.commons.lang3.StringUtils.defaultString;
import static org.apache.commons.lang3.StringUtils.isBlank;
@ -128,7 +130,7 @@ public class Controller extends BaseController {
return "resource";
}
String id = StringUtils.defaultString(theServletRequest.getParameter("resource-delete-id"));
String id = sanitizeUrlPart(defaultString(theServletRequest.getParameter("resource-delete-id")));
if (StringUtils.isBlank(id)) {
populateModelForResource(theServletRequest, theRequest, theModel);
theModel.put("errorMsg", toDisplayError("No ID specified", null));
@ -184,7 +186,7 @@ public class Controller extends BaseController {
FhirContext context = getContext(theRequest);
GenericClient client = theRequest.newClient(theReq, context, myConfig, interceptor);
String url = defaultString(theReq.getParameter("page-url"));
String url = sanitizeUrlPart(defaultString(theReq.getParameter("page-url")));
if (myConfig.isRefuseToFetchThirdPartyUrls()) {
if (!url.startsWith(theModel.get("base").toString())) {
ourLog.warn(logPrefix(theModel) + "Refusing to load page URL: {}", url);
@ -230,7 +232,7 @@ public class Controller extends BaseController {
theModel.put("errorMsg", toDisplayError(e.toString(), e));
return "resource";
}
String id = StringUtils.defaultString(theServletRequest.getParameter("id"));
String id = sanitizeUrlPart(defaultString(theServletRequest.getParameter("id")));
if (StringUtils.isBlank(id)) {
populateModelForResource(theServletRequest, theRequest, theModel);
theModel.put("errorMsg", toDisplayError("No ID specified", null));
@ -238,7 +240,7 @@ public class Controller extends BaseController {
}
ResultType returnsResource = ResultType.RESOURCE;
String versionId = StringUtils.defaultString(theServletRequest.getParameter("vid"));
String versionId = sanitizeUrlPart(defaultString(theServletRequest.getParameter("vid")));
String outcomeDescription;
if (StringUtils.isBlank(versionId)) {
versionId = null;
@ -353,7 +355,7 @@ public class Controller extends BaseController {
return "resource";
}
clientCodeJsonWriter.name("resource");
clientCodeJsonWriter.value(theServletRequest.getParameter("resource"));
clientCodeJsonWriter.value(sanitizeUrlPart(theServletRequest.getParameter("resource")));
} else {
query = search.forAllResources();
clientCodeJsonWriter.name("resource");
@ -394,7 +396,7 @@ public class Controller extends BaseController {
clientCodeJsonWriter.name("includes");
clientCodeJsonWriter.beginArray();
String[] incValues = theServletRequest.getParameterValues(Constants.PARAM_INCLUDE);
String[] incValues = sanitizeUrlPart(theServletRequest.getParameterValues(Constants.PARAM_INCLUDE));
if (incValues != null) {
for (String next : incValues) {
if (isNotBlank(next)) {
@ -407,7 +409,7 @@ public class Controller extends BaseController {
clientCodeJsonWriter.name("revincludes");
clientCodeJsonWriter.beginArray();
String[] revIncValues = theServletRequest.getParameterValues(Constants.PARAM_REVINCLUDE);
String[] revIncValues = sanitizeUrlPart(theServletRequest.getParameterValues(Constants.PARAM_REVINCLUDE));
if (revIncValues != null) {
for (String next : revIncValues) {
if (isNotBlank(next)) {
@ -418,7 +420,7 @@ public class Controller extends BaseController {
}
clientCodeJsonWriter.endArray();
String limit = theServletRequest.getParameter("resource-search-limit");
String limit = sanitizeUrlPart(theServletRequest.getParameter("resource-search-limit"));
if (isNotBlank(limit)) {
if (!limit.matches("[0-9]+")) {
populateModelForResource(theServletRequest, theRequest, theModel);
@ -434,13 +436,13 @@ public class Controller extends BaseController {
clientCodeJsonWriter.nullValue();
}
String[] sort = theServletRequest.getParameterValues("sort_by");
String[] sort = sanitizeUrlPart(theServletRequest.getParameterValues("sort_by"));
if (sort != null) {
for (String next : sort) {
if (isBlank(next)) {
continue;
}
String direction = theServletRequest.getParameter("sort_direction");
String direction = sanitizeUrlPart(theServletRequest.getParameter("sort_direction"));
if ("asc".equals(direction)) {
query.sort().ascending(new StringClientParam(next));
} else if ("desc".equals(direction)) {
@ -545,6 +547,7 @@ public class Controller extends BaseController {
type = def.getImplementingClass();
}
// Don't sanitize this param, it's a raw resource body and may well be XML
String body = validate ? theReq.getParameter("resource-validate-body") : theReq.getParameter("resource-create-body");
if (isBlank(body)) {
theModel.put("errorMsg", toDisplayError("No message body specified", null));
@ -583,7 +586,7 @@ public class Controller extends BaseController {
outcomeDescription = "Validate Resource";
client.validate().resource(resource).prettyPrint().execute();
} else {
String id = theReq.getParameter("resource-create-id");
String id = sanitizeUrlPart(theReq.getParameter("resource-create-id"));
if ("update".equals(theMethod)) {
outcomeDescription = "Update Resource";
client.update(id, resource);
@ -626,17 +629,17 @@ public class Controller extends BaseController {
if ("history-type".equals(theMethod)) {
RuntimeResourceDefinition def = getContext(theRequest).getResourceDefinition(theRequest.getResource());
type = def.getImplementingClass();
id = StringUtils.defaultString(theReq.getParameter("resource-history-id"));
id = sanitizeUrlPart(defaultString(theReq.getParameter("resource-history-id")));
}
DateTimeDt since = null;
String sinceStr = theReq.getParameter("since");
String sinceStr = sanitizeUrlPart(theReq.getParameter("since"));
if (isNotBlank(sinceStr)) {
since = new DateTimeDt(sinceStr);
}
Integer limit = null;
String limitStr = theReq.getParameter("limit");
String limitStr = sanitizeUrlPart(theReq.getParameter("limit"));
if (isNotBlank(limitStr)) {
limit = Integer.parseInt(limitStr);
}
@ -811,17 +814,17 @@ public class Controller extends BaseController {
}
private boolean handleSearchParam(String paramIdxString, HttpServletRequest theReq, IQuery theQuery, JsonWriter theClientCodeJsonWriter) throws IOException {
String nextName = theReq.getParameter("param." + paramIdxString + ".name");
String nextName = sanitizeUrlPart(theReq.getParameter("param." + paramIdxString + ".name"));
if (isBlank(nextName)) {
return false;
}
String nextQualifier = StringUtils.defaultString(theReq.getParameter("param." + paramIdxString + ".qualifier"));
String nextType = theReq.getParameter("param." + paramIdxString + ".type");
String nextQualifier = sanitizeUrlPart(defaultString(theReq.getParameter("param." + paramIdxString + ".qualifier")));
String nextType = sanitizeUrlPart(theReq.getParameter("param." + paramIdxString + ".type"));
List<String> parts = new ArrayList<String>();
for (int i = 0; i < 5; i++) {
parts.add(defaultString(theReq.getParameter("param." + paramIdxString + "." + i)));
parts.add(sanitizeUrlPart(defaultString(theReq.getParameter("param." + paramIdxString + "." + i))));
}
List<String> values;

4
hapi-fhir-testpage-overlay/src/test/java/ca/uhn/fhir/jpa/test/FhirTesterConfig.java
View File

@ -48,13 +48,13 @@ public class FhirTesterConfig {
.addServer()
.withId("hapi")
.withFhirVersion(FhirVersionEnum.DSTU2)
.withBaseUrl("http://fhirtest.uhn.ca/baseDstu2")
.withBaseUrl("http://hapi.fhir.org/baseDstu2")
.withName("Public HAPI Test Server")
.allowsApiKey()
.addServer()
.withId("home3")
.withFhirVersion(FhirVersionEnum.DSTU3)
.withBaseUrl("http://fhirtest.uhn.ca/baseDstu3")
.withBaseUrl("http://hapi.fhir.org/baseDstu3")
.withName("Public HAPI Test Server (STU3)")
.addServer()
.withId("home")