Vanilla 1 is no longer supported or maintained. If you need a copy, you can get it here.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.
Options

Security Bug - Cookie Authentication

edited June 2006 in Vanilla 1.0 Help
I just found a considerably large Security issue with the Cookie Authentication used in Vanilla, so I thought it would be nice of me to share it. The current mechanism in place to keep Sessions persistant via Cookies uses the following code: $EncryptedUserID = ForceIncomingCookieString("pass", ""); $VerificationKey = ForceIncomingCookieString("name", ""); *snip* $s->SetMainTable("User", "u"); $s->AddSelect("UserID", "u"); $s->AddSelect("UserID", "u", "EncryptedUserID", "md5"); $s->AddWhere("md5(UserID)", $EncryptedUserID, "="); $s->AddWhere("VerificationKey", $VerificationKey, "="); From this we can tell that to return a valid cookie we need a UserID and the relevent Verification Key. So where is the flaw? Well it resides in the creation of the Verification Key which is a little more predictable than you might expect. Let us take a looka the code for DefineVerificationKey() to see where the problem is: function DefineVerificationKey() { return md5(str_replace(".","",GetRemoteIp()).time()); } At first this seems reasonable, however take a moment to think about it. From http://us3.php.net/time we find time() returns the current time measured in the number of seconds since the Unix Epoch (January 1 1970 00:00:00 GMT). The issue here is the use of seconds. The first challenge is to calculate the current Servers time. We already know our own IP, so how do we find the current Serverside timestamp? Given the Servers time is in sync, lets say your IP is 216.118.161.18, and your current local timestamp is 1149533263 and you just logged in. Your Verification Key would have 60*60*24 = 86,400 possiblities (24 hours). So your own Verification Key is going to be between MD5("21611816118"."1149490063") and MD5("21611816118"."1149576463"). Given that it should only take a few seconds to determine the current server time. All you need now is the UserID, IP and rough time of login to give a small set of Verification Key's to find a collision in. You could obtain these values as follows: UserID - Just look at the Users profile, for example mine is http://lussumo.com/community/account/2461/ which tells me my UserID is 2461. IP Address - Put up a display Icon for your User and log every User who visits a given discussion. Remember, Administrators and Moderators are quite often the most active and easiest to antagonize and make post. Generally speaking someone who glimpes in a Discussion will only appear once in the log, while a User who posts will appear twice. Login Time - Yet again the profile is too the rescue, at the time of writing my profile at http://lussumo.com/community/account/2461/ clearly states 'Last Active 52 minutes ago' which is a timestamp accururate to 120 seconds of our derived timestamp. So, for example lets say I monitor when Mark (as he is the Master Administrator) is active, then start a Discussion regarding him to make him respond. I then go through the log for image requests and attempt to identify which IP is Marks. I already know his UserID is 1, so I MD5 hash that, and then check his profile to find the rough time he returned to the site. That gives me a total of 120 possible Verification Keys which are valid till Mark next returns. Even given there are three entries in my log, that is still only a mere 3 * 120 = 360 possibly Verification Keys. If I was to code something up to validate a single Verification Key only once a second, that would still only take me six minutes to find the valid value so I can login to his account without his password. My appologies if that is a little hard to follow, but feel free to read it again. A change to the Verification Key routine as simple as the following would solve the problem and make an extremely difficult key to bruteforce: function DefineVerificationKey() { return md5(sprintf('%04x%04x%04x%03x4%04x%04x%04x%04x', mt_rand(0, 65535), mt_rand(0, 65535), mt_rand(0, 4095), bindec(substr_replace(sprintf('%016b', mt_rand(0, 65535)), '01', 6, 2)), mt_rand(0, 65535), mt_rand(0, 65535), mt_rand(0, 65535)) ); }
«1

Comments

  • Options
    MarkMark Vanilla Staff
    Hmmm. Interesting stuff. I'll give your new verificationkey code a go and see how it does...
  • Options
    edited June 2006
    Its code is untested, but it should be valid. I would say combine that with the plaintext UserID aswell. There are plenty of other ways to create unique values for this kinda thing.
  • Options
    MarkMark Vanilla Staff
    Hold onto your hat - here we go with those changes right here on this installation...
  • Options
    Dagnabbit. I was just about to hack entriples account!
  • Options
    The changes seem to be working nicely to me.
  • Options
    TreyTrey Charlotte NC New
    edited June 2006
    is this something everyone should be doing to their forums then? and would it work for version 0.9.2.6?
  • Options
    MarkMark Vanilla Staff
    Thanks Entriple.

    You seem to know your way around security. Right now Vanilla invalidates cookies if you sign in from another browser or location. How would you recommend changing the current authenticator so that you can stay logged in from many different sources, but still remain secure?
  • Options
    MarkMark Vanilla Staff
    I'd just wait for Vanilla 1, trey. Unless you don't plan on upgrading that is...
  • Options
    TreyTrey Charlotte NC New
    oh I do plan on upgrading and cant wait for the upgrade to come out! I'm just thinking of right now because my forum is starting to get pretty active, and I dont want someone logging in as me before version 1 is released. I'm always checking back here on the site to see the progress of it so I know I wont miss when it does come out.
  • Options
    MarkMark Vanilla Staff
    Well, it's been a while, but I think that the authentication in the older version was different anyway. It just randomly generated a key, so this issue shouldn't apply.
  • Options
    TreyTrey Charlotte NC New
    ok, thanks for the info Mark :)
  • Options
    There are plenty of different ways you could acheive this, however the simplest way without sacrificing too much security would be to only update the Verification Key when the User had a failed login attempt. This would also act as a way to warn a User if his or her account is being bruteforced, but you could always add a Cookie Authentication attempt counter if you prefered. The code given above should return a MD5 hash which is virtually unbreakable within any acceptable period of time so the chances of it being bruteforced through web requests is minimal, and it makes bruteforcing someones password via plaintext requests more viable than attacking the Cookie itself. If someone has their Cookie stolen then they are gone either way regardless.
  • Options
    On a side note, out of curiosity, why dont you cache the UserDiscussionWatch results for the current User in the Users Session just to put a little less strain on your DBMS and leave it only for INSERT/UPDATE?
  • Options
    MarkMark Vanilla Staff
    UserDiscussionWatch is simply joined to in a query. From the tests I've run, It makes almost no change in speed for the discussion query if I remove the join.
  • Options
    Fair enough, maybe MySQL is doing something properly for once afterall.
  • Options
    Um, there are only 7 arguments being passed to sprintf in the new code. The format string being used requires 8 arguments.
  • Options
    Yeah, didn't bother testing. Anyway, I am using the following code for my own purposes which works fine: private function createVerificationKey() { return md5( sprintf('%04x%04x-%04x-%03x4-%04x-%04x%04x%04x', mt_rand(0, 65535), mt_rand(0, 65535), mt_rand(0, 65535), mt_rand(0, 4095), bindec(substr_replace(sprintf('%016b', mt_rand(0, 65535)), '01', 6, 2)), mt_rand(0, 65535), mt_rand(0, 65535), mt_rand(0, 65535) )); }
  • Options
    lechlech Chicagoland
    Holy shit, Entriple, that's some pretty hard logic behind the cracking method and well done at providing a sizeable solution! Kudos for finding a potential and deadly soft-spot. My (potentially stupid) question is, since I notice a lot of random values being generated in the above code, would there be any use of extending a part of this into the admin panel to allow the admins to tack on and introduce a custom salt string of any type? I presume by doing so it could generate a little more variety in the verification key methods.
  • Options
    It would not really be that necessary, but if you did then I would suggest using a slightly more suitable algorithm than md5. Most random number generators use the current time as part of the seed, plus the speed of execution of the processor would affect the subsequent mt_rand() calls aswell giving a considerably massive possible result set. The possible result set is almost definately going to be larger than a wordlist or bruteforce attack on any given users password, and given the Verification Key changes more often than a password especially with a regular user I don't see why any attacker would concentrate on attempting to predict it.
  • Options
    Hello!! I'm a hacker who just hacked into Jazzman's account!! Muhahahah!!






    *Ehm.. just kidding.. I'm actually Jazzman himself -.^
This discussion has been closed.