Skip to content

Attempt to restrict XSS filtering #210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Attempt to restrict XSS filtering #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 19 additions & 24 deletions omod/src/main/java/org/openmrs/web/xss/XSSFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {

}
}
13 changes: 6 additions & 7 deletions omod/src/main/java/org/openmrs/web/xss/XSSMultipartRequestWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down
29 changes: 2 additions & 27 deletions omod/src/main/java/org/openmrs/web/xss/XSSRequestWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -28,7 +22,6 @@ public XSSRequestWrapper(HttpServletRequest request) {

@Override
public String[] getParameterValues(String parameter) {

String[] values = super.getParameterValues(parameter);
if (values == null) {
return null;
Expand All @@ -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);
}
}
2 changes: 1 addition & 1 deletion omod/src/main/resources/config.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading