Interesting (Real world) Security Issue

I found an interesting security bug today in some code at a client's site. (And unlike some other security holes, this was is real.) The problem was that the logon system was letting people in if they had an invalid logon. Users were correctly blocked at first, but as soon as they had an invalid logon, and then reloaded, they were let in. Why? Consider this code and make a guess before I show the answer:

<CFIF NOT ISDefined("Session.ProLoginOK")> <CFLOCATION URL="login/login.cfm" ADDTOKEN="No"> <CFELSEIF Session.ProLoginOK IS ""> <CFLOCATION URL="login/login.cfm" ADDTOKEN="No"> </CFIF>

Figured it out? On an invalid login, the code set session.prologinok to "No". Since "No" defined the variable, and wasn't "", there was no forced logon template run.

Something to watch out for in your own code!

Archived Comments

Comment 1 by Nick Tong posted on 1/25/2006 at 10:48 PM

Without not wishing to to be pedantic should the client not use 'structKeyExists'?

Comment 2 by Raymond Camden posted on 1/25/2006 at 10:50 PM

I am a late convert to SKE over isDefined. You will see most of my code (from my downloads) uses isDefined. I'm not trying to use SKE instead. In this case though I just fixed the client security problem.

Comment 3 by jj posted on 1/25/2006 at 10:54 PM

Where does Session.ProLoginOk get set to 'No'? And how does it set the variable?

Comment 4 by Raymond Camden posted on 1/25/2006 at 10:59 PM

It was set in the validation. I assume the client was using it as a flag to show a msg ("Your logon was sucky...").

Comment 5 by Raymond Camden posted on 1/25/2006 at 11:00 PM

Oh - and I don't want to paste any more code from the client since - well, it's not my code. :)

Comment 6 by jj posted on 1/26/2006 at 1:04 AM

Looks like Ray Horn has found some other security holes, oh and it looks like he sold "his" blog software for $1.5million! LOL...Funniest. Thing. Eva.

http://rayhorn.contentopia....

Comment 7 by Raymond Camden posted on 1/26/2006 at 1:09 AM

One note on that. While his first point is just.... wrong, his second point is almost right. I'm going to remove the email address when comments are sent out.

Comment 8 by jhey posted on 1/26/2006 at 2:17 AM

Security Issue?

Shouldn't that have been tested some how early in the development phases?

Comment 9 by Raymond Camden posted on 1/26/2006 at 2:19 AM

Jhey - of course. This was code definitely -not- written by myself or my company.

Comment 10 by Michael posted on 1/31/2006 at 9:53 PM

Lol .. I ran into one of these myself when a client clicked and it automatically logged him in during testing.

I learned my lesson!

As a side note - I just went to rabid lunatics site - wow ! - a true disgrace to web design. THIS IS A READ ONLY BLOG. Just laughable .. Im going to wash my hands now, I think I got some crap on my fingers just by clicking around.