From fb1b978a6829e33c41c020494fb233740c1b5b47 Mon Sep 17 00:00:00 2001 From: Ian Date: Sat, 16 Nov 2024 16:12:15 +0530 Subject: [PATCH] Attempt to restrict XSS filtering --- .../java/org/openmrs/web/xss/XSSFilter.java | 43 ++++++++----------- .../web/xss/XSSMultipartRequestWrapper.java | 13 +++--- .../openmrs/web/xss/XSSRequestWrapper.java | 29 +------------ omod/src/main/resources/config.xml | 2 +- 4 files changed, 28 insertions(+), 59 deletions(-) diff --git a/omod/src/main/java/org/openmrs/web/xss/XSSFilter.java b/omod/src/main/java/org/openmrs/web/xss/XSSFilter.java index c3857ffa..e8d2f1ad 100644 --- a/omod/src/main/java/org/openmrs/web/xss/XSSFilter.java +++ b/omod/src/main/java/org/openmrs/web/xss/XSSFilter.java @@ -10,42 +10,37 @@ package org.openmrs.web.xss; import java.io.IOException; +import java.util.Locale; -import javax.servlet.Filter; import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; -import org.apache.commons.fileupload.servlet.ServletFileUpload; -import org.springframework.web.multipart.support.DefaultMultipartHttpServletRequest; +import org.springframework.web.filter.OncePerRequestFilter; +import org.springframework.web.multipart.MultipartHttpServletRequest; -public class XSSFilter implements Filter { +public class XSSFilter extends OncePerRequestFilter { @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, - ServletException { + public void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) + throws IOException, ServletException { - if (!"GET".equalsIgnoreCase(((HttpServletRequest) request).getMethod())) { - if (ServletFileUpload.isMultipartContent((HttpServletRequest) request)) { - request = new XSSMultipartRequestWrapper((DefaultMultipartHttpServletRequest) request); - } else { - request = new XSSRequestWrapper((HttpServletRequest) request); + String method = request.getMethod(); + String contentType = request.getContentType(); + if (!"GET".equals(method) && !"OPTIONS".equals(method) && !"HEAD".equalsIgnoreCase(method) && contentType != null) { + if (contentType.toLowerCase(Locale.ROOT).startsWith("multipart/form-data")) { + if (!(request instanceof MultipartHttpServletRequest)) { + response.sendError(HttpServletResponse.SC_BAD_REQUEST); + return; + } + + request = new XSSMultipartRequestWrapper((MultipartHttpServletRequest) request); + } else if (contentType.toLowerCase(Locale.ROOT).startsWith("application/x-www-form-urlencoded")) { + request = new XSSRequestWrapper(request); } } chain.doFilter(request, response); } - - @Override - public void init(FilterConfig filterConfig) throws ServletException { - - } - - @Override - public void destroy() { - - } } diff --git a/omod/src/main/java/org/openmrs/web/xss/XSSMultipartRequestWrapper.java b/omod/src/main/java/org/openmrs/web/xss/XSSMultipartRequestWrapper.java index e6df7bb2..58324b46 100644 --- a/omod/src/main/java/org/openmrs/web/xss/XSSMultipartRequestWrapper.java +++ b/omod/src/main/java/org/openmrs/web/xss/XSSMultipartRequestWrapper.java @@ -14,17 +14,17 @@ import org.owasp.encoder.Encode; import org.springframework.util.MultiValueMap; import org.springframework.web.multipart.MultipartFile; -import org.springframework.web.multipart.support.DefaultMultipartHttpServletRequest; +import org.springframework.web.multipart.MultipartHttpServletRequest; +import org.springframework.web.multipart.support.StandardMultipartHttpServletRequest; -public class XSSMultipartRequestWrapper extends DefaultMultipartHttpServletRequest { +public class XSSMultipartRequestWrapper extends StandardMultipartHttpServletRequest { - public XSSMultipartRequestWrapper(DefaultMultipartHttpServletRequest request) { + public XSSMultipartRequestWrapper(MultipartHttpServletRequest request) { super(request); } @Override public String getParameter(String name) { - String value = getRequest().getParameter(name); if (value == null) { return null; @@ -35,7 +35,6 @@ public String getParameter(String name) { @Override public String[] getParameterValues(String name) { - String[] values = getRequest().getParameterValues(name); if (values == null) { return null; @@ -51,8 +50,8 @@ public String[] getParameterValues(String name) { } @Override - public DefaultMultipartHttpServletRequest getRequest() { - return (DefaultMultipartHttpServletRequest) super.getRequest(); + public MultipartHttpServletRequest getRequest() { + return (MultipartHttpServletRequest) super.getRequest(); } @Override diff --git a/omod/src/main/java/org/openmrs/web/xss/XSSRequestWrapper.java b/omod/src/main/java/org/openmrs/web/xss/XSSRequestWrapper.java index 844bd55c..9bdd06c6 100644 --- a/omod/src/main/java/org/openmrs/web/xss/XSSRequestWrapper.java +++ b/omod/src/main/java/org/openmrs/web/xss/XSSRequestWrapper.java @@ -9,15 +9,9 @@ */ package org.openmrs.web.xss; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.nio.charset.StandardCharsets; - -import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; -import org.apache.commons.io.IOUtils; import org.owasp.encoder.Encode; public class XSSRequestWrapper extends HttpServletRequestWrapper { @@ -28,7 +22,6 @@ public XSSRequestWrapper(HttpServletRequest request) { @Override public String[] getParameterValues(String parameter) { - String[] values = super.getParameterValues(parameter); if (values == null) { return null; @@ -37,7 +30,7 @@ public String[] getParameterValues(String parameter) { int count = values.length; String[] encodedValues = new String[count]; for (int i = 0; i < count; i++) { - encodedValues[i] = Encode.forHtml(values[i]); + encodedValues[i] = Encode.forHtmlContent(values[i]); } return encodedValues; @@ -45,29 +38,11 @@ public String[] getParameterValues(String parameter) { @Override public String getParameter(String name) { - String value = super.getParameter(name); if (value == null) { return null; } - return Encode.forHtml(value); - } - - @Override - public ServletInputStream getInputStream() throws IOException { - - String requestBody = IOUtils.toString(super.getInputStream(), StandardCharsets.UTF_8.name()); - String sanitizedBody = Encode.forHtmlContent(requestBody); - - return new ServletInputStream() { - - private final ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(sanitizedBody.getBytes()); - - @Override - public int read() throws IOException { - return byteArrayInputStream.read(); - } - }; + return Encode.forHtmlContent(value); } } diff --git a/omod/src/main/resources/config.xml b/omod/src/main/resources/config.xml index b4009f68..a3265ab5 100644 --- a/omod/src/main/resources/config.xml +++ b/omod/src/main/resources/config.xml @@ -386,7 +386,7 @@ dashboard.formEntry.maximumNumberEncountersToShow - + Allows one to limit the number of encounters shown on the form entry tab of the patient dashboard specifically