You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2003/01/08 18:03:16 UTC

DO NOT REPLY [Bug 15894] New: - Access to other sessions possible (session is immediately recycled after invalidation/expiration)

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=15894>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=15894

Access to other sessions possible (session is immediately recycled after invalidation/expiration)

           Summary: Access to other sessions possible (session is
                    immediately recycled after invalidation/expiration)
           Product: Tomcat 3
           Version: 3.3.1 Final
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: Critical
          Priority: Other
         Component: Servlet
        AssignedTo: tomcat-dev@jakarta.apache.org
        ReportedBy: cwicke@ics.uci.edu


Hello 
 
it is possible to access a session of somebody else through calling 
session.invalidate. Here is an example JSP: 
 
<% 
  HttpSession s = request.getSession(true); 
  String name = request.getParameter("name"); 
  s.setAttribute("name", name); 
  Thread.sleep(5000); 
  s.invalidate(); 
  Thread.sleep(5000); 
  s = request.getSession(true); 
  String nameFromSession = (String) s.getAttribute("name"); %> 
<html> 
<head><title>Test Invalidate</title></head> 
<body bgcolor="white"> 
<font size=4> 
 
<% if (nameFromSession == null) { %> 
  second session was clean. 
<% } else { %> 
 
  Session not Empty after invalidate!!!<p> 
 
  I got the session from <%=nameFromSession%> 
<% } %> 
 
</font> 
</body> 
</html> 
 
Login on one browser with 
http://localhost:8080/examples/testInvalidate.jsp?name=Martin 
and then after 8 seconds on another browser (if cookies are disabled you can 
use two windows of the same browser) with 
http://localhost:8080/examples/testInvalidate.jsp?name=Waldemar 
You'll get the session of Martin. 
 
The problem is that SimpleSessionStore recycles the sessions right away. I 
attached a path to delay the recycling. 
The patch also touches three files to reduce racing problems. To fix the 
problem only SimpleSessionStore needs to be updated. 
 
Christian Wicke 
 
Here is my patch: 
 
diff -Naur ./facade22/org/apache/tomcat/facade/HttpSessionFacade.java 
../../../jakarta-tomcat-3.3.1-src/src/facade22/org/apache/tomcat/facade/HttpSessionFacade.java 
--- ./facade22/org/apache/tomcat/facade/HttpSessionFacade.java	Wed Jan  8 
15:17:18 2003 
+++ 
../../../jakarta-tomcat-3.3.1-src/src/facade22/org/apache/tomcat/facade/HttpSessionFacade.java	
Tue Mar 26 16:36:48 2002 
@@ -148,8 +148,7 @@ 
      * @exception IllegalStateException if this method is called on 
      *  an invalidated session 
      */ 
-    // avoid parallel call to invalidate through synchronized 
-    public synchronized void invalidate() { 
+    public void invalidate() { 
 	checkValid(); 
  	realSession.getTimeStamp().setValid( false ); 
 	// remove all attributes 
diff -Naur ./share/org/apache/tomcat/modules/session/SessionExpirer.java 
../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SessionExpirer.java 
--- ./share/org/apache/tomcat/modules/session/SessionExpirer.java	Wed 
Jan  8 13:32:21 2003 
+++ 
../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SessionExpirer.java	
Tue Mar 26 16:36:49 2002 
@@ -159,14 +159,7 @@ 
 	} 
 
 	public void expired(TimeStamp o ) { 
-	  ServerSession sses=(ServerSession)o.getParent(); 
-        // double check if this session is not touched in 
-        // meantime by SessionId.processSession 
-       synchronized (sses) { 
-          if (System.currentTimeMillis() - o.getLastAccessedTime() < 
o.getMaxInactiveInterval()) 
-            // session touched in meantime, don't expire 
-            return; 
- 
+	    ServerSession sses=(ServerSession)o.getParent(); 
 	    if( debug > 0  ) { 
 		se.log( "Session expired " + sses); 
 	    } 
@@ -174,7 +167,6 @@ 
 	    // After expiring it, we clean up. 
 	    if( debug > 0 ) se.log( "Recycling " + sses); 
 	    sses.recycle(); 
-        } 
 	} 
     } 
 } 
diff -Naur ./share/org/apache/tomcat/modules/session/SessionId.java 
../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SessionId.java 
--- ./share/org/apache/tomcat/modules/session/SessionId.java	Wed Jan  8 
15:13:14 2003 
+++ 
../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SessionId.java	
Tue Mar 26 16:37:15 2002 
@@ -239,13 +239,6 @@ 
 				      sessionId,  false ); 
 	    if( sess!=null ) break; 
 	} 
-      if (sess == null) 
-        return null; 
-     // avoid parallel expiration of the SessionExpirer 
-     synchronized (sess) { 
-       //double check whether the session has been expired meantime 
-       if (sess.getState() == ServerSession.STATE_EXPIRED) 
-         return null; 
 
         /* The following block of code verifies if Tomcat session matches 
            SSL session (if one was ever passed to Tomcat). Just in case 
@@ -253,7 +246,7 @@ 
            We can't verify that if SSL is not used. */ 
 
         // Do this only if request is over SSL 
-        if(checkSSLSessionId && request.isSecure() ){ 
+        if(checkSSLSessionId && sess != null && request.isSecure() ){ 
           // SSL session ID from session and request - they have to be equal! 
           String 
ids=(String)sess.getAttribute("javax.servlet.session.ssl_session"), 
                  
idr=(String)request.getAttribute("javax.servlet.request.ssl_session"); 
@@ -283,7 +276,7 @@ 
 	    // it and adjust the session 
 	    request.setSession( sess ); 
 	    request.setSessionId( sessionId ); 
- 
+ 
 	    sess.touch( System.currentTimeMillis() ); 
 
 	    // if the session was NEW ( never accessed - change it's state ) 
@@ -291,10 +284,9 @@ 
 		sess.setState( ServerSession.STATE_ACCESSED, request); 
 	    } 
 	} 
-     } 
 	return sess; 
     } 
- 
+ 
 //     /** Fix the session id. If the session is not valid return null. 
 //      *  It will also clean up the session from load-balancing strings. 
 //      * @return sessionId, or null if not valid 
diff -Naur ./share/org/apache/tomcat/modules/session/SimpleSessionStore.java 
../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SimpleSessionStore.java 
--- ./share/org/apache/tomcat/modules/session/SimpleSessionStore.java	Tue 
Jan  7 14:21:48 2003 
+++ 
../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SimpleSessionStore.java	
Wed Jan  8 16:15:19 2003 
@@ -207,7 +207,9 @@ 
 	SimpleSessionManager sm= getManager( ctx ); 
 
 	if( sm == null ) { 
-	    sm=new SimpleSessionManager(); 
+          SessionRecycler r = new SessionRecycler(); 
+          r.startRecycler() 
+	    sm=new SimpleSessionManager(r); 
 	    sm.setDebug( debug ); 
 	    sm.setModule( this ); 
 	    ctx.getContainer().setNote( manager_note, sm ); 
@@ -236,16 +238,16 @@ 
 	    session.setState( ServerSession.STATE_SUSPEND ); 
 	    session.setState( ServerSession.STATE_EXPIRED ); 
 	} 
+      sm.getSessionRecycler.stopRecycler(); 
     } 
 
     public int sessionState( Request req, ServerSession session, int state ) 
{ 
 	TimeStamp ts=session.getTimeStamp(); 
 
 	if( state==ServerSession.STATE_EXPIRED ) { 
-	    // session moved to expire state - remove all attributes from 
-	    // storage 
-	    SimpleSessionManager 
ssm=(SimpleSessionManager)session.getManager(); 
-	    ssm.removeSession( session ); 
+	    // session moved to expire state - register to be recycled 
+	    ((SimpleSessionManager)session.getManager()).getSessionRecycler() 
+             .registerToRecycle(session); 
 	} 
 	return state; 
     } 
@@ -288,8 +290,9 @@ 
      * The actual "simple" manager 
      * 
      */ 
-    public static class SimpleSessionManager 
+    public static class SimpleSessionManager 
     { 
+      private SessionRecycler sessionRecycler; 
 	private int debug=0; 
 	private BaseInterceptor mod; 
 	/** The set of previously recycled Sessions for this Manager. 
@@ -302,8 +305,13 @@ 
 	 */ 
 	protected Hashtable sessions = new Hashtable(); 
 
-	public SimpleSessionManager() { 
+	public SimpleSessionManager(SessionRecycler r) { 
+        sessionRecycler = r 
 	} 
+ 
+      public SessionRecycler getSessionRecycler() { 
+        return sessionRecycler; 
+      } 
 
 	public void setDebug( int l ) { 
 	    debug=l; 
@@ -385,4 +393,62 @@ 
 	} 
 
     } 
+/** 
+ * Recycles Sessions after waiting for some time. 
+ * Fast Recycling is dangerous this it allows Threads to 
+ * access sessions after it's been recycled 
+ * 
+ * @author Christian Wicke cwicke@ics.uci.edu 
+ */ 
+ // Some of the code was borrowed from Expirer.java and Reaper.java 
+public final class SessionRecycler extends Thread { 
+    private long interval = 1000 * 60; //ms 
+    private Stack toBeRecycledNow; 
+    private Stack toBeRecycledLater; 
+    private boolean running; 
+ 
+    private SessionRecycler() { 
+      toBeRecycledNow = new Stack(); 
+      toBeRecycledLater = new Stack(); 
+    } 
+ 
+    public synchronized void startRecycler() { 
+      this.running = true; 
+      this.start(); 
+    } 
+ 
+    public synchronized void stopRecycler() { 
+      this.running = false; 
+      System.out.println("Stop recycler"); 
+      this.interrupt(); 
+    } 
+ 
+    public synchronized void registerToRecycle(ServerSession s) { 
+      toBeRecycledLater.push(s); 
+    } 
+ 
+    public void run() { 
+ 	while (running) { 
+	    if( !running) break; 
+	    try { 
+		this.sleep(interval); 
+	    } catch (InterruptedException ie) { 
+		// wake up when stopRecycler has been called 
+	    } 
+ 
+	    if( !running) break; 
+          while (!toBeRecycledNow.empty()) { 
+            ServerSession s = (ServerSession) toBeRecycledNow.pop(); 
+            SimpleSessionManager ssm=(SimpleSessionManager)s.getManager(); 
+            ssm.removeSession( s ); 
+          } 
+          synchronized (this) { 
+            Stack temp = toBeRecycledNow; 
+            toBeRecycledNow = toBeRecycledLater; 
+            toBeRecycledLater = temp; 
+          } 
+	} 
+    } 
+} 
+ 
 }

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>