Attempt to restrict XSS filtering

openmrs · Nov 16, 2024 · fb1b978 · fb1b978
1 parent 2449683
commit fb1b978
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 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
@@ -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
@@ -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
@@ -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,37 +30,19 @@ 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;
 	}
 
 	@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
@@ -386,7 +386,7 @@
 	</globalProperty>
 	<globalProperty>
 		<property>dashboard.formEntry.maximumNumberEncountersToShow</property>
-		<defaultValue></defaultValue>
+		<defaultValue />
 		<description>
 			Allows one to limit the number of encounters shown on the form entry tab of the patient dashboard specifically
 		</description>