Permalink
Browse files

add better query string filter rules for the invalid url check and bu…

…mp the version
  • Loading branch information...
Chris Heisterkamp
Chris Heisterkamp committed Jun 6, 2012
1 parent 98c00f0 commit 0a83ed81afc22c0e99bed07044b7e8ba2c734506
Showing with 31 additions and 2 deletions.
  1. +1 −1 jetpack.gemspec
  2. +9 −0 spec/filter_spec.rb
  3. +21 −1 src/java/jetpack/filter/ValidUrlFilter.java
View
@@ -2,7 +2,7 @@
Gem::Specification.new do |s|
s.name = "jetpack"
- s.version = "0.1.1"
+ s.version = "0.1.2"
s.platform = Gem::Platform::RUBY
s.authors = ["Steve Conover", "Xavier Shay", "Taylor Phillips", "Chris Heisterkamp"]
s.email = ["steve@squareup.com", "xavier@squareup.com", "taylor@squareup.com", "cheister@squareup.com"]
View
@@ -16,19 +16,28 @@
begin
x!("curl https://localhost:11443/hello --insecure")[:stdout].split("<br/>").first.strip.should == "Hello World"
+ x!("curl https://localhost:11443/hello?foo=bar --insecure")[:stdout].split("<br/>").first.strip.should == "Hello World"
+
x!("curl --head --request DEBUG https://localhost:11443/ --insecure")[:stdout].split("\n").first.strip.should == "HTTP/1.1 405 Method Not Allowed"
x!("curl --head 'https://localhost:11443/<script>xss</script>.aspx' --insecure")[:stdout].split("\n").first.strip.should == "HTTP/1.1 400 Bad Request"
+ x!("curl --head 'https://localhost:11443/?foo=<script>xss</script>.aspx' --insecure")[:stdout].split("\n").first.strip.should == "HTTP/1.1 400 Bad Request"
+
x!("curl http://localhost:11080/hello")[:stdout].split("<br/>").first.strip.should == "Hello World"
+ x!("curl http://localhost:11080/hello?foo=bar")[:stdout].split("<br/>").first.strip.should == "Hello World"
+
x!("curl http://#{Socket.gethostname}:11080/hello")[:stdout].split("<br/>").first.strip.should == "Hello World"
x!("curl http://127.0.0.1:11080/hello")[:stdout].split("<br/>").first.strip.should == "Hello World"
x!("curl --head --request DEBUG http://localhost:11080/")[:stdout].split("\n").first.strip.should == "HTTP/1.1 405 Method Not Allowed"
x!("curl --head 'http://localhost:11080/<script>xss</script>.aspx'")[:stdout].split("\n").first.strip.should == "HTTP/1.1 400 Bad Request"
+
+ x!("curl --head 'http://localhost:11080/?foo=<script>xss</script>.aspx'")[:stdout].split("\n").first.strip.should == "HTTP/1.1 400 Bad Request"
+
ensure
system("kill -9 #{pid_to_kill}")
end
@@ -1,6 +1,7 @@
package jetpack.filter;
import java.io.IOException;
+import java.util.regex.Pattern;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
@@ -31,12 +32,31 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
HttpServletRequest req = (HttpServletRequest)request;
- if (urlValidator.isValid(req.getRequestURL().toString())) {
+ String requestUrl = req.getRequestURL().toString();
+ String queryString = req.getQueryString();
+ if (queryString != null) {
+ requestUrl += "?" + queryString;
+ }
+
+ if (urlValidator.isValid(requestUrl) && isValidQuery(queryString)) {
chain.doFilter(request, response);
} else {
HttpServletResponse res = (HttpServletResponse)response;
res.sendError(HttpServletResponse.SC_BAD_REQUEST);
return;
}
}
+
+ // commons validator allows any character in query string, we want to restrict it a bit
+ // and not allow unescaped angle brackets for example.
+ private static final String QUERY_REGEX = "^([-\\w:@&=~+,.!*'%$_;\\(\\)]*)$";
+ private static final Pattern QUERY_PATTERN = Pattern.compile(QUERY_REGEX);
+
+ protected boolean isValidQuery(String query) {
+ if (query == null) {
+ return true;
+ }
+
+ return QUERY_PATTERN.matcher(query).matches();
+ }
}

0 comments on commit 0a83ed8

Please sign in to comment.