Post by maxsmith » Wed Sep 02, 2015 3:42 am

admin/controller/common/login.php:

Code: Select all

			$this->session->data['token'] = md5(mt_rand());
This is your XSRF defense token? Umm...yeah, that's not even close to secure. mt_rand() is guessable because the seed is based on the system clock. All an attacker has to do is craft a very minimal page and they can completely take over any OC-based system that has its system clock correctly in sync (and most do). md5() is just an obfuscation AND you don't have enough data anyway to leverage a cryptographic hash algorithm, which means that, not only is it weak, it is susceptible to additional attacks.

I remember that ~5 years ago that the XSRF thing hit Slashdot because the author of OC thought that XSRF attacks weren't real or something. I'd wager this code was the original response AFTER finally capitulating to the public humiliation you received and that this non-XSRF code has not changed at all in five years. Sigh. Learn software security already. You are opening yourself up to liability suits - the GPL means nothing if a large enough organization comes after you - they can simply create a virtual prison for you by wasting decades of your life while you deal with the legal system and they won't even necessarily have to win, just sufficiently ruin your life.

Start with this: Your XSRF token needs to be unguessable. That means either a precalculated secret using a CSPRNG during install or just using a CSPRNG. mt_rand() is NOT random. It's a PRNG based on Mersenne Twister, which has a larger period than the system rand(). If you need a cryptographic hash, use a HMAC. And XSRF defenses are further strengthened when a unique token is used on a per-controller basis.

Newbie

Posts

Joined
Fri Aug 21, 2015 6:51 am
Who is online

Users browsing this forum: No registered users and 10 guests