Author: markt
Date: Tue Aug 24 09:30:11 2010
New Revision: 988448

URL: http://svn.apache.org/viewvc?rev=988448&view=rev
Log:
Correctly handle anchors in URLs with the CSRF prevention filter.

Added:
    tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java 
  (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: 
tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java?rev=988448&r1=988447&r2=988448&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java Tue 
Aug 24 09:30:11 2010
@@ -128,6 +128,7 @@ public class CsrfPreventionFilter extend
         }
     }
 
+    @Override
     public void doFilter(ServletRequest request, ServletResponse response,
             FilterChain chain) throws IOException, ServletException {
 
@@ -215,7 +216,7 @@ public class CsrfPreventionFilter extend
         return buffer.toString();
     }
 
-    private static class CsrfResponseWrapper
+    protected static class CsrfResponseWrapper
             extends HttpServletResponseWrapper {
 
         private String nonce;
@@ -248,7 +249,7 @@ public class CsrfPreventionFilter extend
         }
         
         /**
-         * Return the specified URL with the nonce added to the query string
+         * Return the specified URL with the nonce added to the query string. 
          *
          * @param url URL to be modified
          * @param nonce The nonce to add
@@ -261,18 +262,17 @@ public class CsrfPreventionFilter extend
             String path = url;
             String query = "";
             String anchor = "";
-            int question = url.indexOf('?');
-            if (question >= 0) {
-                path = url.substring(0, question);
-                query = url.substring(question);
-            }
             int pound = path.indexOf('#');
             if (pound >= 0) {
                 anchor = path.substring(pound);
                 path = path.substring(0, pound);
             }
+            int question = path.indexOf('?');
+            if (question >= 0) {
+                query = path.substring(question);
+                path = path.substring(0, question);
+            }
             StringBuilder sb = new StringBuilder(path);
-            sb.append(anchor);
             if (query.length() >0) {
                 sb.append(query);
                 sb.append('&');
@@ -282,6 +282,7 @@ public class CsrfPreventionFilter extend
             sb.append(Constants.CSRF_NONCE_REQUEST_PARAM);
             sb.append('=');
             sb.append(nonce);
+            sb.append(anchor);
             return (sb.toString());
         }
     }

Added: 
tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java?rev=988448&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java 
(added)
+++ tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java 
Tue Aug 24 09:30:11 2010
@@ -0,0 +1,76 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.catalina.filters;
+
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.core.DummyResponse;
+import org.apache.catalina.startup.TomcatBaseTest;
+
+public class TestCsrfPreventionFilter extends TomcatBaseTest {
+
+    private static final String RESULT_NONCE =
+        Constants.CSRF_NONCE_SESSION_ATTR_NAME + "=TESTNONCE";
+
+    private final HttpServletResponse wrapper =
+        new CsrfPreventionFilter.CsrfResponseWrapper(
+                new NonEncodingResponse(), "TESTNONCE");
+
+    public void testAddNonceNoQueryNoAnchor() throws Exception {
+        assertEquals("/test?" + RESULT_NONCE ,
+                wrapper.encodeRedirectURL("/test"));
+    }
+    
+    public void testAddNonceQueryNoAnchor() throws Exception {
+        assertEquals("/test?a=b&" + RESULT_NONCE ,
+                wrapper.encodeRedirectURL("/test?a=b"));
+    }
+    
+    public void testAddNonceNoQueryAnchor() throws Exception {
+        assertEquals("/test?" + RESULT_NONCE + "#c",
+                wrapper.encodeRedirectURL("/test#c"));
+    }
+    
+    public void testAddNonceQueryAnchor() throws Exception {
+        assertEquals("/test?a=b&" + RESULT_NONCE + "#c",
+                wrapper.encodeRedirectURL("/test?a=b#c"));
+    }
+    
+    private static class NonEncodingResponse extends DummyResponse {
+
+        @Override
+        public String encodeRedirectURL(String url) {
+            return url;
+        }
+
+        @Override
+        public String encodeRedirectUrl(String url) {
+            return url;
+        }
+
+        @Override
+        public String encodeURL(String url) {
+            return url;
+        }
+
+        @Override
+        public String encodeUrl(String url) {
+            return url;
+        }
+    }
+}

Propchange: 
tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=988448&r1=988447&r2=988448&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Aug 24 09:30:11 2010
@@ -75,6 +75,10 @@
         response by repeating the POST request including a request body. Any
         request body provided at this point is now swallowed. (markt)  
       </fix>
+      <fix>
+        CSRF prevention filter did not correctly handle URLs that used anchors.
+        (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to