Bug 36995 - duplicate session ids
Summary: duplicate session ids
Status: RESOLVED INVALID
Alias: None
Product: Tomcat 4
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 4.1.31
Hardware: PC other
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-10 18:15 UTC by Christian Roslawski
Modified: 2005-10-12 06:09 UTC (History)
0 users



Attachments
A simple test case to check for duplicate session ids. (2.44 KB, text/plain)
2005-10-11 14:01 UTC, Christian Roslawski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Roslawski 2005-10-10 18:15:38 UTC
Greetings,

We ran a simple load test against Tomcat 4.1.x after having some strange
session effects on our website, to see if the generated session ids are
really unique. The test made HTTP requests with wget, extracted the newly
generated session ids and wrote them into a file. It turned up two pairs
of duplicate session ids from active sessions.

I read the source code to see how the session ids are generated, and found
this code in org.apache.catalina.session.ManagerBase#createSession() from
Tomcat 4.1.31, line 544 to 555:

        synchronized (sessions) {
            while (sessions.get(sessionId) != null){ // Guarantee uniqueness
                sessionId = generateSessionId();
                duplicates++;
                // @todo Move appending of jvmRoute generateSessionId()???
                if (jvmRoute != null) {
                    sessionId += '.' + jvmRoute;
                }
            }
        }

        session.setId(sessionId);

I came up with the following scenario, where duplicate session ids could
be generated:

Thread 1 runs into the method, generates a session id, checks if the session
id is unique and leaves the synchronized block. There the thread 1 pauses and
thread 2, a request from a different user, takes over.

Thread 2 runs into the method, generates the same session id by pure chance
and checks if the session id is unique. Since thread 1 has not yet called
setId, the session id isn't stored in the HashMap, the session id is
considered unique and the loop is left. The method setId() is called, which
results in a call to the method add() of the ManageBase class. There the new
session object is stored in the HashMap with the session id.

Thread 1 continues and calls setId() as well, overwriting the already present
session object in the HashMap with a new one.

Both threads continue and return a duplicate session id to different user.
Both users use the same session object for subsequent requests.

I know the chances for this scenario are slim, as it must generate the same
session id and run into the race condition between the two syncronised blocks
in createSession() and add(), but it appears to be happening.

Shouldn't the check for uniqueness and the adding of the newly generated
session id be in the same syncronized block of "sessions", to really
ensure uniqueness? Or did I get something wrong here?

Tomcat 5.0.28 and Tomcat 5.5.9 appears to have the same problem, btw.

Thanks,

Christian
Comment 1 Peter Rossbach 2005-10-10 19:58:08 UTC
I hava look inside also inside 5.5.x code (ManagerBase.generateSessionId()) and
think the duplication risk is there. But the risk is small, as random generator
works really good. I have test with Linux Suse 9.3 and have no chance to
reproduce your test result. Can you please send your testscripts and os information?

Thanks
Peter 
Comment 2 Remy Maucherat 2005-10-11 09:09:17 UTC
(In reply to comment #1)
> I hava look inside also inside 5.5.x code (ManagerBase.generateSessionId()) and
> think the duplication risk is there. But the risk is small, as random generator
> works really good. I have test with Linux Suse 9.3 and have no chance to
> reproduce your test result. Can you please send your testscripts and os
information?
> 

This does not make any sense. The risk does not exist, and the uniqueness check
is just there to make insecure people more secure. I am actually tired of it,
and would want it removed.
Comment 3 Christian Roslawski 2005-10-11 14:01:58 UTC
Created attachment 16657 [details]
A simple test case to check for duplicate session ids.
Comment 4 Christian Roslawski 2005-10-11 14:02:45 UTC
We have Suse 8.2 with kernel 2.4.20-64GB-SMP on our servers. Java version is
1.4.2_03 and Tomcat 4.1.29.

As the chances for the described scenario are slim, I suggest to reduce the
value of ManagerBase.SESSION_ID_BYTES from 16 to 2 or 3 for testing. This
should increase the chances of duplicates returned by
ManagerBase.generateSessionId() without affecting the behaviour of Tomcat.

Additionally, I put a Thread.yield() below the end of the sychronized block
in ManagerBase.createSession(), to provoke the racing time condition, further
increasing the chances for the scenario.

Then I started Tomcat with the JSP page "session.jsp":

<%@ page language="java" %><%= request.getSession().getId() %>

The test application performs repeated request from different threads,
recoding the returned session ids and checking for duplicates. Even with
the reduced random range it might take several runs to stumble into a
duplicate. I'm sure there are better ways to test it, it is just a simple
test.

I'm not saying this is an urgent problem, or that it happens all the time, I
merely think that it can happen, because random numbers, however large the
range might be, are not unique by themselves, are they? And if it can happen,
it will happen, eventually. Or am I missing something here?
Comment 5 Remy Maucherat 2005-10-12 14:09:21 UTC
(In reply to comment #4)
> We have Suse 8.2 with kernel 2.4.20-64GB-SMP on our servers. Java version is
> 1.4.2_03 and Tomcat 4.1.29.
> 
> As the chances for the described scenario are slim, I suggest to reduce the
> value of ManagerBase.SESSION_ID_BYTES from 16 to 2 or 3 for testing. This
> should increase the chances of duplicates returned by
> ManagerBase.generateSessionId() without affecting the behaviour of Tomcat.
> 
> Additionally, I put a Thread.yield() below the end of the sychronized block
> in ManagerBase.createSession(), to provoke the racing time condition, further
> increasing the chances for the scenario.
> 
> Then I started Tomcat with the JSP page "session.jsp":
> 
> <%@ page language="java" %><%= request.getSession().getId() %>
> 
> The test application performs repeated request from different threads,
> recoding the returned session ids and checking for duplicates. Even with
> the reduced random range it might take several runs to stumble into a
> duplicate. I'm sure there are better ways to test it, it is just a simple
> test.
> 
> I'm not saying this is an urgent problem, or that it happens all the time, I
> merely think that it can happen, because random numbers, however large the
> range might be, are not unique by themselves, are they? And if it can happen,
> it will happen, eventually. Or am I missing something here?

The whole idea of even checking for duplicates is a nonsense (beyond giving
people some sense of safety). If the id generation has conflicts, then it means
the system is completely insecure, so fixing a bug which will never actually
occur doesn't have any benefits.