[jetty-commit] r166 - in jetty/trunk: . jetty-security/src/main/java/org

[genie@xxxxxxxxxxx]

Author: gwilkins
Date: 2009-04-21 00:34:48 -0400 (Tue, 21 Apr 2009)
New Revision: 166

Added:
   jetty/trunk/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
Modified:
   jetty/trunk/VERSION.txt
   jetty/trunk/jetty-security/src/main/java/org/eclipse/jetty/security/IdentityService.java
   jetty/trunk/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
   jetty/trunk/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
   jetty/trunk/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java
   jetty/trunk/jetty-util/src/test/java/org/eclipse/jetty/util/URITest.java
Log:
273011 XSS in directory listing

Modified: jetty/trunk/VERSION.txt
===================================================================
--- jetty/trunk/VERSION.txt	2009-04-21 03:11:25 UTC (rev 165)
+++ jetty/trunk/VERSION.txt	2009-04-21 04:34:48 UTC (rev 166)
@@ -5,7 +5,7 @@
  + JETTY-695 Handler dump
  + Reworked authentication for deferred authentication
  + JETTY-983 DefaultServlet generates accept-ranges for cached/gzip content
- 
+ + 273011 JETTY-980 JETTY-992 Security / Directory Listing XSS present
 
 jetty-7.0.0.M0
  + JETTY-496 Support inetd/xinetd through use of System.inheritedChannel()

Modified: jetty/trunk/jetty-security/src/main/java/org/eclipse/jetty/security/IdentityService.java
===================================================================
--- jetty/trunk/jetty-security/src/main/java/org/eclipse/jetty/security/IdentityService.java	2009-04-21 03:11:25 UTC (rev 165)
+++ jetty/trunk/jetty-security/src/main/java/org/eclipse/jetty/security/IdentityService.java	2009-04-21 04:34:48 UTC (rev 166)
@@ -30,8 +30,11 @@
     
     /* ------------------------------------------------------------ */
     /**
-     * Scope the {@link UserIdentity} to a {@link UserIdentity.Scope}.
-     * @param user The current user or null for no user associated.
+     * Associate a user identity with the current thread.
+     * This is called with as a thread enters the 
+     * {@link SecurityHandler#handle(String, javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)}
+     * method and then again with a null argument as that call exits.
+     * @param user The current user or null for no user to associated.
      */
     void associate(UserIdentity user);
     

Modified: jetty/trunk/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
===================================================================
--- jetty/trunk/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java	2009-04-21 03:11:25 UTC (rev 165)
+++ jetty/trunk/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java	2009-04-21 04:34:48 UTC (rev 166)
@@ -312,10 +312,8 @@
 
         // Get the base requests
         final Request base_request=(request instanceof Request)?((Request)request):HttpConnection.getCurrentConnection().getRequest();
-        final String old_servlet_name=base_request.getServletName();
         final String old_servlet_path=base_request.getServletPath();
         final String old_path_info=base_request.getPathInfo();
-        UserIdentity scoped_identity = null;
 
         DispatcherType type = base_request.getDispatcherType();
         Object request_listeners=null;

Added: jetty/trunk/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
===================================================================
--- jetty/trunk/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java	                        (rev 0)
+++ jetty/trunk/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java	2009-04-21 04:34:48 UTC (rev 166)
@@ -0,0 +1,104 @@
+package org.eclipse.jetty.servlet;
+
+import java.io.File;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.eclipse.jetty.server.LocalConnector;
+import org.eclipse.jetty.server.Server;
+
+public class DefaultServletTest extends TestCase
+{
+    private Server server;
+    private LocalConnector connector;
+    private ServletContextHandler context;
+
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+
+        server = new Server();
+        server.setSendServerVersion(false);
+
+        connector = new LocalConnector();
+
+        context = new ServletContextHandler();
+        context.setContextPath("/context");
+        context.setWelcomeFiles(new String[] {}); // no welcome files
+
+        server.setHandler(context);
+        server.addConnector(connector);
+
+        server.start();
+    }
+
+    protected void tearDown() throws Exception
+    {
+        super.tearDown();
+
+        if (server != null)
+        {
+            server.stop();
+        }
+    }
+
+    public void testListingXSS() throws Exception
+    {
+        ServletHolder defholder = context.addServlet(DefaultServlet.class,"/*");
+        defholder.setInitParameter("dirAllowed","true");
+        defholder.setInitParameter("redirectWelcome","false");
+        defholder.setInitParameter("gzip","false");
+
+        File resBase = new File("src/test/resources");
+
+        assertTrue("resBase.exists",resBase.exists());
+        assertTrue("resBase.isDirectory",resBase.isDirectory());
+
+        String resBasePath = resBase.getAbsolutePath();
+        defholder.setInitParameter("resourceBase",resBasePath);
+
+        StringBuffer req1 = new StringBuffer();
+        req1.append("GET /context/org/mortbay/resource/;<script>window.alert(\"hi\");</script> HTTP/1.1\n");
+        req1.append("Host: localhost\n");
+        req1.append("\n");
+
+        String response = connector.getResponses(req1.toString());
+
+        assertResponseContains("org/mortbay/resource/one/",response);
+        assertResponseContains("org/mortbay/resource/two/",response);
+        assertResponseContains("org/mortbay/resource/three/",response);
+
+        assertResponseNotContains("<script>",response);
+    }
+
+    private void assertResponseNotContains(String forbidden, String response)
+    {
+        int idx = response.indexOf(forbidden);
+        if (idx != (-1))
+        {
+            // Found (when should not have)
+            StringBuffer err = new StringBuffer();
+            err.append("Response contain forbidden string \"").append(forbidden).append("\"");
+            err.append("\n").append(response);
+
+            System.err.println(err);
+            throw new AssertionFailedError(err.toString());
+        }
+    }
+
+    private void assertResponseContains(String expected, String response)
+    {
+        int idx = response.indexOf(expected);
+        if (idx == (-1))
+        {
+            // Not found
+            StringBuffer err = new StringBuffer();
+            err.append("Response does not contain expected string \"").append(expected).append("\"");
+            err.append("\n").append(response);
+
+            System.err.println(err);
+            throw new AssertionFailedError(err.toString());
+        }
+    }
+}


Property changes on: jetty/trunk/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
___________________________________________________________________
Name: svn:eol-style
   + native

Modified: jetty/trunk/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
===================================================================
--- jetty/trunk/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java	2009-04-21 03:11:25 UTC (rev 165)
+++ jetty/trunk/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java	2009-04-21 04:34:48 UTC (rev 166)
@@ -75,13 +75,17 @@
                 char c=path.charAt(i);
                 switch(c)
                 {
-                  case '%':
-                  case '?':
-                  case ';':
-                  case '#':
-                  case ' ':
-                      buf=new StringBuilder(path.length()<<1);
-                      break loop;
+                    case '%':
+                    case '?':
+                    case ';':
+                    case '#':
+                    case '\'':
+                    case '"':
+                    case '<':
+                    case '>':
+                    case ' ':
+                        buf=new StringBuilder(path.length()<<1);
+                        break loop;
                 }
             }
             if (buf==null)
@@ -107,6 +111,18 @@
                   case '#':
                       buf.append("%23");
                       continue;
+                  case '"':
+                      buf.append("%22");
+                      continue;
+                  case '\'':
+                      buf.append("%27");
+                      continue;
+                  case '<':
+                      buf.append("%3C");
+                      continue;
+                  case '>':
+                      buf.append("%3E");
+                      continue;
                   case ' ':
                       buf.append("%20");
                       continue;

Modified: jetty/trunk/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java
===================================================================
--- jetty/trunk/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java	2009-04-21 03:11:25 UTC (rev 165)
+++ jetty/trunk/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java	2009-04-21 04:34:48 UTC (rev 166)
@@ -429,7 +429,7 @@
         Arrays.sort(ls);
         
         String decodedBase = URIUtil.decodePath(base);
-        String title = "Directory: "+decodedBase;
+        String title = "Directory: "+deTag(decodedBase);
 
         StringBuilder buf=new StringBuilder(4096);
         buf.append("<HTML><HEAD><TITLE>");
@@ -440,9 +440,9 @@
         
         if (parent)
         {
-            buf.append("<TR><TD><A HREF=");
-            buf.append(URIUtil.addPaths(base,"../"));
-            buf.append(">Parent Directory</A></TD><TD></TD><TD></TD></TR>\n");
+            buf.append("<TR><TD><A HREF=\"");
+            URIUtil.encodePath(buf,URIUtil.addPaths(base,"../"));
+            buf.append("\">Parent Directory</A></TD><TD></TD><TD></TD></TR>\n");
         }
         
         DateFormat dfmt=DateFormat.getDateTimeInstance(DateFormat.MEDIUM,
@@ -457,9 +457,9 @@
             
             if (item.isDirectory() && !path.endsWith("/"))
                 path=URIUtil.addPaths(path,URIUtil.SLASH);
-            buf.append(path);
+            URIUtil.encodePath(buf,path);
             buf.append("\">");
-            buf.append(StringUtil.replace(StringUtil.replace(ls[i],"<","&lt;"),">","&gt;"));
+            buf.append(deTag(ls[i]));
             buf.append("&nbsp;");
             buf.append("</TD><TD ALIGN=right>");
             buf.append(item.length());
@@ -473,6 +473,10 @@
         return buf.toString();
     }
     
+    private static String deTag(String raw) {
+        return StringUtil.replace( StringUtil.replace(raw,"<","&lt;"), ">", "&gt;");
+    }
+    
     /* ------------------------------------------------------------ */
     /** 
      * @param out 

Modified: jetty/trunk/jetty-util/src/test/java/org/eclipse/jetty/util/URITest.java
===================================================================
--- jetty/trunk/jetty-util/src/test/java/org/eclipse/jetty/util/URITest.java	2009-04-21 03:11:25 UTC (rev 165)
+++ jetty/trunk/jetty-util/src/test/java/org/eclipse/jetty/util/URITest.java	2009-04-21 04:34:48 UTC (rev 166)
@@ -57,6 +57,9 @@
         URIUtil.encodeString(buf,"foo%23;,:=b a r",";,= ");
         assertEquals("foo%2523%3b%2c:%3db%20a%20r",buf.toString());
         
+        buf.setLength(0);
+        URIUtil.encodePath(buf,"/context/'list'/\"me\"/;<script>window.alert('xss');</script>");
+        assertEquals("/context/%27list%27/%22me%22/%3B%3Cscript%3Ewindow.alert(%27xss%27)%3B%3C/script%3E", buf.toString());
     }    
     
     /* ------------------------------------------------------------ */