3

Hi in my project Veracode reported a XSS issue CWE ID 80. Here in my request handler method:

@RequestMapping(value = "/Update.mvc")
public @ResponseBody String execute(@ModelAttribute UpdateForm updateForm, BindingResult result,
       HttpServletRequest request, HttpServletResponse response) throws ActionException {
    return executeAjax(updateForm, request, response, result);
}

So executeAjax comes from an abstract class and has different implementations? In these implementations user input from the form is get and is manipulated in order to construct the String which is returned.

So my question is : Is the Veracode assumption is that in the implementation can have XSS? Or something general? - How can prevent this? I always use transforms the input data and it is not returned as the user enters it? - So how to prevent it? - Do I have to escape all the headers/request paramters from the HttpServiceRequest?

Edit: Do I have to use filters like: SecurityWrapperRequest

Xelian
  • 16,680
  • 25
  • 99
  • 152
  • You need to escape all request parameters. E.g. by adding a filter – StanislavL Jan 30 '17 at 14:56
  • @StanislavL can you give more detail explanation. I need to add HttpRequest filter which to escape all headers and all parameters from the request? Or I can work on local level and to wrap the request just for all implementations of my executeAjax classes? LikeL https://www.javacodegeeks.com/2012/07/anti-cross-site-scripting-xss-filter.html – Xelian Jan 30 '17 at 15:58

1 Answers1

5

You can use XSSFilter to escape all request parameters. See here

public class XSSFilter implements Filter {

    @Override
    public void init(FilterConfig filterConfig) throws ServletException {
    }

    @Override
    public void destroy() {
    }

    @Override
    public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
        throws IOException, ServletException {
        chain.doFilter(new XSSRequestWrapper((HttpServletRequest) request), response);
    }

}

and wrapper

import java.util.regex.Pattern;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;

public class XSSRequestWrapper extends HttpServletRequestWrapper {

    public XSSRequestWrapper(HttpServletRequest servletRequest) {
        super(servletRequest);
    }

    @Override
    public String[] getParameterValues(String parameter) {
        String[] values = super.getParameterValues(parameter);

        if (values == null) {
            return null;
        }

        int count = values.length;
        String[] encodedValues = new String[count];
        for (int i = 0; i < count; i++) {
            encodedValues[i] = stripXSS(values[i]);
        }

        return encodedValues;
    }

    @Override
    public String getParameter(String parameter) {
        String value = super.getParameter(parameter);

        return stripXSS(value);
    }

    @Override
    public String getHeader(String name) {
        String value = super.getHeader(name);
        return stripXSS(value);
    }

    private String stripXSS(String value) {
        if (value != null) {
            // NOTE: It's highly recommended to use the ESAPI library and uncomment the following line to
            // avoid encoded attacks.
            // value = ESAPI.encoder().canonicalize(value);

            // Avoid null characters
            value = value.replaceAll("", "");

            // Avoid anything between script tags
            Pattern scriptPattern = Pattern.compile("<script>(.*?)</script>", Pattern.CASE_INSENSITIVE);
            value = scriptPattern.matcher(value).replaceAll("");

            // Avoid anything in a src='...' type of expression
            scriptPattern = Pattern.compile("src[\r\n]*=[\r\n]*\\\'(.*?)\\\'", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE | Pattern.DOTALL);
            value = scriptPattern.matcher(value).replaceAll("");

            scriptPattern = Pattern.compile("src[\r\n]*=[\r\n]*\\\"(.*?)\\\"", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE | Pattern.DOTALL);
            value = scriptPattern.matcher(value).replaceAll("");

            // Remove any lonesome </script> tag
            scriptPattern = Pattern.compile("</script>", Pattern.CASE_INSENSITIVE);
            value = scriptPattern.matcher(value).replaceAll("");

            // Remove any lonesome <script ...> tag
            scriptPattern = Pattern.compile("<script(.*?)>", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE | Pattern.DOTALL);
            value = scriptPattern.matcher(value).replaceAll("");

            // Avoid eval(...) expressions
            scriptPattern = Pattern.compile("eval\\((.*?)\\)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE | Pattern.DOTALL);
            value = scriptPattern.matcher(value).replaceAll("");

            // Avoid expression(...) expressions
            scriptPattern = Pattern.compile("expression\\((.*?)\\)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE | Pattern.DOTALL);
            value = scriptPattern.matcher(value).replaceAll("");

            // Avoid javascript:... expressions
            scriptPattern = Pattern.compile("javascript:", Pattern.CASE_INSENSITIVE);
            value = scriptPattern.matcher(value).replaceAll("");

            // Avoid vbscript:... expressions
            scriptPattern = Pattern.compile("vbscript:", Pattern.CASE_INSENSITIVE);
            value = scriptPattern.matcher(value).replaceAll("");

            // Avoid onload= expressions
            scriptPattern = Pattern.compile("onload(.*?)=", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE | Pattern.DOTALL);
            value = scriptPattern.matcher(value).replaceAll("");
        }
        return value;
    }
}

Actually you can use the filter as a basement and extend it to add any desired logic to wrapper.

StanislavL
  • 56,971
  • 9
  • 68
  • 98
  • Will have any negative impact on the performance, because these Patters matching are not very light? – Xelian Jan 31 '17 at 07:54
  • 1
    Anyway security features have negative impact on the performance. Try to replace the patterns checks with faster ones. – StanislavL Jan 31 '17 at 07:56
  • What about ESAPI wrapper? Can I use it? https://github.com/ESAPI/esapi-java-legacy/blob/2ff45b140a7e7b527a93bcdbc6bd7a1b3c188aca/src/main/java/org/owasp/esapi/filters/SecurityWrapperRequest.java – Xelian Jan 31 '17 at 08:01
  • 1
    You can use any filter which matches your requirements and does not allow request parameters with invalid content. Or even more than one filter. To make the filter faster you can compile the patterns once on create and reuse them. – StanislavL Jan 31 '17 at 08:28
  • Can we also apply this for HttpServletResponse? Because I do not see my response being passed through this filter – Muneeb Mirza Feb 10 '20 at 12:43
  • Request is sent from external and you can't control request content. That's why you need the logic. Response is created by the app. What's the reason to hack itself? – StanislavL Feb 10 '20 at 13:49