views:

2873

answers:

3

An existing web application is running on Tomcat 4.1. There is an XSS issue with a page, but I can't modify the source. I've decided to write a servlet filter to sanitize the parameter before it is seen by the page.

I would like to write a Filter class like this:

import java.io.*;
import javax.servlet.*;

public final class XssFilter implements Filter {

  public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
      throws IOException, ServletException
  {
    String badValue = request.getParameter("dangerousParamName");
    String goodValue = sanitize(badValue);
    request.setParameter("dangerousParamName", goodValue);
    chain.doFilter(request, response);
  }

  public void destroy() {
  }

  public void init(FilterConfig filterConfig) {
  }
}

But ServletRequest.setParameter doesn't exist.

How can I change the value of the request parameter before passing the request down the chain?

+1  A: 

Write a simple class that subcalsses HttpServletRequestWrapper with a getParamater() method that returns the sanitized version of the input. Then pass an instance of your HttpServletRequestWrapper to Filter.doChain instead of the response object directly.

Asaph
+5  A: 

As you've noted HttpServletRequest does not have a setParameter method. This is deliberate, since the class represents the request as came from the client, and modifying the parameter would not represent that.

One solution is to use the HttpServletRequestWrapper class, which allows you to wrap one request with another. You can subclass that, and override the getParameter method to return your sanitized value. You can then pass that wrapped request to chain.doFilter instead of the original request.

It's a bit ugly, but that's what the servlet API says you should do. If you try to pass anything else to doFilter, some servlet contains will complain that you have violatyed the spec, and will refuse to handle it.

A more elegant solution is more work - modify the original servlet/JSP that processes the parameter, so that it expects a request attribute instead of a parameter. The filter examines the parameter, sanitizes it, and sets the attribute (using request.setAttribute) with the sanitized value. No subclassing, no spoofing, but does require you to modify other parts of your application.

skaffman
HttpServletRequestWrapper is wonderful; I never knew it existed. Thanks!
Jeremy Stein
+3  A: 

For the record, here is the class I ended up writing:

import java.io.IOException;

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.HttpServletRequestWrapper;

public final class XssFilter implements Filter {

    static class FilteredRequest extends HttpServletRequestWrapper {

     /* These are the characters allowed by the Javascript validation */
     static String allowedChars = "+-0123456789#*";

     public FilteredRequest(ServletRequest request) {
      super((HttpServletRequest)request);
     }

     public String sanitize(String input) {
      String result = "";
      for (int i = 0; i < input.length(); i++) {
       if (allowedChars.indexOf(input.charAt(i)) >= 0) {
        result += input.charAt(i);
       }
      }
      return result;
     }

     public String getParameter(String paramName) {
      String value = super.getParameter(paramName);
      if ("dangerousParamName".equals(paramName)) {
       value = sanitize(value);
      }
      return value;
     }

     public String[] getParameterValues(String paramName) {
      String values[] = super.getParameterValues(paramName);
      if ("dangerousParamName".equals(paramName)) {
       for (int index = 0; index < values.length; index++) {
        values[index] = sanitize(values[index]);
       }
      }
      return values;
     }
    }

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

    public void destroy() {
    }

    public void init(FilterConfig filterConfig) {
    }
}
Jeremy Stein
You might also need to account for the getParameterMap method. Maybe throw and unsupported exception just so no components use the method and skip the sanitize logic.
Tom
Good point, Tom. In this particular case, I checked and found it wasn't being called, but I should have added that for completeness and for the next person's sake. Thanks!
Jeremy Stein