Reflected XSS vulnerability in Testpage overlay (#5719)

* HAPI FHIR Testpage potential XSS vulnerability - fix
This commit is contained in:
volodymyr-korzh 2024-02-20 15:12:53 -07:00 committed by GitHub
parent e5d410d10b
commit b4155e5d11
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 60 additions and 6 deletions

View File

@ -0,0 +1,4 @@
---
type: security
issue: 5717
title: "Fixed a potential XSS vulnerability in the HAPI FHIR Testpage Overlay module."

View File

@ -36,6 +36,7 @@ import ca.uhn.fhir.to.model.ResourceRequest;
import ca.uhn.fhir.to.model.TransactionRequest;
import ca.uhn.fhir.to.util.HfqlRenderingUtil;
import ca.uhn.fhir.util.StopWatch;
import ca.uhn.fhir.util.UrlUtil;
import com.google.gson.stream.JsonWriter;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
@ -279,7 +280,7 @@ public class Controller extends BaseController {
theModel.put("errorMsg", toDisplayError(e.toString(), e));
return "resource";
}
String id = sanitizeUrlPart(defaultString(theServletRequest.getParameter("id")));
String id = sanitizeUrlPart(UrlUtil.unescape(theServletRequest.getParameter("id")));
if (StringUtils.isBlank(id)) {
populateModelForResource(theServletRequest, theRequest, theModel);
theModel.put("errorMsg", toDisplayError("No ID specified", null));
@ -287,7 +288,7 @@ public class Controller extends BaseController {
}
ResultType returnsResource = ResultType.RESOURCE;
String versionId = sanitizeUrlPart(defaultString(theServletRequest.getParameter("vid")));
String versionId = sanitizeUrlPart(UrlUtil.unescape((theServletRequest.getParameter("vid"))));
String outcomeDescription;
if (StringUtils.isBlank(versionId)) {
versionId = null;
@ -800,9 +801,9 @@ public class Controller extends BaseController {
final BindingResult theBindingResult,
final ModelMap theModel) {
String instanceType = theReq.getParameter("instanceType");
String instanceId = theReq.getParameter("instanceId");
String operationName = theReq.getParameter("operationName");
String instanceType = sanitizeUrlPart((UrlUtil.unescape(theReq.getParameter("instanceType"))));
String instanceId = sanitizeUrlPart((UrlUtil.unescape(theReq.getParameter("instanceId"))));
String operationName = sanitizeUrlPart((UrlUtil.unescape(theReq.getParameter("operationName"))));
boolean finished = false;

View File

@ -31,6 +31,7 @@ import org.htmlunit.WebClient;
import org.htmlunit.cssparser.parser.CSSErrorHandler;
import org.htmlunit.html.HtmlAnchor;
import org.htmlunit.html.HtmlButton;
import org.htmlunit.html.HtmlElement;
import org.htmlunit.html.HtmlPage;
import org.htmlunit.html.HtmlTable;
import org.htmlunit.html.HtmlTableCell;
@ -41,12 +42,14 @@ import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.htmlunit.MockMvcWebConnection;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
import org.springframework.web.servlet.DispatcherServlet;
@ -57,10 +60,15 @@ import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.stream.Stream;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
@ -191,6 +199,47 @@ public class WebTest {
assertThat(summaryPage.asNormalizedText(), containsString("Result Narrative\t\nHELLO WORLD DOCUMENT"));
}
private static Stream<Arguments> getButtonMappingPredicates() {
Predicate<HtmlElement> readButtonPredicate = t -> t.getAttribute("value").equals("read");
Predicate<HtmlElement> summaryButtonPredicate = t -> t.asNormalizedText().equals("$summary");
return Stream.of(arguments(readButtonPredicate), arguments(summaryButtonPredicate));
}
@ParameterizedTest
@MethodSource(value = "getButtonMappingPredicates")
public void testInvokeOperation_withReflectedXssAttack_resultHtmlIsSanitized(
Predicate<HtmlElement> theButtonMappingPredicate) throws IOException {
register5Patients();
HtmlPage searchResultPage = searchForPatients();
HtmlTable controlsTable = searchResultPage.getHtmlElementById("resultControlsTable");
List<HtmlTableRow> controlRows = controlsTable.getBodies().get(0).getRows();
HtmlTableCell controlsCell = controlRows.get(0).getCell(0);
// find the button
HtmlElement summaryButton = controlsCell
.getElementsByTagName("button")
.stream()
.filter(theButtonMappingPredicate)
.findFirst()
.orElseThrow();
// alter button attributes to imitate Reflected XSS attack
summaryButton.setAttribute("data2", "A0%3Cscript%3Ealert(2)%3C/script%3E");
summaryButton.setAttribute("data3", "%24diff%3Cscript%3Ealert(1)%3C/script%3E");
HtmlPage summaryPage = summaryButton.click();
// validate that there is no <script> span in result summary page
Optional<HtmlElement> scriptSpans = summaryPage.getHtmlElementById("requestUrlAnchor")
.getElementsByTagName("span")
.stream()
.filter(span -> span.asXml().contains("<script>"))
.findAny();
assertTrue(scriptSpans.isEmpty());
}
@Test
public void testInvokeCustomOperation_Validate() throws IOException {
register5Patients();