Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serious security flaw #37

Closed
hegra opened this issue Aug 7, 2012 · 9 comments
Closed

Serious security flaw #37

hegra opened this issue Aug 7, 2012 · 9 comments

Comments

@hegra
Copy link

hegra commented Aug 7, 2012

Opencart uses md5 and no salt to store passwords for users. This is a serious security flaw!

Without salt, rainbow tables are available that directly gives you the non md5 password. (Just google the md5!)
Also with salts md5 is broken nowadays.

This security flaw must be fixed in open cart else opencart will be targeted everywhere for easy access of password tables.

A direct implementation into opencart of an extension like this works quite well:
http://www.opencart.com/index.php?route=extension/extension/info&extension_id=6843&filter_search=%20compatibility&page=2

If openssl is not installed, for fallback, a random generator something like this works quite well:
for ($j = 0; $j < 22; $j++) {
$salt .= substr("./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789", mt_rand(0, 63), 1);
}

@opencart
Copy link
Collaborator

opencart commented Aug 7, 2012

theres a reason i use md5. its so people can reset there admin passwords without needign to remeber what there salt was.

and hackers actually have to get to the db to get the tables!

@opencart opencart closed this as completed Aug 7, 2012
@hegra
Copy link
Author

hegra commented Aug 7, 2012

If you look at crypt() the salt is stored in the password. You don't need to remember anything. I reseted my own admin password yesterday from phpmyadmin from an md5 string when I made my own SHA-512 implementation. So, this in not a good reason for using md5.

Since it is know that opencart stores that passwords in md5, there are incentives for attacking the db since you are certain you will get a lot of user information out. By having a higher security, hackers don't have any reason for attacking opencart installations. With an argument that you first have to get to the db... why bother with md5 at all? Just store the password in plain text and opencart can offer function that you can get your password mailed to you in case you forget!

The most severe thing though is that everyone that owns an opencart store now probably have full access to a lot of their customers e-mail accounts! I use a crap password today when i bought from an opencart site since I know opencart is not safe.

In addition brute force might also be possible to use to get access to the admin account since the md5 is so extremly fast.

This issue can't be closed, it is highly unresponsible for everyone that shops in an opencart store. This will severly affect the reputation of opencart if it isn't fixed.

@gsdevme
Copy link

gsdevme commented Aug 7, 2012

@opencart your point isn't valid, its not just about ensuring the security of opencart, common users will use the same password for their email account, if you get hold of a database dump of Emails and Unsalted MD5 password you then can do lookups/use rainbow tables get the plain text and use that to login to their email accounts, amazon etc.

I'd like to echo what @hegra states your lack of care of the subject or perhaps understanding is shocking.

Be nice to see a password strengthening system inplace then salt & sha512 hash. Perhaps a sha512 looped 10,000 times then salt then final sha512.

The loop of 10,000 times helps slow the rainbow table lookup the hacker does. So say your hacker has a rainbow table of 10 million passwords and needs to apply it to your 50,000 row OpenCart database it will mean his CPU will have to calculate 5.005×10¹⁴ hashes... also he would need to know your Salt.

Without a Salt then its just a lookup, he can of computed the hashes months in advance of a truly massive dataset. Also without the password strengthening (10000x loop) he could run through his rainbow table much quicker... well 10,000 quicker!

However to 1 user logging into the system its 0.1 - 2 seconds to loop 10,001 times per password (depending on server specs).

@hegra
Copy link
Author

hegra commented Aug 7, 2012

Great that more people understand this issue.

The crypt() function in php is already nicely designed to add salts and everything. It is especially made for just this issue of storing passwords. One salt for every password so rainbowtables need to be made for every user in case of someone would go that way :)

I made my own implementation now in opencart with SHA-512, 50000 rounds and it works like a charm. Around 0.2 seconds just.

@opencart opencart reopened this Aug 7, 2012
@opencart
Copy link
Collaborator

opencart commented Aug 7, 2012

fine i will look into it. i have nearly finished the fedex extension. the encryption will be looked into next then the hash.

@opencart
Copy link
Collaborator

opencart commented Aug 7, 2012

post some code showing how you did the SHA-512 it will make my life a little easier.

@hegra
Copy link
Author

hegra commented Aug 7, 2012

I have a new install so I just added this class:

@gsdevme
Copy link

gsdevme commented Aug 8, 2012

I'm not a big fan of crypt() myself it tends to be behind on certain PHP installs.

Crypt()
5.3.2 Added SHA-256 and SHA-512 crypt based on Ulrich Drepper's » implementation.

hash_algos()
5.3.0 Support for md2, ripemd256, ripemd320, salsa10, salsa20, snefru256 and sha224 was added

So using a custom setup would allow for a lower PHP install base. Im pretty sure hash_algos has had sha512 since PHP 5.2 the changelog doesnt show much

I tend to rewrite something along the lines of this into the applications im writing, however might be wise to have a branch for 2.x since changing anything related to passwords is going to piss people off with current databases.

https://gist.github.com/3294324

Running.
http://codepad.viper-7.com/Km2y7B

@opencart
Copy link
Collaborator

ok fixed. i hope!

i can't see any one cracking whats been added.

savage4pro pushed a commit to savage4pro/opencart that referenced this issue Dec 7, 2015
догоняем оригинал
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants