OpenCart v 2.x [Full version = 188.8.131.52]
[Also applicable on previous versions (2.x series only) and will probably prevail for near future updates in 2.x series]
OpenCart saves password in a 40-bit length field name in (prefix)user and (prefix)customer table, along with a 9-bit salt value, in the adjoining field, which is good.
The login check involves 3 cycles of concatenating the salt with SHA1 hashed password, which is also ok.
However, in the SQL for the login check, there is an alternate option to check for a valid password(with an actual OR statement) by simply checking MD5 hash of the password.
This is a strange behavior, and may have been introduced to add some kind of backward compatibility (which I am not sure is the actual cause, due to my lack of understanding of previous versions of OpenCart), but this *should* not have been done.
What this means is that if someone is having access to your OpenCart database, and the attacker simply updates the password field with the MD5 hash, the password will work for admin user as well, and the salt would not even be part of the equation.
If the OR part of the SQL would have been dropped, then even direct access to database would have prevented any attacker to get hold of admin access by directly changing the password to MD5. The salt would also be needed to be updated, and a (small) layer of security could have been added.
Actual SQL (final) that is generated by OpenCart:
// For Customers
“SELECT * FROM oc_customer WHERE LOWER(email) = ‘” . $this->db->escape(utf8_strtolower($email)) . “‘ AND (password = SHA1(CONCAT(salt, SHA1(CONCAT(salt, SHA1(‘” . $this->db->escape($password) . “‘))))) OR password = ‘” . $this->db->escape(md5($password)) . “‘) AND status = ‘1’ AND approved = ‘1’ ”
// For Users
“SELECT * FROM oc_user WHERE username = ‘” . $this->db->escape($username) . “‘ AND (password = SHA1(CONCAT(salt, SHA1(CONCAT(salt, SHA1(‘” . $this->db->escape($password) . “‘))))) OR password = ‘” . $this->db->escape(md5($password)) . “‘) AND status = ‘1’ ”
The highlighted part is the one that should have been avoided.