Ben Nadel
On User Experience (UX) Design, JavaScript, ColdFusion, Node.js, Life, and Love.
I am the chief technical officer at InVision App, Inc - a prototyping and collaboration platform for designers, built by designers. I also rock out in JavaScript and ColdFusion 24x7.
Meanwhile on Twitter
Loading latest tweet...
Ben Nadel at CFUNITED 2010 (Landsdown, VA) with: Rolando Lopez

Large Mistake In My Session Management Logic

By Ben Nadel on
Tags: ColdFusion

I do a good amount of work in my Application.cfc to ensure that spiders, bots, and uncool people do not create unused session variables on my server. This has some processing overhead associated with it. To balance that out, I use some cookies to keep track of whether or not my session management logic has already been performed. Last night, while reviewing some of my own code, I realized that I had a HUGE hole in my logic around the short-circuit evaluation of an IF statement.

Here is the original statement:

  • // Check user agent.
  • if (
  • (NOT Len(strTempUserAgent)) OR
  •  
  • // We are testing the cookie values so that we are not
  • // duplicating logic. This should provide a performance
  • // increase of anyone accepting cookies.
  • (
  • COOKIE.SessionScopeTested AND
  • (NOT COOKIE.HasSessionScope)
  • ) OR
  •  
  • ... MANY OTHER CHECKS ...
  •  
  • } else {
  •  
  • ... LOGIC FOR SESSION-BASED USERS ...
  •  
  • }

My original thought was that if the use had already been tested and did NOT have the session scope enabled, then I could skip directly down to the ELSE statement. However, I totally spaced out on the fact that the first statement is a bunch of OR statements and therefore, even if that second statement was false, no short-circuiting would take place. Furthermore, I was checking to see if a user did NOT have the session scope. I don't care about that. I only care about people who HAVE a session scope. My whole mindset was wrong. I have fixed this by changing my session check and breaking the IF statement in to two statements AND'ed together:

  • // Check user agent and session testing.
  • if (
  • // We need to and this clause to force the user to
  • // go to the ELSE clause if this is False. That will
  • // short-circuit this IF statement and skip the rest
  • // of the processing. We want to skip if: The session
  • // scope has been tested AND they have the session scope.
  • (
  • NOT (
  • COOKIE.SessionScopeTested AND
  • COOKIE.HasSessionScope
  • )
  • )
  • AND
  • // Assuming we make it this far, we are dealing either
  • // with users who have not yet been tested or do NOT
  • // have a session scope. Either way let's try to
  • // short-circuit this by testing for session.
  • (
  • // Check to see if we have tested this user to NOT
  • // have a session scope available.
  • (
  • COOKIE.SessionScopeTested AND
  • (NOT COOKIE.HasSessionScope)
  • ) OR
  •  
  • // If we made it this far, then we have to fully
  • // test the user.
  • (NOT Len(strTempUserAgent)) OR
  •  
  • ... MANY OTHER CHECKS ...
  •  
  • } else {
  •  
  • ... LOGIC FOR SESSION-BASED USERS ...
  •  
  • }

As you can see from the updated IF statement, we are now creating an environment for good short-circuiting. Now, if the user has already been tested and HAS a cookie scope, then the first AND clause will evaluate to NOT TRUE, which will be FALSE, which will short-circuit the parent IF statement and jump over all the non-session based testing and proceed directly to the application setting for user's with a session scope.




Reader Comments

Thanks for this great code. It was very useful as I've been having some issues at our hosting provider and I think this will clear up a lot of it. Thanks!!!

Hmm.. I could have sworn I saw him say that somewhere. I thought it was about this topic but maybe I got confused. Either way, I think it would be of help to users out there. I wrote an article a while back about <a href="http://cfdj.sys-con.com/read/42049.htm">Defeating DoS Attacks</a> but I've since stopped using that technique because the spiders were screwing with it so much.

I've implemented your solution now, and I think it would be a good refresh to some of the issues that article dealt with.

Thanks Ben for publicly exploring this issue in such detail. Along with Michael D's posts it's really helped me get a handle on it.

In case you're interested I've come up with a somewhat simpler implementation which still seems to cover your bases, although I'm with Michael in preferring to limit the timeout rather than disable sessions (no reason why you couldn't though I don't think).

This is all at the top (pseudo-constructor) of Application.cfc:
-------------------

// 1) Set a flag testing the existence of a cookie set on a prior request (NB: no param-ing):

request.sessionCookiesSet = structKeyExists(cookie,"sessionCookiesSet");

// 2 If it exists this must be the second or later request so we can be sure the client is a "normal" browser with cookies enabled so give them a decent session timeout and skip the other stuff:

if(request.sessionCookiesSet){
this.sessionTimeout = createTimeSpan(0,3,0,0);
} else {

// If no cookie found then it could mean it's:
// 1) the first request by a "normal" visitor OR
// 2) a browser with cookies disabled OR
// 3) a bot

// We'll assume either 2) or 3) and set a short timeout:

this.sessionTimeout = createTimeSpan(0,0,0,1);

// if it's 1) then no problem because this will be overridden on the next request
// At this point you can distinguish bots from users with cookies disabled. I use a UDF. For normal cookie-accepting browsers this will only fire on the first request, so they won't be troubled by it subsequently:

request.isBot = myUdfs.isBot();
}

// Now set the cookie which will make sure normal users aren't bothered by the tests on the next request. Use your own preferred cookie setting method depending on how permanent you want them etc..

cookie.sessionCookiesSet=true;

-----------------
If you see any glaring holes please point them out, but since setting it up a few hours ago on a reasonably busy public site the number of unnecessary sessions seems to be dropping away quite nicely.

Julian,

That looks pretty nice to me. I will probably switch over to something like this. My original thinking was to try an eliminate all session stuff. But if Michael is allowing bots to have very short sessions than I trust that methodology (as my original method was based on his suggestions).

Thanks for pseudo-coding this out for me. When my code is updated, I will post so that we are on the same page (more or less).

Ben, I don't see why you couldn't disable sessionManagement for bots and users with cookies off instead of just changing the timeout. That should save even more overhead as the vars aren't even created. But you're presumably also having to be very careful with all session references in your code, no?

Actually, that's a good point. I don't really use session information anywhere on the site. I think (but not 100% sure off the top of my head) that I could probably turn off session management all together and it would work.

I would like to come up with a generic boilerplate for this, though. So even if it is not required, it would be nice to have it fleshed out for use with future applications.

I've decided to take a slightly different approach for dealing with extraneous sessions in a new version of my site I've been working on (I don't do much web work in my day job anymore, so this is more a hobby)

My default session time out is 2 seconds.

I use cookies for any settings that don't have to be secure, ie colour, font and layout options for the casual users.

I only start to use sessions when user logs in.

When a user logs in I increase the session timeout to 60 min, but with the option for user customization (for accessibility issues), and I'm also looking at including a keep-alive option using javascript.

Also to simplify my code in other areas I wanted all sessions to contain a user object, so I can call functions like session.user.isValidated(); to see if the user has logged in, or show the user name on each page using session.user.getName(); without having to do existence checks.

I didn't want a new user object created for each session, so I created a "proxy" user object with role=guest, name=unknown and other default values in the APPLICATION scope and created a reference to the "proxy" in the SESSION scope.

In applicaiton.cfc, onSessionStart
session.user = application.userproxy

Once the user "logs in", the "proxy" is replaced with an instance of the real user object containing the information for the validated user.

When the user "logs out", the user object is replaced by the proxy.

Bit of a side track...

I forgot to mention that if you are not using ColdFusion 8 and are hosted on a shared ColdFusion MX server, you session scope is probably not secure... see the code below:

<cfapplication name="">
<cfloop collection="#application#" item="s_application">
<cfscript>
o_sessions = createObject("java","coldfusion.runtime.SessionTracker");
o_activesessions = o_sessions.getSessionCollection('#s_application#');
i_activeSessions = ListLen(structkeyList(o_activeSessions));
</cfscript>
<cfif i_activeSessions>
<cfoutput>
<h3>Total Sessions for #s_application#: #i_activeSessions#</h3>
</cfoutput>
<cfdump var="#o_activesessions#">
</cfif>
</cfloop>

If you run this on your hosting server, and see a list of all active applications, and all active sessions within each application... well if security is a major concern for your site, you may want to start looking at other hosting servers.

Why?

Things get really scary when you use o_sessions.getSession("asessionid") with one of the session ids you just listed. You get a pointer to the session that you can use to manipulate any variables or objects contained within the session, even for sessions that are not in your application.

Coldfusion 8 added an admin option to disable access to internal ColdFusion Java components ie. "coldfuion.", if this option is checked, the script above will throw an error "Permission denied for creating Java object: coldfusion.runtime.SessionTracker"

Luckily regular java calls still work

@Steve,

I really like the whole shared application CFC method that you use for users that are not logged in. Genius. Seems like such a smart way to perform memory management.

Thanks for the good advice.

I've been tweaking my approach.

Since I only use sessions when a user is logged, I wondered if I could just kill the session if it wasn't needed.

I have a function isKnownUser() in my SessionManager (the proxy always returns false)

I tried adding the following lines to my onRequestEnd
if(SESSION.sks.manager.isKnownUser()) structClear(SESSION)
(.sks. is the scope I use to prevent conflict with other peoples code)

Coldfusion returned an error that the SESSION scope wasn't defined... Something I'll need to look in to further some time.

So I tried adding the following line to onRequestStart
REQUEST.sks.session = SESSION

So my code in onRequestEnd became
if (NOT REQUEST.SESSION.sks.manager.isKnownUser()) structClear(REQUEST.sks.SESSION);

This seems to work. If you use the session lister code I mentioned in an earlier comment you'll see that the session is now an empty structure. (you have to reload the page with the lister to see the result of the onRequestEnd function from the last page load)

I also discovered using the session lister that coldfusion seems to clear expired sessions on a 10 second schedule.

So in summary what I've cooked up so far is:

In general:
THIS.clientManagement = false;
THIS.sessionManagement = true;
THIS.loginStorage = "session";
THIS.setClientCookies = false;

I use cookies contain things that I don't care if a user tries to hack, such as personal preferences for font settings etc.
All cookies are set manually using cfcookie or COOKIE.name = value;

If a user is not logged in:
Sessions time out after 2 seconds (still useful to reduce the number of session instances)
I use a "Proxy" stored in my APPLICATION scope for managing the "guest" user's session, this prevents creating a component for each new session.
SESSION.sks.manager = APPLICATION.sks.sessionManagerProxy
The Session scope is cleared at the end of the request (onRequestEnd)

Once a user logs in:
Cookies are manually set (Cookie.cfid = Session.cfid)
Sessions time out after 30 minutes
The "Proxy" is replaced with a unique session manager for the verified user's session.
SESSION.sks.manger = newInstance("sessionManager").init(o_user); (where o_user is a verified user object)

When a user logs out:
custom session manager is replaced with the proxy (user changed back in to a guest)
cookies linking the session to the user are set to expire
<cfcookie name="cfid" expires="NOW">
<cfcookie name="cftoken" expires="NOW">

Why don't I just shut of sessions when a user isn't logged in?
I didn't what to clutter up my code with existence checks.
With this approach there will always be a sessionManager that I can can call, either a "real" one unique to a known user, or the "proxy" one used for unknown users.

@Steve,

Like I said before, I really like the way you have this proxy object singleton for all non-logged-in users. I think that is so clever.

I am very cautious about your calling StructClear() no your SESSION object. When you do this, you will be clearing your CFID and CFTOKEN values from the SESSION; I see that you are storing the ID and Token in the cookie, but I would opt to keep it in the session as well (just me).

Also, I am not sure I follow the reason for calling StructClear(). This action does not actually end the session, as far as I know, it just clears that structure.

But very interesting stuff you got here.ideas

You're right, I probably don't need to call structClear on my session scope with the way I currently have my site set up.

Thanks for your feedback.