Reflected XSS vulnerability in Testpage overlay (#5719)
* HAPI FHIR Testpage potential XSS vulnerability - fix
This commit is contained in:
parent
e71efe5029
commit
538acc6fc5
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
type: security
|
||||
issue: 5717
|
||||
title: "Fixed a potential XSS vulnerability in the HAPI FHIR Testpage Overlay module."
|
|
@ -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;
|
||||
|
||||
|
|
|
@ -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();
|
||||
|
|
Loading…
Reference in New Issue