Tim Funk wrote:
If I read this correctly (big if), the solution could be a security problem. If I were a phisher, I might be able to send you an email with a link to that would redirect you to a tomcat server with the new configuration in question with a special id. If the user were to perform other actions of a confidential nature, the attacker could swoop in and impersonate that session.

The phisher could easily be notified that the special session id was used and have a farm of wamr bodies ready to attack.

My patch at the moment is attached to the email for more detailed review. It no longer reuses session ids submitted in the URL (very good catch).


Rémy
Index: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java
===================================================================
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java,v
retrieving revision 1.14
diff -u -r1.14 Manager.java
--- jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java 
7 Sep 2004 20:57:02 -0000       1.14
+++ jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/Manager.java 
7 Feb 2005 16:49:28 -0000
@@ -257,6 +257,7 @@
      */
     public void addPropertyChangeListener(PropertyChangeListener listener);
 
+
     /**
      * Get a session from the recycled ones or create a new empty one.
      * The PersistentManager manager does not need to create session data
@@ -264,17 +265,22 @@
      */                                                                        
 
     public Session createEmptySession();
 
+
     /**
      * Construct and return a new session object, based on the default
      * settings specified by this Manager's properties.  The session
-     * id will be assigned by this method, and available via the getId()
-     * method of the returned session.  If a new session cannot be created
-     * for any reason, return <code>null</code>.
-     *
+     * id specified will be used as the session id.
+     * If a new session cannot be created for any reason, return 
+     * <code>null</code>.
+     * 
+     * @param sessionId The session id which should be used to create the
+     *  new session; if <code>null</code>, the session
+     *  id will be assigned by this method, and available via the getId()
+     *  method of the returned session.
      * @exception IllegalStateException if a new session cannot be
      *  instantiated for any reason
      */
-    public Session createSession();
+    public Session createSession(String sessionId);
 
 
     /**
Index: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java
===================================================================
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java,v
retrieving revision 1.18
diff -u -r1.18 Request.java
--- 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java
       20 Nov 2004 21:10:47 -0000      1.18
+++ 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java
       7 Feb 2005 16:49:29 -0000
@@ -2196,7 +2196,14 @@
               (sm.getString("coyoteRequest.sessionCreateCommitted"));
         }
 
-        session = manager.createSession();
+        // Attempt to reuse session id if one was submitted in a cookie
+        // Do not reuse the session id if it is from a URL, to prevent possible
+        // phishing attacks
+        if (isRequestedSessionIdFromCookie()) {
+            session = manager.createSession(getRequestedSessionId());
+        } else {
+            session = manager.createSession(null);
+        }
 
         // Creating a new session cookie based on that session
         if ((session != null) && (getContext() != null)
Index: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/ApplicationHttpRequest.java
===================================================================
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/ApplicationHttpRequest.java,v
retrieving revision 1.24
diff -u -r1.24 ApplicationHttpRequest.java
--- 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/ApplicationHttpRequest.java
     15 Jan 2005 20:31:21 -0000      1.24
+++ 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/core/ApplicationHttpRequest.java
     7 Feb 2005 16:49:29 -0000
@@ -529,13 +529,8 @@
                     // Ignore
                 }
                 if (localSession == null && create) {
-                    localSession = context.getManager().createEmptySession();
-                    localSession.setNew(true);
-                    localSession.setValid(true);
-                    localSession.setCreationTime(System.currentTimeMillis());
-                    localSession.setMaxInactiveInterval
-                        (context.getManager().getMaxInactiveInterval());
-                    localSession.setId(other.getId());
+                    localSession = 
+                        context.getManager().createSession(other.getId());
                 }
                 if (localSession != null) {
                     localSession.access();
Index: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/ManagerBase.java
===================================================================
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/ManagerBase.java,v
retrieving revision 1.37
diff -u -r1.37 ManagerBase.java
--- 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/ManagerBase.java
     22 Nov 2004 14:50:23 -0000      1.37
+++ 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/ManagerBase.java
     7 Feb 2005 16:49:29 -0000
@@ -727,15 +727,18 @@
     /**
      * Construct and return a new session object, based on the default
      * settings specified by this Manager's properties.  The session
-     * id will be assigned by this method, and available via the getId()
-     * method of the returned session.  If a new session cannot be created
-     * for any reason, return <code>null</code>.
-     *
+     * id specified will be used as the session id.  
+     * If a new session cannot be created for any reason, return 
+     * <code>null</code>.
+     * 
+     * @param sessionId The session id which should be used to create the
+     *  new session; if <code>null</code>, a new session id will be
+     *  generated
      * @exception IllegalStateException if a new session cannot be
      *  instantiated for any reason
      */
-    public Session createSession() {
-
+    public Session createSession(String sessionId) {
+        
         // Recycle or create a Session instance
         Session session = createEmptySession();
 
@@ -744,15 +747,17 @@
         session.setValid(true);
         session.setCreationTime(System.currentTimeMillis());
         session.setMaxInactiveInterval(this.maxInactiveInterval);
-        String sessionId = generateSessionId();
+        if (sessionId == null) {
+            sessionId = generateSessionId();
+        }
         session.setId(sessionId);
         sessionCounter++;
 
         return (session);
 
     }
-
-
+    
+    
     /**
      * Get a session from the recycled ones or create a new empty one.
      * The PersistentManager manager does not need to create session data
Index: 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/StandardManager.java
===================================================================
RCS file: 
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/StandardManager.java,v
retrieving revision 1.27
diff -u -r1.27 StandardManager.java
--- 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/StandardManager.java
 22 Nov 2004 16:35:18 -0000      1.27
+++ 
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/StandardManager.java
 7 Feb 2005 16:49:29 -0000
@@ -278,7 +278,7 @@
      * @exception IllegalStateException if a new session cannot be
      *  instantiated for any reason
      */
-    public Session createSession() {
+    public Session createSession(String sessionId) {
 
         if ((maxActiveSessions >= 0) &&
             (sessions.size() >= maxActiveSessions)) {
@@ -287,7 +287,7 @@
                 (sm.getString("standardManager.createSession.ise"));
         }
 
-        return (super.createSession());
+        return (super.createSession(sessionId));
 
     }
 
Index: 
jakarta-tomcat-catalina/modules/cluster/src/share/org/apache/catalina/cluster/session/DeltaManager.java
===================================================================
RCS file: 
/home/cvs/jakarta-tomcat-catalina/modules/cluster/src/share/org/apache/catalina/cluster/session/DeltaManager.java,v
retrieving revision 1.36
diff -u -r1.36 DeltaManager.java
--- 
jakarta-tomcat-catalina/modules/cluster/src/share/org/apache/catalina/cluster/session/DeltaManager.java
     22 Nov 2004 14:51:18 -0000      1.36
+++ 
jakarta-tomcat-catalina/modules/cluster/src/share/org/apache/catalina/cluster/session/DeltaManager.java
     7 Feb 2005 16:49:30 -0000
@@ -235,8 +235,8 @@
      * @exception IllegalStateException if a new session cannot be
      *  instantiated for any reason
      */
-    public Session createSession() {
-        return createSession(true);
+    public Session createSession(String sessionId) {
+        return createSession(sessionId, true);
     }
 
     
@@ -247,7 +247,7 @@
      * @param distribute
      * @return
      */
-    public Session createSession(boolean distribute) {
+    public Session createSession(String sessionId, boolean distribute) {
 
       if ((maxActiveSessions >= 0) &&
           (sessions.size() >= maxActiveSessions)) {
@@ -258,13 +258,16 @@
 
       // Recycle or create a Session instance
       DeltaSession session = getNewDeltaSession();
-      String sessionId = generateSessionId();
-
-      synchronized (sessions) {
-        while (sessions.get(sessionId) != null) { // Guarantee uniqueness
-          duplicates++;
+      if (sessionId == null) {
           sessionId = generateSessionId();
-         }
+          
+          synchronized (sessions) {
+              while (sessions.get(sessionId) != null) { // Guarantee uniqueness
+                  duplicates++;
+                  sessionId = generateSessionId();
+              }
+          }
+
       }
 
       session.setNew(true);
@@ -849,7 +852,8 @@
                        if (log.isDebugEnabled())
                            log.debug("Manager (" + name + ") received session 
("
                             + msg.getSessionID() + ") created.");
-                       DeltaSession session = 
(DeltaSession)createSession(false);
+                       DeltaSession session = 
+                           (DeltaSession)createSession(msg.getSessionID(), 
false);
                        // Q: Why inform all session listener a replicate node?
                        session.setId(msg.getSessionID());
                        session.setNew(false);

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to