Add protection against HTML injection attacks
This commit is contained in:
parent
83b8abf75e
commit
b4aa4c0e89
|
@ -9,9 +9,9 @@ package ca.uhn.fhir.util;
|
|||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
|
@ -34,8 +34,17 @@ public class UrlPathTokenizer {
|
|||
return myTok.hasMoreTokens();
|
||||
}
|
||||
|
||||
public String nextToken() {
|
||||
return UrlUtil.unescape(myTok.nextToken());
|
||||
/**
|
||||
* Returns the next portion. Any URL-encoding is undone, but we will
|
||||
* HTML encode the < and " marks since they are both
|
||||
* not useful un URL paths in FHIR and potentially represent injection
|
||||
* attacks.
|
||||
*
|
||||
* @see UrlUtil#sanitizeUrlPart(String)
|
||||
* @see UrlUtil#unescape(String)
|
||||
*/
|
||||
public String nextTokenUnescapedAndSanitized() {
|
||||
return UrlUtil.sanitizeUrlPart(UrlUtil.unescape(myTok.nextToken()));
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -25,9 +25,9 @@ import static org.apache.commons.lang3.StringUtils.isBlank;
|
|||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
|
@ -70,7 +70,7 @@ public class UrlUtil {
|
|||
return theExtensionUrl;
|
||||
}
|
||||
if (theExtensionUrl == null) {
|
||||
return theExtensionUrl;
|
||||
return null;
|
||||
}
|
||||
|
||||
int parentLastSlashIdx = theParentExtensionUrl.lastIndexOf('/');
|
||||
|
@ -119,6 +119,18 @@ public class UrlUtil {
|
|||
return value.startsWith("http://") || value.startsWith("https://");
|
||||
}
|
||||
|
||||
public static boolean isNeedsSanitization(String theString) {
|
||||
if (theString != null) {
|
||||
for (int i = 0; i < theString.length(); i++) {
|
||||
char nextChar = theString.charAt(i);
|
||||
if (nextChar == '<' || nextChar == '"') {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
public static boolean isValid(String theUrl) {
|
||||
if (theUrl == null || theUrl.length() < 8) {
|
||||
return false;
|
||||
|
@ -164,7 +176,7 @@ public class UrlUtil {
|
|||
}
|
||||
|
||||
public static Map<String, String[]> parseQueryString(String theQueryString) {
|
||||
HashMap<String, List<String>> map = new HashMap<String, List<String>>();
|
||||
HashMap<String, List<String>> map = new HashMap<>();
|
||||
parseQueryString(theQueryString, map);
|
||||
return toQueryStringMap(map);
|
||||
}
|
||||
|
@ -197,17 +209,13 @@ public class UrlUtil {
|
|||
nextKey = unescape(nextKey);
|
||||
nextValue = unescape(nextValue);
|
||||
|
||||
List<String> list = map.get(nextKey);
|
||||
if (list == null) {
|
||||
list = new ArrayList<>();
|
||||
map.put(nextKey, list);
|
||||
}
|
||||
List<String> list = map.computeIfAbsent(nextKey, k -> new ArrayList<>());
|
||||
list.add(nextValue);
|
||||
}
|
||||
}
|
||||
|
||||
public static Map<String, String[]> parseQueryStrings(String... theQueryString) {
|
||||
HashMap<String, List<String>> map = new HashMap<String, List<String>>();
|
||||
HashMap<String, List<String>> map = new HashMap<>();
|
||||
for (String next : theQueryString) {
|
||||
parseQueryString(next, map);
|
||||
}
|
||||
|
@ -222,7 +230,6 @@ public class UrlUtil {
|
|||
* <li>[Resource Type]/[Resource ID]/_history/[Version ID]
|
||||
* </ul>
|
||||
*/
|
||||
//@formatter:on
|
||||
public static UrlParts parseUrl(String theUrl) {
|
||||
String url = theUrl;
|
||||
UrlParts retVal = new UrlParts();
|
||||
|
@ -243,7 +250,7 @@ public class UrlUtil {
|
|||
retVal.setVersionId(id.getVersionIdPart());
|
||||
return retVal;
|
||||
}
|
||||
if (url.matches("\\/[a-zA-Z]+\\?.*")) {
|
||||
if (url.matches("/[a-zA-Z]+\\?.*")) {
|
||||
url = url.substring(1);
|
||||
}
|
||||
int nextStart = 0;
|
||||
|
@ -282,12 +289,47 @@ public class UrlUtil {
|
|||
|
||||
}
|
||||
|
||||
//@formatter:off
|
||||
/**
|
||||
* This method specifically HTML-encodes the " and
|
||||
* < characters in order to prevent injection attacks
|
||||
*/
|
||||
public static String sanitizeUrlPart(String theString) {
|
||||
if (theString == null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
boolean needsSanitization = isNeedsSanitization(theString);
|
||||
|
||||
if (needsSanitization) {
|
||||
// Ok, we're sanitizing
|
||||
StringBuilder buffer = new StringBuilder(theString.length() + 10);
|
||||
for (int j = 0; j < theString.length(); j++) {
|
||||
|
||||
char nextChar = theString.charAt(j);
|
||||
switch (nextChar) {
|
||||
case '"':
|
||||
buffer.append(""");
|
||||
break;
|
||||
case '<':
|
||||
buffer.append("<");
|
||||
break;
|
||||
default:
|
||||
buffer.append(nextChar);
|
||||
break;
|
||||
}
|
||||
|
||||
} // for build escaped string
|
||||
|
||||
return buffer.toString();
|
||||
}
|
||||
|
||||
return theString;
|
||||
}
|
||||
|
||||
private static Map<String, String[]> toQueryStringMap(HashMap<String, List<String>> map) {
|
||||
HashMap<String, String[]> retVal = new HashMap<String, String[]>();
|
||||
HashMap<String, String[]> retVal = new HashMap<>();
|
||||
for (Entry<String, List<String>> nextEntry : map.entrySet()) {
|
||||
retVal.put(nextEntry.getKey(), nextEntry.getValue().toArray(new String[nextEntry.getValue().size()]));
|
||||
retVal.put(nextEntry.getKey(), nextEntry.getValue().toArray(new String[0]));
|
||||
}
|
||||
return retVal;
|
||||
}
|
||||
|
|
|
@ -10,7 +10,6 @@ import org.springframework.data.jpa.repository.Modifying;
|
|||
import org.springframework.data.jpa.repository.Query;
|
||||
import org.springframework.data.repository.query.Param;
|
||||
|
||||
import java.util.Date;
|
||||
import java.util.List;
|
||||
|
||||
/*
|
||||
|
@ -22,9 +21,9 @@ import java.util.List;
|
|||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
|
@ -38,20 +37,6 @@ public interface ITermConceptDao extends JpaRepository<TermConcept, Long> {
|
|||
@Query("SELECT COUNT(t) FROM TermConcept t WHERE t.myCodeSystem.myId = :cs_pid")
|
||||
Integer countByCodeSystemVersion(@Param("cs_pid") Long thePid);
|
||||
|
||||
/**
|
||||
* Used in Smile CDR - Do not delete
|
||||
*/
|
||||
@SuppressWarnings("unused")
|
||||
@Query("SELECT COUNT(*) FROM TermConcept t WHERE t.myUpdated > :cutoff")
|
||||
long countUpdatedAfter(@Param("cutoff") Date theFullTextIndexedUntil);
|
||||
|
||||
/**
|
||||
* Used in Smile CDR - Do not delete
|
||||
*/
|
||||
@SuppressWarnings("unused")
|
||||
@Query("SELECT t FROM TermConcept t ORDER BY t.myUpdated ASC")
|
||||
Slice<TermConcept> findAllOrderedByLastUpdated(Pageable thePage);
|
||||
|
||||
@Query("SELECT c FROM TermConcept c WHERE c.myCodeSystem = :code_system AND c.myCode = :code")
|
||||
TermConcept findByCodeSystemAndCode(@Param("code_system") TermCodeSystemVersion theCodeSystem, @Param("code") String theCode);
|
||||
|
||||
|
@ -64,13 +49,6 @@ public interface ITermConceptDao extends JpaRepository<TermConcept, Long> {
|
|||
@Query("SELECT t FROM TermConcept t WHERE t.myIndexStatus = null")
|
||||
Page<TermConcept> findResourcesRequiringReindexing(Pageable thePageRequest);
|
||||
|
||||
/**
|
||||
* Used in Smile CDR - Do not delete
|
||||
*/
|
||||
@SuppressWarnings("unused")
|
||||
@Query("SELECT t FROM TermConcept t WHERE t.myUpdated > :cutoff ORDER BY t.myUpdated ASC")
|
||||
Slice<TermConcept> findUpdatedAfterOrderedByLastUpdated(Pageable thePage, @Param("cutoff") Date theFullTextIndexedUntil);
|
||||
|
||||
@Query("UPDATE TermConcept t SET t.myIndexStatus = null")
|
||||
@Modifying
|
||||
int markAllForReindexing();
|
||||
|
|
|
@ -7,6 +7,7 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
|
|||
import ca.uhn.fhir.rest.server.IRestfulServerDefaults;
|
||||
import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor;
|
||||
import ca.uhn.fhir.rest.server.interceptor.IServerOperationInterceptor;
|
||||
import ca.uhn.fhir.util.UrlUtil;
|
||||
import org.apache.commons.lang3.Validate;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.hl7.fhir.instance.model.api.IIdType;
|
||||
|
@ -19,6 +20,8 @@ import java.io.Reader;
|
|||
import java.io.UnsupportedEncodingException;
|
||||
import java.nio.charset.Charset;
|
||||
import java.util.*;
|
||||
import java.util.function.BiFunction;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import static org.apache.commons.lang3.StringUtils.isBlank;
|
||||
|
||||
|
@ -184,6 +187,21 @@ public abstract class RequestDetails {
|
|||
public void setParameters(Map<String, String[]> theParams) {
|
||||
myParameters = theParams;
|
||||
myUnqualifiedToQualifiedNames = null;
|
||||
|
||||
// Sanitize keys if necessary to prevent injection attacks
|
||||
boolean needsSanitization = false;
|
||||
for (String nextKey : theParams.keySet()) {
|
||||
if (UrlUtil.isNeedsSanitization(nextKey)) {
|
||||
needsSanitization = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (needsSanitization) {
|
||||
myParameters = myParameters
|
||||
.entrySet()
|
||||
.stream()
|
||||
.collect(Collectors.toMap(t -> UrlUtil.sanitizeUrlPart((String) ((Map.Entry) t).getKey()), t -> (String[]) ((Map.Entry) t).getValue()));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -1246,7 +1246,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
|
|||
String operation = null;
|
||||
String compartment = null;
|
||||
if (tok.hasMoreTokens()) {
|
||||
resourceName = tok.nextToken();
|
||||
resourceName = tok.nextTokenUnescapedAndSanitized();
|
||||
if (partIsOperation(resourceName)) {
|
||||
operation = resourceName;
|
||||
resourceName = null;
|
||||
|
@ -1255,7 +1255,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
|
|||
theRequestDetails.setResourceName(resourceName);
|
||||
|
||||
if (tok.hasMoreTokens()) {
|
||||
String nextString = tok.nextToken();
|
||||
String nextString = tok.nextTokenUnescapedAndSanitized();
|
||||
if (partIsOperation(nextString)) {
|
||||
operation = nextString;
|
||||
} else {
|
||||
|
@ -1265,10 +1265,10 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
|
|||
}
|
||||
|
||||
if (tok.hasMoreTokens()) {
|
||||
String nextString = tok.nextToken();
|
||||
String nextString = tok.nextTokenUnescapedAndSanitized();
|
||||
if (nextString.equals(Constants.PARAM_HISTORY)) {
|
||||
if (tok.hasMoreTokens()) {
|
||||
String versionString = tok.nextToken();
|
||||
String versionString = tok.nextTokenUnescapedAndSanitized();
|
||||
if (id == null) {
|
||||
throw new InvalidRequestException("Don't know how to handle request path: " + theRequestPath);
|
||||
}
|
||||
|
@ -1290,7 +1290,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
|
|||
String secondaryOperation = null;
|
||||
|
||||
while (tok.hasMoreTokens()) {
|
||||
String nextString = tok.nextToken();
|
||||
String nextString = tok.nextTokenUnescapedAndSanitized();
|
||||
if (operation == null) {
|
||||
operation = nextString;
|
||||
} else if (secondaryOperation == null) {
|
||||
|
|
|
@ -43,7 +43,7 @@ public class UrlBaseTenantIdentificationStrategy implements ITenantIdentificatio
|
|||
public void extractTenant(UrlPathTokenizer theUrlPathTokenizer, RequestDetails theRequestDetails) {
|
||||
String tenantId = null;
|
||||
if (theUrlPathTokenizer.hasMoreTokens()) {
|
||||
tenantId = defaultIfBlank(theUrlPathTokenizer.nextToken(), null);
|
||||
tenantId = defaultIfBlank(theUrlPathTokenizer.nextTokenUnescapedAndSanitized(), null);
|
||||
ourLog.trace("Found tenant ID {} in request string", tenantId);
|
||||
theRequestDetails.setTenantId(tenantId);
|
||||
}
|
||||
|
|
|
@ -0,0 +1,251 @@
|
|||
package ca.uhn.fhir.rest.server.interceptor;
|
||||
|
||||
import ca.uhn.fhir.context.FhirContext;
|
||||
import ca.uhn.fhir.rest.annotation.IdParam;
|
||||
import ca.uhn.fhir.rest.annotation.OptionalParam;
|
||||
import ca.uhn.fhir.rest.annotation.Read;
|
||||
import ca.uhn.fhir.rest.annotation.Search;
|
||||
import ca.uhn.fhir.rest.api.Constants;
|
||||
import ca.uhn.fhir.rest.param.TokenParam;
|
||||
import ca.uhn.fhir.rest.server.IResourceProvider;
|
||||
import ca.uhn.fhir.rest.server.RestfulServer;
|
||||
import ca.uhn.fhir.util.PortUtil;
|
||||
import ca.uhn.fhir.util.TestUtil;
|
||||
import ca.uhn.fhir.util.UrlUtil;
|
||||
import com.google.common.base.Charsets;
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.apache.http.client.methods.CloseableHttpResponse;
|
||||
import org.apache.http.client.methods.HttpGet;
|
||||
import org.apache.http.impl.client.CloseableHttpClient;
|
||||
import org.apache.http.impl.client.HttpClientBuilder;
|
||||
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
|
||||
import org.eclipse.jetty.server.Server;
|
||||
import org.eclipse.jetty.servlet.ServletHandler;
|
||||
import org.eclipse.jetty.servlet.ServletHolder;
|
||||
import org.hl7.fhir.r4.model.IdType;
|
||||
import org.hl7.fhir.r4.model.Patient;
|
||||
import org.junit.AfterClass;
|
||||
import org.junit.BeforeClass;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.not;
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
public class InjectionAttackTest {
|
||||
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(InjectionAttackTest.class);
|
||||
private static CloseableHttpClient ourClient;
|
||||
private static FhirContext ourCtx = FhirContext.forR4();
|
||||
private static int ourPort;
|
||||
private static Server ourServer;
|
||||
private static RestfulServer ourServlet;
|
||||
|
||||
@Test
|
||||
public void testPreventHtmlInjectionViaInvalidContentType() throws Exception {
|
||||
String requestUrl = "http://localhost:" +
|
||||
ourPort +
|
||||
"/Patient/123";
|
||||
|
||||
// XML HTML
|
||||
HttpGet httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, "application/<script>");
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(200, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPreventHtmlInjectionViaInvalidParameterName() throws Exception {
|
||||
String requestUrl = "http://localhost:" +
|
||||
ourPort +
|
||||
"/Patient?a" +
|
||||
UrlUtil.escapeUrlParam("<script>") +
|
||||
"=123";
|
||||
|
||||
// XML HTML
|
||||
HttpGet httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_HTML + ", " + Constants.CT_FHIR_XML_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(400, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
assertEquals("text/html", status.getFirstHeader("Content-Type").getValue().toLowerCase().replaceAll(";.*", "").trim());
|
||||
}
|
||||
|
||||
// JSON HTML
|
||||
httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_HTML + ", " + Constants.CT_FHIR_JSON_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(400, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
assertEquals("text/html", status.getFirstHeader("Content-Type").getValue().toLowerCase().replaceAll(";.*", "").trim());
|
||||
}
|
||||
|
||||
// XML HTML
|
||||
httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_FHIR_XML_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(400, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
assertEquals(Constants.CT_FHIR_XML_NEW, status.getFirstHeader("Content-Type").getValue().toLowerCase().replaceAll(";.*", "").trim());
|
||||
}
|
||||
|
||||
// JSON Plain
|
||||
httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_FHIR_JSON_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(400, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
assertEquals(Constants.CT_FHIR_JSON_NEW, status.getFirstHeader("Content-Type").getValue().toLowerCase().replaceAll(";.*", "").trim());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPreventHtmlInjectionViaInvalidResourceType() throws Exception {
|
||||
String requestUrl = "http://localhost:" +
|
||||
ourPort +
|
||||
"/AA" +
|
||||
UrlUtil.escapeUrlParam("<script>");
|
||||
|
||||
// XML HTML
|
||||
HttpGet httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_HTML + ", " + Constants.CT_FHIR_XML_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(404, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
assertEquals("text/html", status.getFirstHeader("Content-Type").getValue().toLowerCase().replaceAll(";.*", "").trim());
|
||||
}
|
||||
|
||||
// JSON HTML
|
||||
httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_HTML + ", " + Constants.CT_FHIR_JSON_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(404, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
assertEquals("text/html", status.getFirstHeader("Content-Type").getValue().toLowerCase().replaceAll(";.*", "").trim());
|
||||
}
|
||||
|
||||
// XML HTML
|
||||
httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_FHIR_XML_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(404, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
assertEquals(Constants.CT_FHIR_XML_NEW, status.getFirstHeader("Content-Type").getValue().toLowerCase().replaceAll(";.*", "").trim());
|
||||
}
|
||||
|
||||
// JSON Plain
|
||||
httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_FHIR_JSON_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(404, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
assertEquals(Constants.CT_FHIR_JSON_NEW, status.getFirstHeader("Content-Type").getValue().toLowerCase().replaceAll(";.*", "").trim());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPreventHtmlInjectionViaInvalidTokenParamModifier() throws Exception {
|
||||
String requestUrl = "http://localhost:" +
|
||||
ourPort +
|
||||
"/Patient?identifier:" +
|
||||
UrlUtil.escapeUrlParam("<script>") +
|
||||
"=123";
|
||||
HttpGet httpGet = new HttpGet(requestUrl);
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, "application/<script>");
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(200, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>")));
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@AfterClass
|
||||
public static void afterClassClearContext() throws Exception {
|
||||
ourServer.stop();
|
||||
TestUtil.clearAllStaticFieldsForUnitTest();
|
||||
}
|
||||
|
||||
@BeforeClass
|
||||
public static void beforeClass() throws Exception {
|
||||
|
||||
ourPort = PortUtil.findFreePort();
|
||||
ourServer = new Server(ourPort);
|
||||
|
||||
DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider();
|
||||
|
||||
ServletHandler proxyHandler = new ServletHandler();
|
||||
ourServlet = new RestfulServer(ourCtx);
|
||||
ourServlet.setFhirContext(ourCtx);
|
||||
ourServlet.setResourceProviders(patientProvider);
|
||||
ourServlet.registerInterceptor(new ResponseHighlighterInterceptor());
|
||||
ServletHolder servletHolder = new ServletHolder(ourServlet);
|
||||
proxyHandler.addServletWithMapping(servletHolder, "/*");
|
||||
ourServer.setHandler(proxyHandler);
|
||||
ourServer.start();
|
||||
|
||||
PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS);
|
||||
HttpClientBuilder builder = HttpClientBuilder.create();
|
||||
builder.setConnectionManager(connectionManager);
|
||||
ourClient = builder.build();
|
||||
|
||||
}
|
||||
|
||||
public static class DummyPatientResourceProvider implements IResourceProvider {
|
||||
|
||||
@Override
|
||||
public Class<? extends Patient> getResourceType() {
|
||||
return Patient.class;
|
||||
}
|
||||
|
||||
@Read
|
||||
public Patient read(@IdParam IdType theId) {
|
||||
Patient patient = new Patient();
|
||||
patient.setId(theId);
|
||||
patient.setActive(true);
|
||||
return patient;
|
||||
}
|
||||
|
||||
@Search
|
||||
public List<Patient> search(@OptionalParam(name = "identifier") TokenParam theToken) {
|
||||
return new ArrayList<>();
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
||||
}
|
|
@ -5,18 +5,14 @@ import ca.uhn.fhir.rest.annotation.IdParam;
|
|||
import ca.uhn.fhir.rest.annotation.Read;
|
||||
import ca.uhn.fhir.rest.annotation.RequiredParam;
|
||||
import ca.uhn.fhir.rest.annotation.Search;
|
||||
import ca.uhn.fhir.rest.api.Constants;
|
||||
import ca.uhn.fhir.rest.param.TokenParam;
|
||||
import ca.uhn.fhir.rest.server.IResourceProvider;
|
||||
import ca.uhn.fhir.rest.server.RestfulServer;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
||||
import ca.uhn.fhir.util.PortUtil;
|
||||
import ca.uhn.fhir.util.TestUtil;
|
||||
import ca.uhn.fhir.util.UrlUtil;
|
||||
import com.google.common.base.Charsets;
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.apache.http.HttpResponse;
|
||||
import org.apache.http.client.methods.CloseableHttpResponse;
|
||||
import org.apache.http.client.methods.HttpGet;
|
||||
import org.apache.http.impl.client.CloseableHttpClient;
|
||||
import org.apache.http.impl.client.HttpClientBuilder;
|
||||
|
@ -33,14 +29,12 @@ import org.junit.Test;
|
|||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.not;
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
public class ServerWithResponseHighlightingInterceptorExceptionTest {
|
||||
private static CloseableHttpClient ourClient;
|
||||
|
||||
private static FhirContext ourCtx = FhirContext.forR4();
|
||||
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ServerWithResponseHighlightingInterceptorExceptionTest.class);
|
||||
private static CloseableHttpClient ourClient;
|
||||
private static FhirContext ourCtx = FhirContext.forR4();
|
||||
private static int ourPort;
|
||||
private static Server ourServer;
|
||||
private static RestfulServer ourServlet;
|
||||
|
@ -52,7 +46,7 @@ public class ServerWithResponseHighlightingInterceptorExceptionTest {
|
|||
String responseContent = IOUtils.toString(status.getEntity().getContent());
|
||||
IOUtils.closeQuietly(status.getEntity().getContent());
|
||||
ourLog.info(responseContent);
|
||||
|
||||
|
||||
assertEquals(400, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, containsString("<diagnostics value=\"AAABBB\"/>"));
|
||||
}
|
||||
|
@ -65,44 +59,11 @@ public class ServerWithResponseHighlightingInterceptorExceptionTest {
|
|||
String responseContent = IOUtils.toString(status.getEntity().getContent());
|
||||
IOUtils.closeQuietly(status.getEntity().getContent());
|
||||
ourLog.info(responseContent);
|
||||
|
||||
|
||||
assertEquals(500, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, containsString("<diagnostics value=\"Failed to call access method: java.lang.Error: AAABBB\"/>"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPreventHtmlInjectionViaInvalidResourceType() throws Exception {
|
||||
// XML
|
||||
HttpGet httpGet = new HttpGet(
|
||||
"http://localhost:" +
|
||||
ourPort +
|
||||
"/AA" +
|
||||
UrlUtil.escapeUrlParam("<script>"));
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_HTML+", " +Constants.CT_FHIR_XML_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(404, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>>")));
|
||||
}
|
||||
|
||||
// JSON
|
||||
httpGet = new HttpGet(
|
||||
"http://localhost:" +
|
||||
ourPort +
|
||||
"/AA" +
|
||||
UrlUtil.escapeUrlParam("<script>"));
|
||||
httpGet.addHeader(Constants.HEADER_ACCEPT, Constants.CT_HTML+", " +Constants.CT_FHIR_JSON_NEW);
|
||||
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
|
||||
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
|
||||
ourLog.info(responseContent);
|
||||
|
||||
assertEquals(404, status.getStatusLine().getStatusCode());
|
||||
assertThat(responseContent, not(containsString("<script>>")));
|
||||
}
|
||||
}
|
||||
|
||||
@AfterClass
|
||||
public static void afterClassClearContext() throws Exception {
|
||||
ourServer.stop();
|
||||
|
@ -147,7 +108,7 @@ public class ServerWithResponseHighlightingInterceptorExceptionTest {
|
|||
}
|
||||
|
||||
@Search
|
||||
public Patient search(@RequiredParam(name="identifier") TokenParam theToken) {
|
||||
public Patient search(@RequiredParam(name = "identifier") TokenParam theToken) {
|
||||
throw new Error("AAABBB");
|
||||
}
|
||||
|
||||
|
|
|
@ -144,6 +144,12 @@
|
|||
client.operation() call. Previously this caused a strange
|
||||
NullPointerException.
|
||||
</action>
|
||||
<action type="fix">
|
||||
The REST Server now sanitizes URL path components and query parameter
|
||||
names to escape several reserved characters (e.g. " and <)
|
||||
in order to prevent HTML injection attacks via maliciously
|
||||
crafted URLs.
|
||||
</action>
|
||||
</release>
|
||||
<release version="3.4.0" date="2018-05-28">
|
||||
<action type="add">
|
||||
|
|
Loading…
Reference in New Issue