Post by boopbeepforum » Wed Aug 04, 2021 11:25 pm

I saw a news article that German authorities fined a website 65,500€ for violating GDPR by using an outdated version of xt:commerce that had SQL injection vulnerabilities and the version was warned against continued use by the publisher which did not provide anymore new security updates, and the passwords were stored with md5 hash without salt.

I was wondering if OpenCart has risks of such fines?

Current and older releases of OpenCart seem to store password with sha1 with salt in md5. The Github master uses password_hash.

The source document points to the German cryptographic procedures which says md5 is not compliant and also warns against using sha1.

Also, would using a 1.5x or 2.x version of OpenCart be considered a security risk with risk of fines since they're no longer being updated? But are there any known vulnerabilities with older versions?

Ernie's/IP-CAM's version of 1.5.6.5_rc has some small changes for what some threads say are small SQL injection vulnerabilities. It also has changes to run on PHP 7.x. I wonder if using this version of 1.5.6.5_rc is in violation of GDPR?

Would continuing to use end of life PHP versions, 7.2 and older, also be considered in violation if they don't have anymore security updates?

News Article:
https://www.heise.de/news/DSGVO-Bussgel ... 54208.html

Source document for the fine:
https://lfd.niedersachsen.de/download/169169

Cryptographic Procedures:
https://www.bsi.bund.de/SharedDocs/Down ... 02102.html

Google Translate of the description of the fine from the source document:
Fine for operating a website with outdated software
I took a report according to Article 33 GDPR as an opportunity to check a company's website from a technical point of view. It turned out that the web shop application xt: Commerce in version 3.0.4 SP2.1 was used on the site. This version has been out of date since 2014 at the latest and is no longer provided with security updates by the manufacturer. The software used contained considerable security gaps, which the manufacturer had pointed out. Among other things, the security holes made SQL injection attacks possible. The manufacturer also warned against continuing to use version 3 of the software.
With SQL injection attacks, attackers can gain access to the access data of everyone registered in the application. This attack vector arises when not all entries that can be changed by the end user are masked in such a way that the database cannot understand them as commands. Without masking, the database executes any command with its own rights. In this way, entire database tables can be output or deleted. Shutting down the server may also be possible.

My investigations showed that the passwords stored in the database were secured with the cryptographic hash function "MD5", which, however, is not designed for use for passwords. A quick 'calculation' of the plaintext passwords would therefore have been possible. There are also so-called “rainbow tables” on the Internet, which can be used to read the password associated with a hash without any calculation.
In addition, no “salt” was used. Such a salt, which is generated individually for each password, extends a password and thus makes the systematic calculation much more difficult. The aim of the Salt is that the attacker has to perform a complete recalculation for each password and prefabricated rainbow tables become worthless. Without Salt, on the other hand, a common calculation for the entire downloaded database was sufficient.
Without appropriate security precautions, in the present case it would have been possible to determine the plaintext passwords and then try out further attack vectors with manageable effort. An attacker would have the passwords determined z. B. can test with the e-mail addresses also stored in the database and, if successful, cause considerable (consequential) damage.
The implementation of a salt function and a current hash algorithm designed for passwords would not have been associated with disproportionate effort for the company, especially if this functionality is implemented with newer versions of the software. This also applies to the elimination of known security vulnerabilities for which updates are available. Regularly updating the software is sufficient to close known security gaps and other weak points. This can be associated with procurement and implementation costs, which, however, generally do not represent a disproportionate effort.
The technical measures taken by the person responsible were therefore not appropriate to the protection requirements according to Art. 25 GDPR, so that I discovered a violation of Art. 32 Paragraph 1 GDPR. I imposed a fine of 65,500 euros that the company accepted.
When determining the fine, I was able to take into account that the company had already informed the persons concerned that a change of password was necessary before the fine procedure.

Securing passwords
The requirements for passwords are identical from the point of view of data protection and information security. The Federal Office for Information Security (BSI) provides constantly updated recommendations on its website and in the technical guideline "Cryptographic Procedures: Recommendations and Key Lengths" (BSI TR-02102-1). Since the recommendations can change due to more recent findings, this article only offers a snapshot.

Instead of a very efficient calculation function such as MD5, a cryptographic hash function specially developed for passwords should be used. In order to determine passwords through systematic trial and error (brute force method), significantly more computational effort is required for functions2 specially designed for passwords. Furthermore, an individual salt should be created for each password. This means that so much computing power has to be invested that it can become uninteresting to determine the passwords for the entire database.
However, there may be individual access points that are of particular interest to attackers. Above all, this can be administration access and access for prominent people. The weak point therefore remains the password chosen by the user. The rule here is that the password strength is significantly more influenced by its length than by its complexity (uppercase letters, lowercase letters, digits, special characters). However, longer passwords can also be determined with reasonable effort if they can be found in password lists. Users should therefore not use proverbs or quotations as passwords.
There are no hard and fast rules for the length and complexity of passwords. When a password is sufficiently strong also depends on the environment and the typical attack vectors. If, for example, access is blocked for a longer period of time after three to five incorrect entries or an additional medium is retained after unsuccessful attempts (e.g. EC card), a shorter and less complex password is sufficient.
In the case of high risks or sensitive data, multi-factor authentication should also be used. In addition to user ID and personal password, one-time passwords could be sent or security tokens could be used.

Further fine proceedings related to technical and organizational measures
Violations of Articles 25 and 32 GDPR also play a role in other proceedings, albeit often subordinate. In the absence of a legal basis, video recordings can violate Article 6 GDPR or Section 26 BDSG, and there can also be a violation of Article 25 GDPR if the areas in which people are moving or staying have not been pixelated. Contrary to Art. 25 Paragraph 2 GDPR, in such cases the person responsible has not taken any suitable technical measures by presetting that only personal data are processed, the processing of which is necessary for the respective specific processing purpose. In such a case, the violation of Article 6 GDPR or Section 26 BDSG has priority.

Newbie

Posts

Joined
Tue May 18, 2021 9:40 pm

Post by by mona » Wed Aug 04, 2021 11:38 pm

Whilst you may get opinions - we are developers not lawyers.
This is such a serious issue that constantly changes - commenting with the appearance of knowledge which will soon be out of date for any future searches will do more harm than good.

Speak to a lawyer.

DISCLAIMER:
You should not modify core files .. if you would like to donate a cup of coffee I will write it in a modification for you.


https://www.youtube.com/watch?v=zXIxDoCRc84


User avatar
Expert Member

Posts

Joined
Mon Jun 10, 2019 9:31 am

Post by ADD Creative » Thu Aug 05, 2021 7:25 am

There are known vulnerabilities with all versions of OpenCart. As far as I've aware only 2.0.1.0 had one of the same magnitude as in the document you posted.

There will be more minor issues in older versions. However, it could be argued that the later version have equally as many problems when it comes to securing data. The lack of cache control headers in 3.x springs to mind. I think the biggest risk comes from themes and extensions. There are so many poorly written ones out there. You don't see many reports of OpenCart sites being hacked, and when you do it's usually an issue with the theme or an extension.

If you are using an old PHP version, make sure it's one that has had the security fixes backported.
viewtopic.php?f=202&t=224811#p825259

What constitutes knowledge of a problem and proportionate effort is one for legal experts.

www.add-creative.co.uk


Expert Member

Posts

Joined
Sat Jan 14, 2012 1:02 am
Location - United Kingdom

Post by OSWorX » Thu Aug 05, 2021 2:28 pm

ADD Creative wrote:
Thu Aug 05, 2021 7:25 am
There are known vulnerabilities with all versions of OpenCart.
ADD Creative wrote:
Thu Aug 05, 2021 7:25 am
I think the biggest risk comes from themes and extensions. There are so many poorly written ones out there. You don't see many reports of OpenCart sites being hacked, and when you do it's usually an issue with the theme or an extension.
It should be clearly stated: OpenCart itself has no known vulnerabilities.

As you are already saying: when, themes and extensions are the bad ones.
And one more: if OpenCart AND Wordpress is operated on the same webspace, WP is mostly the one which open doors!

Full Stack Web Developer :: Dedicated OpenCart Development & Support DACH Region
Contact for Custom Work / Fast Support.


User avatar
Guru Member
Online

Posts

Joined
Mon Jan 11, 2010 10:52 pm
Location - Austria

Post by IP_CAM » Thu Aug 05, 2021 5:55 pm

---It also has changes to run on PHP 7.x. I wonder if using this version of 1.5.6.5_rc is in violation of GDPR?---
Just to have it mentioned, my 1.5.6.5_rc-based OC Releases so far all work with PHP v.7.4.21. And my Test Sites are targeted by Hackers on a daily bases, without producing any failures. I so far blocked 4'400 IP-Ranges in my HTACCESS file, to keep out, what seems to belong to such hacking attempts, just to make sure.
Ernie

User avatar
Legendary Member

Posts

Joined
Tue Mar 04, 2014 1:37 am
Location - Switzerland

Post by ADD Creative » Thu Aug 05, 2021 8:55 pm

OSWorX wrote:
Thu Aug 05, 2021 2:28 pm
It should be clearly stated: OpenCart itself has no known vulnerabilities.
That is just not true. There are known vulnerabilities in all versions. While only the SQL injection vulnerability in 2.0.1.0 would be as easy to exploit as the one the original poster referrers to. There are others that could be exploited if catching a administrator, affiliate or customer off guard.

www.add-creative.co.uk


Expert Member

Posts

Joined
Sat Jan 14, 2012 1:02 am
Location - United Kingdom

Post by boopbeepforum » Thu Aug 05, 2021 10:20 pm

IP_CAM wrote:
Thu Aug 05, 2021 5:55 pm
Just to have it mentioned, my 1.5.6.5_rc-based OC Releases so far all work with PHP v.7.4.21. And my Test Sites are targeted by Hackers on a daily bases, without producing any failures. I so far blocked 4'400 IP-Ranges in my HTACCESS file, to keep out, what seems to belong to such hacking attempts, just to make sure.
Ernie
I didn't want openbay or recurring profiles, so I took 1.5.5.x and copied over the other changes from Ernie's 1.5.6.5_rc, which I think added a few fixes for (int) pages, library/cart, and checking for php in upload files. Then I updated mysqli, encryption, and utf8, partly from 1.5.6.5_rc and partly from 3.0.3.7, so that it works with php 7.4. I also added (int) for zone id and country id.

From browsing the threads, I got the impression that the other known vulnerabilities required customers and users to be logged in already, so the data would already be compromised. The CSRF mod I found on some threads seemed to be updated to work for 3.x, and the version for 1.5x deleted.

Were there any other vulnerabilities for leaking personal data to be fix for 1.5.6.5_rc?

Looking at the password_hash in the github master branch, it seems possible to swap out with the sha1 in 1.5.6.5_rc, and it would replace the old sha1 hash on the next login.

I also copied over the getLoginAttempts from 3.x, but it can be bypassed by changing subsequent attempts from a different IP address. I fixed it by totaling all attempts from different IPs for the same email.
https://github.com/opencart/opencart/issues/10084

Newbie

Posts

Joined
Tue May 18, 2021 9:40 pm

Post by OSWorX » Thu Aug 05, 2021 10:26 pm

ADD Creative wrote:
Thu Aug 05, 2021 8:55 pm
OSWorX wrote:
Thu Aug 05, 2021 2:28 pm
It should be clearly stated: OpenCart itself has no known vulnerabilities.
That is just not true. There are known vulnerabilities in all versions.
Sorry, but all versions is clearly much to much.
For example: some here may remember the times when OpenCart 1.4.x was published.
And yes, also this version had vulnerabilities (a 3PD library).

And if we speak about vulnerabilities in the OpenCart core, most of them require admin access and/or special permissions.
Also the improper usage of chmod with 0777 on all files and folders (as advised by some users here!).
Beside the fact, that many "users" are installing extensions from obtruse sources - only to avoid to buy them regular!
And installing also 3PD tools (like adminer - search the forum here).

We could write here novels what could be going wrong, but in most cases the "eval" is: the user in front of the screen.

Full Stack Web Developer :: Dedicated OpenCart Development & Support DACH Region
Contact for Custom Work / Fast Support.


User avatar
Guru Member
Online

Posts

Joined
Mon Jan 11, 2010 10:52 pm
Location - Austria

Post by by mona » Thu Aug 05, 2021 10:49 pm

Thank you OSWorX - that is EXACTLY THE POINT - don’t pass the buck.

27,483 stalking cases were reported in the UK in 2020. Correction 3,073 were charged, there no figure how many were actually prosecuted, not enough.
0_NTI_DRONFIELD_ATTACK_16.jpg

0_NTI_DRONFIELD_ATTACK_16.jpg (83.8 KiB) Viewed 7382 times

Don’t put sh*t on the system that you don’t need and remove it when it is no longer required. That is one of the most critical part of GDPR and one where the responsibility is with the data controller. No data on a vulnerable system is safer than Fort Knox. The consequences of failing to protect an individuals identity can be devastating. It is unfortunate that you see the consequences to you and not to those whose data you may expose.

When people start to understand what it actually is they are supposed to be protecting maybe they will take it seriously, it is very very serious. It is not about the system it is about the concept.

Yes this woman was stalked by her ex - and he tracked her down and did this - some women are not so lucky. You have a duty to provide a SAR and you have a duty to ensure you are not giving to the wrong person - you know what - don’t have the information and you won’t have a problem.

DISCLAIMER:
You should not modify core files .. if you would like to donate a cup of coffee I will write it in a modification for you.


https://www.youtube.com/watch?v=zXIxDoCRc84


User avatar
Expert Member

Posts

Joined
Mon Jun 10, 2019 9:31 am

Post by IP_CAM » Fri Aug 06, 2021 1:09 am

Well, one main Problem of Opencart is, that OC Newcomers always expect 'Latest' to also be 'best'. And just about noone of the 'Dev's is really interested, to tell the Folks, that 'latest' Versions most always contain widely 'untested' new Code as well, and so partly no longer able, to be compatible to all Extensions/Themes, available to former (Sub-)Versions. And as a result of that, some Extensions/Themes may not work as planned, and new Users don't understand, that 'free' Help is rarely available, when it comes to Problems, resulting out of that.

I kept, what I had, beeing the best out of 1.5.6.(5_rc), by only replacing on Files+Code, what has been mentioned/shown here at the Forum, partly including 'things', originally created for later OC (v.2.x) Versions. Like all those Files and/or Code, known to potentially be vulnerable, when it comes to attacks. And despite of uncounted amounts of attempts, not one of them was ever able yet, to kill/harm one of my Demo Sites or their Database.

More 'neutral' Information would likely help, to at least keep some from screwing up, or then move on to something else, known at least to work as told. Every 'lost' User was also a potential Buyer, like in any other Business too ... 8)
Good Luck!
Ernie

User avatar
Legendary Member

Posts

Joined
Tue Mar 04, 2014 1:37 am
Location - Switzerland

Post by boopbeepforum » Sun Aug 08, 2021 12:38 am

IP_CAM wrote:
Fri Aug 06, 2021 1:09 am
I kept, what I had, beeing the best out of 1.5.6.(5_rc), by only replacing on Files+Code, what has been mentioned/shown here at the Forum, partly including 'things', originally created for later OC (v.2.x) Versions.
I made a VQmod for switching the SHA1 hashing to password_hash, but I have not tested it yet. If you like to try, here it is. Please let me know if you find out anything.

It's based on the github master, with the original pull:
https://github.com/opencart/opencart/pull/5798/files

I think it might work for 1.5.5.x - 15.6.5_rc.
But depending on whether you have changed your core code, you may need to add or remove html_entity_decode in some places.

In the database, password for customer, affiliate, and user needs to be increased from 40 to 255 characters in case future default hash algorithms increase the hash length. And PASSWORD_DEFAULT bcrypt truncates the input password to 72 characters maximum length.
https://www.php.net/manual/en/function. ... d-hash.php

Questions I had with I assume would likely not be possible:
Is there any risk of breaking password_verify to return a true instead of false when the hashes don't match, or breaking the if statement and continuing on to publish the customer or user data?
Or breaking password_verify and password_needs_rehash to rehash another password?


Code: Select all

<?xml version="1.0" encoding="UTF-8"?>
<modification>
	<id>password_hash</id>
	<version>1.0</version>
	<vqmver>2.X</vqmver>
	<author></author>
	<opencart>1.5.6.5_rc</opencart>
	
	
	<file name="system/library/customer.php">  
		
		<operation info="changes to password_hash customer login">
			<search position="replace" offset="6"><![CDATA[if ($override) {]]></search>
			<add><![CDATA[
		// password_hash
	
		$customer_query = $this->db->query("SELECT * FROM " . DB_PREFIX . "customer where LOWER(email) = '" . $this->db->escape(utf8_strtolower($email)) . "' AND status = '1'");
		
		if ($customer_query->row) {
			if (!$override) {
				if (password_verify(html_entity_decode($password, ENT_QUOTES, 'UTF-8'), $customer_query->row['password'])) {
					//$rehash = password_needs_rehash($customer_query->row['password'], PASSWORD_DEFAULT);
					//changed to !empty in case of undefined error
					$rehash = !empty(password_needs_rehash($customer_query->row['password'], PASSWORD_DEFAULT)) ? true : false;
					//changed !isset to empty in case salt has no string stored in database
				} elseif (!empty($customer_query->row['salt']) && $customer_query->row['password'] == sha1($customer_query->row['salt'] . sha1($customer_query->row['salt'] . sha1($password)))) {
					$rehash = true;
				} elseif ($customer_query->row['password'] == md5($password)) {
					$rehash = true;
				} else {
					return false;
				}

				if ($rehash) {
					$this->db->query("UPDATE " . DB_PREFIX . "customer SET salt = '', password = '" . $this->db->escape(password_hash(html_entity_decode($password, ENT_QUOTES, 'UTF-8'), PASSWORD_DEFAULT)) . "' WHERE LOWER(email) = '" . $this->db->escape(utf8_strtolower($email)) . "'");
				}
			}
	
		// password_hash
			]]></add>
		</operation>
		
	</file>
	
	
	
	<file name="system/library/affiliate.php">  
		
		<operation info="changes to password_hash affiliate login">
			<search position="replace" offset="2"><![CDATA[$affiliate_query = $this->db->query("SELECT * FROM " . DB_PREFIX . "affiliate 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'");]]></search>
			<add><![CDATA[
		// password_hash
		
		$affiliate_query = $this->db->query("SELECT * FROM " . DB_PREFIX . "affiliate WHERE LOWER(email) = '" . $this->db->escape(utf8_strtolower($email)) . "' AND status = '1' AND approved = '1'");
		
		if ($affiliate_query->row) {
			if (password_verify(html_entity_decode($password, ENT_QUOTES, 'UTF-8'), $affiliate_query->row['password'])) {
				//$rehash = password_needs_rehash($affiliate_query->row['password'], PASSWORD_DEFAULT);
				//changed to !empty in case of undefined error
				$rehash = !empty(password_needs_rehash($affiliate_query->row['password'], PASSWORD_DEFAULT)) ? true : false;
				//changed isset to !empty in case salt has no string stored in database
			} elseif (!empty($affiliate_query->row['salt']) && $affiliate_query->row['password'] == sha1($affiliate_query->row['salt'] . sha1($affiliate_query->row['salt'] . sha1($password)))) {
				$rehash = true;
			} elseif ($affiliate_query->row['password'] == md5($password)) {
				$rehash = true;
			} else {
				return false;
			}

			if ($rehash) {
				$this->db->query("UPDATE " . DB_PREFIX . "affiliate SET salt = '', password = '" . $this->db->escape(password_hash(html_entity_decode($password, ENT_QUOTES, 'UTF-8'), PASSWORD_DEFAULT)) . "' WHERE LOWER(email) = '" . $this->db->escape(utf8_strtolower($email)) . "'");
			}
	
		// password_hash
			]]></add>
		</operation>
		
	</file>
	
	
	
	<file name="system/library/user.php">  
		
		<operation info="changes to password_hash user login">
			<search position="replace" offset="2"><![CDATA[$user_query = $this->db->query("SELECT * FROM " . DB_PREFIX . "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'");]]></search>
			<add><![CDATA[
		// password_hash
		
		$user_query = $this->db->query("SELECT * FROM " . DB_PREFIX . "user WHERE username = '" . $this->db->escape($username) . "' AND status = '1'");
		
		if ($user_query->row) {
			if (password_verify(html_entity_decode($password, ENT_QUOTES, 'UTF-8'), $user_query->row['password'])) {
				//$rehash = password_needs_rehash($user_query->row['password'], PASSWORD_DEFAULT);
				//changed to !empty in case of undefined error
				$rehash = !empty(password_needs_rehash($user_query->row['password'], PASSWORD_DEFAULT)) ? true : false;
				//changed isset to !empty in case salt has no string stored in database
			} elseif (!empty($user_query->row['salt']) && $user_query->row['password'] == sha1($user_query->row['salt'] . sha1($user_query->row['salt'] . sha1($password)))) {
				$rehash = true;
			} elseif ($user_query->row['password'] == md5($password)) {
				$rehash = true;
			} else {
				return false;
			}

			if ($rehash) {
				$this->db->query("UPDATE " . DB_PREFIX . "user SET salt = '', password = '" . $this->db->escape(password_hash(html_entity_decode($password, ENT_QUOTES, 'UTF-8'), PASSWORD_DEFAULT)) . "' WHERE username = '" . $this->db->escape($username) . "'");
			}
	
		// password_hash
			]]></add>
		</operation>
		
	</file>
	
	
	
	<file name="catalog/model/account/customer.php,catalog/model/affiliate/affiliate.php,admin/model/user/user.php">  
		
		<operation info="changes to password_hash for add">
			<search position="replace"><![CDATA[salt = '" . $this->db->escape($salt = substr(md5(uniqid(rand(), true)), 0, 9)) . "', password = '" . $this->db->escape(sha1($salt . sha1($salt . sha1($data['password']))))]]></search>
			<add><![CDATA[salt = '', password = '" . $this->db->escape(password_hash(html_entity_decode($data['password'], ENT_QUOTES, 'UTF-8'), PASSWORD_DEFAULT))]]></add>
		</operation>
		
		
		<operation info="changes to password_hash for edit password">
			<search position="replace"><![CDATA[salt = '" . $this->db->escape($salt = substr(md5(uniqid(rand(), true)), 0, 9)) . "', password = '" . $this->db->escape(sha1($salt . sha1($salt . sha1($password))))]]></search>
			<add><![CDATA[salt = '', password = '" . $this->db->escape(password_hash(html_entity_decode($password, ENT_QUOTES, 'UTF-8'), PASSWORD_DEFAULT))]]></add>
		</operation>
		
	</file>
	
	
	
	<file path="admin/model/sale/" name="customer.php,affiliate.php">  
		
		<operation info="changes to password_hash for add and edit">
			<search position="replace"><![CDATA[salt = '" . $this->db->escape($salt = substr(md5(uniqid(rand(), true)), 0, 9)) . "', password = '" . $this->db->escape(sha1($salt . sha1($salt . sha1($data['password']))))]]></search>
			<add><![CDATA[salt = '', password = '" . $this->db->escape(password_hash(html_entity_decode($data['password'], ENT_QUOTES, 'UTF-8'), PASSWORD_DEFAULT))]]></add>
		</operation>
		
	</file>
	
	
    
</modification>
Last edited by boopbeepforum on Mon Aug 09, 2021 6:39 am, edited 2 times in total.

Newbie

Posts

Joined
Tue May 18, 2021 9:40 pm

Post by boopbeepforum » Sun Aug 08, 2021 1:08 am

And then here is the VQmod for adding a failed login attempt lock for 1.5.6.5_rc. I tested it and it works ok for me.
It's based on 3.0.3.7, but also fixes the problem with bypass bug where you can get around the lock by changing to a different IP address.
But there may be an easier fix for this:
https://github.com/opencart/opencart/is ... -894666334

It also adds the check to admin user login, based on this thread:
viewtopic.php?t=218405#p790296

But this table need to be added first to the database for customer:

Code: Select all

CREATE TABLE IF NOT EXISTS `oc_customer_login` (
  `customer_login_id` int(11) NOT NULL AUTO_INCREMENT,
  `email` varchar(96) NOT NULL,
  `ip` varchar(40) NOT NULL,
  `total` int(4) NOT NULL,
  `date_added` datetime NOT NULL,
  `date_modified` datetime NOT NULL,
  PRIMARY KEY (`customer_login_id`)
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8;
For user:

Code: Select all

CREATE TABLE IF NOT EXISTS `oc_user_login` (
  `user_login_id` int(11) NOT NULL AUTO_INCREMENT,
  `username` varchar(96) NOT NULL,
  `ip` varchar(40) NOT NULL,
  `total` int(4) NOT NULL,
  `date_added` datetime NOT NULL,
  `date_modified` datetime NOT NULL,
  PRIMARY KEY (`user_login_id`)
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8;

Code: Select all

<?xml version="1.0" encoding="UTF-8"?>
<modification>
	<id>login attempst</id>
	<version>1.0</version>
	<vqmver>2.X</vqmver>
	<author></author>
	<opencart>1.5.6.5_rc</opencart>
	
	
	<file name="catalog/model/account/customer.php">  
		
		<operation info="add functions to customer model">
			<search position="before" ><![CDATA[public function isBanIp($ip) {]]></search>
			<add><![CDATA[
	// login attempt
	
	public function addLoginAttempt($email) {
		$query = $this->db->query("SELECT * FROM " . DB_PREFIX . "customer_login WHERE email = '" . $this->db->escape(utf8_strtolower((string)$email)) . "' AND ip = '" . $this->db->escape($this->request->server['REMOTE_ADDR']) . "'");

		if (!$query->num_rows) {
			$this->db->query("INSERT INTO " . DB_PREFIX . "customer_login SET email = '" . $this->db->escape(utf8_strtolower((string)$email)) . "', ip = '" . $this->db->escape($this->request->server['REMOTE_ADDR']) . "', total = 1, date_added = '" . $this->db->escape(date('Y-m-d H:i:s')) . "', date_modified = '" . $this->db->escape(date('Y-m-d H:i:s')) . "'");
		} else {
			$this->db->query("UPDATE " . DB_PREFIX . "customer_login SET total = (total + 1), date_modified = '" . $this->db->escape(date('Y-m-d H:i:s')) . "' WHERE customer_login_id = '" . (int)$query->row['customer_login_id'] . "'");
		}
	}

	public function getLoginAttempts($email) {
		$query = $this->db->query("SELECT * FROM `" . DB_PREFIX . "customer_login` WHERE email = '" . $this->db->escape(utf8_strtolower($email)) . "'");

		return $query->rows;
	}

	public function deleteLoginAttempts($email) {
		$this->db->query("DELETE FROM `" . DB_PREFIX . "customer_login` WHERE email = '" . $this->db->escape(utf8_strtolower($email)) . "'");
	}
	
	// login attempt
			]]></add>
		</operation>
		
	</file>
	
	
	
	<file name="catalog/controller/account/login.php">  
		
		<operation info="add login attempt check">
			<search position="replace" offset="8" ><![CDATA[if (!$this->customer->login($this->request->post['email'], $this->request->post['password'])) {]]></search>
			<add><![CDATA[
		// login attempt
		
		// Check how many login attempts have been made.
		$login_info = $this->model_account_customer->getLoginAttempts($this->request->post['email']);
		
		//I set this to 5 failed attempts instead of $this->config->get('config_login_attempts')
		$attempt_total = 0;
		foreach ($login_info as $avalue) {
			$attempt_total += $avalue['total'];
		}
		unset($avalue);
		
		if ($attempt_total >= 5) {
			foreach ($login_info as $bvalue) {
				if (strtotime('-1 hour') < strtotime($bvalue['date_modified'])) {
					//I set this text manually instead of $this->language->get('error_attempts')
					$this->error['warning'] = 'You have exceeded the allowed number of login attempts. Please try again after 1 hour.';
					
					$this->model_account_customer->addLoginAttempt($this->request->post['email']);
					break;
				}
			}
			unset($bvalue);
		}
		
		// Check if customer has been approved.
		$customer_info = $this->model_account_customer->getCustomerByEmail($this->request->post['email']);

		if ($customer_info && (!$customer_info['approved'] || !$customer_info['status'])) {
			$this->error['warning'] = $this->language->get('error_approved');
		}

		if (!$this->error) {
			if (!$this->customer->login($this->request->post['email'], $this->request->post['password'])) {
				$this->error['warning'] = $this->language->get('error_login');

				$this->model_account_customer->addLoginAttempt($this->request->post['email']);
			} else {
				$this->model_account_customer->deleteLoginAttempts($this->request->post['email']);
			}
		}
		
		// login attempt
			]]></add>
		</operation>
		
	</file>
	
	
	
	<file name="catalog/controller/checkout/login.php">  
		
		<operation info="add login attempt check">
			<search position="replace" offset="10" ><![CDATA[if (!$this->customer->login($this->request->post['email'], $this->request->post['password'])) {]]></search>
			<add><![CDATA[
			// login attempt
			
			$this->load->model('account/customer');
			
			// Check how many login attempts have been made.
			$login_info = $this->model_account_customer->getLoginAttempts($this->request->post['email']);
			
			//I set this to 5 failed attempts instead of $this->config->get('config_login_attempts')
			$attempt_total = 0;
			foreach ($login_info as $avalue) {
				$attempt_total += $avalue['total'];
			}
			unset($avalue);
			
			if ($attempt_total >= 5) {
				foreach ($login_info as $bvalue) {
					if (strtotime('-1 hour') < strtotime($bvalue['date_modified'])) {
						//I set this text manually instead of $this->language->get('error_attempts')
						$json['error']['warning'] = 'You have exceeded the allowed number of login attempts. Please try again after 1 hour.';
						
						$this->model_account_customer->addLoginAttempt($this->request->post['email']);
						break;
					}
					
				}
				unset($bvalue);
			}
			
			// Check if customer has been approved.
			$customer_info = $this->model_account_customer->getCustomerByEmail($this->request->post['email']);

			if ($customer_info && (!$customer_info['approved'] || !$customer_info['status'])) {
				$json['error']['warning'] = $this->language->get('error_approved');
			}

			if (!isset($json['error'])) {
				if (!$this->customer->login($this->request->post['email'], $this->request->post['password'])) {
					$json['error']['warning'] = $this->language->get('error_login');

					$this->model_account_customer->addLoginAttempt($this->request->post['email']);
				} else {
					$this->model_account_customer->deleteLoginAttempts($this->request->post['email']);
				}
			}
			
			// login attempt
			]]></add>
		</operation>
		
	</file>
	
	
	
	
	<file name="admin/model/user/user.php">  
		
		<operation info="add functions to user model ADMIN">
			<search position="before" ><![CDATA[public function getTotalUsersByEmail($email) {]]></search>
			<add><![CDATA[
	// login attempt
	
	public function addLoginAttempt($username) {
		$query = $this->db->query("SELECT * FROM " . DB_PREFIX . "user_login WHERE username = '" . $this->db->escape(utf8_strtolower((string)$username)) . "' AND ip = '" . $this->db->escape($this->request->server['REMOTE_ADDR']) . "'");

		if (!$query->num_rows) {
			$this->db->query("INSERT INTO " . DB_PREFIX . "user_login SET username = '" . $this->db->escape(utf8_strtolower((string)$username)) . "', ip = '" . $this->db->escape($this->request->server['REMOTE_ADDR']) . "', total = 1, date_added = '" . $this->db->escape(date('Y-m-d H:i:s')) . "', date_modified = '" . $this->db->escape(date('Y-m-d H:i:s')) . "'");
		} else {
			$this->db->query("UPDATE " . DB_PREFIX . "user_login SET total = (total + 1), date_modified = '" . $this->db->escape(date('Y-m-d H:i:s')) . "' WHERE user_login_id = '" . (int)$query->row['user_login_id'] . "'");
		}
	}

	public function getLoginAttempts($username) {
		$query = $this->db->query("SELECT * FROM `" . DB_PREFIX . "user_login` WHERE username = '" . $this->db->escape(utf8_strtolower($username)) . "'");

		return $query->rows;
	}

	public function deleteLoginAttempts($username) {
		$this->db->query("DELETE FROM `" . DB_PREFIX . "user_login` WHERE username = '" . $this->db->escape(utf8_strtolower($username)) . "'");
	}
	
	// login attempt
			]]></add>
		</operation>
		
	</file>
	
	
	<file name="admin/controller/common/login.php">  
		
		<operation info="add login attempt check ADMIN">
			<search position="replace" offset="2" ><![CDATA[if (!isset($this->request->post['username']) || !isset($this->request->post['password']) || !$this->user->login($this->request->post['username'], $this->request->post['password'])) {]]></search>
			<add><![CDATA[
		// login attempt
		
		// Check how many login attempts have been made.
		$login_info = $this->model_user_user->getLoginAttempts($this->request->post['username']);
		
		$attempt_total = 0;
		foreach ($login_info as $avalue) {
			$attempt_total += $avalue['total'];
		}
		unset($avalue);
		
		if ($attempt_total >= 5) {
			foreach ($login_info as $bvalue) {
				if (strtotime('-1 hour') < strtotime($bvalue['date_modified'])) {
					$this->error['warning'] = 'You have exceeded the allowed number of login attempts. Please try again after 1 hour.';
					
					$this->model_user_user->addLoginAttempt($this->request->post['username']);
					break;
				}
			}
			unset($bvalue);
		}
		
		if (!$this->error) {
			if (!isset($this->request->post['username']) || !isset($this->request->post['password']) || !$this->user->login($this->request->post['username'], html_entity_decode($this->request->post['password'], ENT_QUOTES, 'UTF-8'))) {
				$this->error['warning'] = $this->language->get('error_login');
				$this->model_user_user->addLoginAttempt($this->request->post['username']);
			} else {
				$this->model_user_user->deleteLoginAttempts($this->request->post['username']);
			}
		}
		
		// login attempt
			]]></add>
		</operation>
		
		
		<operation info="load model ADMIN">
			<search position="after"><![CDATA[is->document->setTitle($this->language->get('heading_title'));]]></search>
			<add><![CDATA[
		// login attempt
		
		$this->load->model('user/user');
		
		// login attempt
			]]></add>
		</operation>
		
	</file>
	
    
</modification>
For older than 1.5.6.5_rc, the admin/common/login.php may have a different line. Instead of this search term:

Code: Select all

if (!isset($this->request->post['username']) || !isset($this->request->post['password']) || !$this->user->login($this->request->post['username'], $this->request->post['password'])) {
It may need to search for this term:

Code: Select all

if (isset($this->request->post['username']) && isset($this->request->post['password']) && !$this->user->login($this->request->post['username'], $this->request->post['password'])) {

Newbie

Posts

Joined
Tue May 18, 2021 9:40 pm

Post by OSWorX » Sun Aug 08, 2021 3:50 pm

I would please the users not to hijack this thread for other messages like the subject is!
Instead open a new thread for that, otherwise this here will be closed for further comments.

Full Stack Web Developer :: Dedicated OpenCart Development & Support DACH Region
Contact for Custom Work / Fast Support.


User avatar
Guru Member
Online

Posts

Joined
Mon Jan 11, 2010 10:52 pm
Location - Austria

Post by boopbeepforum » Sun Aug 08, 2021 8:16 pm

I'm not sure if you're referring to my posts, but I think if the use of md5 hash for passwords violates GDPR and can result in a fine, and sha1 may potentially also be non compliant, although most opencart versions use sha1 with salt, then versions of opencart older than the master branch may need to be updated to use a different algorithm, like what the master branch does.

The master branch uses the default mode of password_hash which is currently bcrypt. One of the mods I posted above would implement what the master branch does for versions 1.5.6.5 but I haven't tested it yet to see if it works. The other mod I posted is because of the login bug found in 3.x while going through the login process.

Newbie

Posts

Joined
Tue May 18, 2021 9:40 pm

Post by by mona » Sun Aug 08, 2021 10:27 pm

boopbeepforum wrote:
Wed Aug 04, 2021 11:25 pm

Fine for operating a website with outdated software
I took a report according to Article 33 GDPR as an opportunity to check a company's website from a technical point of view. It turned out that the web shop application xt: Commerce in version 3.0.4 SP2.1 was used on the site. This version has been out of date since 2014 at the latest and is no longer provided with security updates by the manufacturer. The software used contained considerable security gaps, which the manufacturer had pointed out. Among other things, the security holes made SQL injection attacks possible. The manufacturer also warned against continuing to use version 3 of the software.
With SQL injection attacks, attackers can gain access to the access data of everyone registered in the application. This attack vector arises when not all entries that can be changed by the end user are masked in such a way that the database cannot understand them as commands. Without masking, the database executes any command with its own rights. In this way, entire database tables can be output or deleted. Shutting down the server may also be possible.

My investigations showed that the passwords stored in the database were secured with the cryptographic hash function "MD5", which, however, is not designed for use for passwords. A quick 'calculation' of the plaintext passwords would therefore have been possible. There are also so-called “rainbow tables” on the Internet, which can be used to read the password associated with a hash without any calculation.
In addition, no “salt” was used. Such a salt, which is generated individually for each password, extends a password and thus makes the systematic calculation much more difficult. The aim of the Salt is that the attacker has to perform a complete recalculation for each password and prefabricated rainbow tables become worthless. Without Salt, on the other hand, a common calculation for the entire downloaded database was sufficient.
Without appropriate security precautions, in the present case it would have been possible to determine the plaintext passwords and then try out further attack vectors with manageable effort. An attacker would have the passwords determined z. B. can test with the e-mail addresses also stored in the database and, if successful, cause considerable (consequential) damage.
The implementation of a salt function and a current hash algorithm designed for passwords would not have been associated with disproportionate effort for the company, especially if this functionality is implemented with newer versions of the software. This also applies to the elimination of known security vulnerabilities for which updates are available. Regularly updating the software is sufficient to close known security gaps and other weak points. This can be associated with procurement and implementation costs, which, however, generally do not represent a disproportionate effort.
The technical measures taken by the person responsible were therefore not appropriate to the protection requirements according to Art. 25 GDPR, so that I discovered a violation of Art. 32 Paragraph 1 GDPR. I imposed a fine of 65,500 euros that the company accepted.
When determining the fine, I was able to take into account that the company had already informed the persons concerned that a change of password was necessary before the fine procedure.

Securing passwords
The requirements for passwords are identical from the point of view of data protection and information security. The Federal Office for Information Security (BSI) provides constantly updated recommendations on its website and in the technical guideline "Cryptographic Procedures: Recommendations and Key Lengths" (BSI TR-02102-1). Since the recommendations can change due to more recent findings, this article only offers a snapshot.

Instead of a very efficient calculation function such as MD5, a cryptographic hash function specially developed for passwords should be used. In order to determine passwords through systematic trial and error (brute force method), significantly more computational effort is required for functions2 specially designed for passwords. Furthermore, an individual salt should be created for each password. This means that so much computing power has to be invested that it can become uninteresting to determine the passwords for the entire database.
However, there may be individual access points that are of particular interest to attackers. Above all, this can be administration access and access for prominent people. The weak point therefore remains the password chosen by the user. The rule here is that the password strength is significantly more influenced by its length than by its complexity (uppercase letters, lowercase letters, digits, special characters). However, longer passwords can also be determined with reasonable effort if they can be found in password lists. Users should therefore not use proverbs or quotations as passwords.
There are no hard and fast rules for the length and complexity of passwords. When a password is sufficiently strong also depends on the environment and the typical attack vectors. If, for example, access is blocked for a longer period of time after three to five incorrect entries or an additional medium is retained after unsuccessful attempts (e.g. EC card), a shorter and less complex password is sufficient.
In the case of high risks or sensitive data, multi-factor authentication should also be used. In addition to user ID and personal password, one-time passwords could be sent or security tokens could be used.

Further fine proceedings related to technical and organizational measures
Violations of Articles 25 and 32 GDPR also play a role in other proceedings, albeit often subordinate. In the absence of a legal basis, video recordings can violate Article 6 GDPR or Section 26 BDSG, and there can also be a violation of Article 25 GDPR if the areas in which people are moving or staying have not been pixelated. Contrary to Art. 25 Paragraph 2 GDPR, in such cases the person responsible has not taken any suitable technical measures by presetting that only personal data are processed, the processing of which is necessary for the respective specific processing purpose. In such a case, the violation of Article 6 GDPR or Section 26 BDSG has priority.
The store owner is responsible is the essence of the case and is NOT able to pass the buck.

To be misguiding others to patch up old software is totally the wrong way and is the complete opposite of what this case is about.

OPENCART does NOT advise using master branch for live sites

If you hold data you are responsible for it, no matter what software you use.

Open a different topic for bugs .

You are not understanding GDPR, it is a legal requirement for data controllers and falls outside the scope of the knowledge of mere computer developers. There are specialist cyber security experts you can contact to ensure your site adheres to the current GDPR standards. Maybe this will help you understand GDPR better : https://www.thecybersecurityexpert.com

Of interest : https://www.privacypolicies.com/blog/gd ... -projects/

DISCLAIMER:
You should not modify core files .. if you would like to donate a cup of coffee I will write it in a modification for you.


https://www.youtube.com/watch?v=zXIxDoCRc84


User avatar
Expert Member

Posts

Joined
Mon Jun 10, 2019 9:31 am

Post by OSWorX » Sun Aug 08, 2021 11:53 pm

In addition to mona, the GDPR force:

1. proper usage of (user) data
2. collect as less as possible user data
3. do nothing without users consent
4. inform users about which data you are processing (and why)
5. inform users what you are doing with their data
6. it is your obligation to store data in a safe place with most available security (e.g. HTTPS, SFTP, TLS, etc.)
7. it is your obligation to hold back unquaified people from accessing user data (as well as you own too)
8. if user data has (or wants) to be submitted to third parties (e.g. delivery, newsletter, analytics, etc. platforms, sharing with social media platforms), you have to sign a contract with them prior (attention: EC <> USA have currently no valid contract for such) - and inform your users about that
9. it is your obligation and duty when a user is asking about his stored data, that you answer with 30 days (can be expanded under some circumstances up to 60 days)

In short: the GDPR is basically about transparency and safeness.

Which leads also to following (which is mentioned in the article about the fine):

* it is also your obligation and duty to keep your system (server, webshop/CMS/etc software) up-to-date
* check also the used libraries
* don't install (already several times mentioned here and in this forum) software from sources you do not know
* don't use (and install) OpenCart extensions which require ionCube (some developers argue that the want to protect you the user, others with to protect their code because of the License [GNU/GPL]) - the true is, you will never know what such encoded scripts will really do in the background (true is, the GNU/GPL forbids the encrypting of any software published under that license - and every and each extension must be published under the GNU/GPL)

Finally: keeping OpenCart "up-to-date" does NOT mean to install or update or upgrade whenever a new version is published to that!
Always wait a few weeks with that if NO security advice was published!
And update/upgrade only if you have good reasons - a bad advice here is to have always the latest version.

And at least: if you are not that technical person, or having not the technical background, contact a professional you trust.

Never give out to someone you cannot trust, you cannot reach by phone (or has no physical address) login data.
And if you have done so, delete the access data to backend right after the work is done.
Have you give away the FTP-credentials to someone, delete that also.

As a general rule: create for everyone you want to give access to the backend or server a new, seperate login.

Full Stack Web Developer :: Dedicated OpenCart Development & Support DACH Region
Contact for Custom Work / Fast Support.


User avatar
Guru Member
Online

Posts

Joined
Mon Jan 11, 2010 10:52 pm
Location - Austria

Post by straightlight » Mon Aug 09, 2021 2:25 am

OSWorX wrote:
Sun Aug 08, 2021 11:53 pm
In addition to mona, the GDPR force:

1. proper usage of (user) data
2. collect as less as possible user data
3. do nothing without users consent
4. inform users about which data you are processing (and why)
5. inform users what you are doing with their data
6. it is your obligation to store data in a safe place with most available security (e.g. HTTPS, SFTP, TLS, etc.)
7. it is your obligation to hold back unquaified people from accessing user data (as well as you own too)
8. if user data has (or wants) to be submitted to third parties (e.g. delivery, newsletter, analytics, etc. platforms, sharing with social media platforms), you have to sign a contract with them prior (attention: EC <> USA have currently no valid contract for such) - and inform your users about that
9. it is your obligation and duty when a user is asking about his stored data, that you answer with 30 days (can be expanded under some circumstances up to 60 days)

In short: the GDPR is basically about transparency and safeness.

Which leads also to following (which is mentioned in the article about the fine):

* it is also your obligation and duty to keep your system (server, webshop/CMS/etc software) up-to-date
* check also the used libraries
* don't install (already several times mentioned here and in this forum) software from sources you do not know
* don't use (and install) OpenCart extensions which require ionCube (some developers argue that the want to protect you the user, others with to protect their code because of the License [GNU/GPL]) - the true is, you will never know what such encoded scripts will really do in the background (true is, the GNU/GPL forbids the encrypting of any software published under that license - and every and each extension must be published under the GNU/GPL)

Finally: keeping OpenCart "up-to-date" does NOT mean to install or update or upgrade whenever a new version is published to that!
Always wait a few weeks with that if NO security advice was published!
And update/upgrade only if you have good reasons - a bad advice here is to have always the latest version.


And at least: if you are not that technical person, or having not the technical background, contact a professional you trust.

Never give out to someone you cannot trust, you cannot reach by phone (or has no physical address) login data.
And if you have done so, delete the access data to backend right after the work is done.
Have you give away the FTP-credentials to someone, delete that also.

As a general rule: create for everyone you want to give access to the backend or server a new, seperate login.
That underlined portion has nothing to do with GPDR ...

Dedication and passion goes to those who are able to push and merge a project.

Regards,
Straightlight
Programmer / Opencart Tester


Legendary Member

Posts

Joined
Mon Nov 14, 2011 11:38 pm
Location - Canada, ON

Post by ADD Creative » Mon Aug 09, 2021 4:31 am

by mona wrote:
Sun Aug 08, 2021 10:27 pm
The store owner is responsible is the essence of the case and is NOT able to pass the buck.

To be misguiding others to patch up old software is totally the wrong way and is the complete opposite of what this case is about.

OPENCART does NOT advise using master branch for live sites

If you hold data you are responsible for it, no matter what software you use.

Open a different topic for bugs .

You are not understanding GDPR, it is a legal requirement for data controllers and falls outside the scope of the knowledge of mere computer developers. There are specialist cyber security experts you can contact to ensure your site adheres to the current GDPR standards. Maybe this will help you understand GDPR better : https://www.thecybersecurityexpert.com

Of interest : https://www.privacypolicies.com/blog/gd ... -projects/
I think there has been a misunderstanding. Nobody is looking to pass the buck. Nobody it looking to process personal data unnecessary. It's just a topic started to discuss, as data controllers, possible issues and ways to improve one of the tools used in processing personal data. This is in the spirit of one of the key principles (f: Integrity and confidentiality (security)) of the GDPR. If you use open source software it's your responsibility to ensure it's technically up to the job. Nobody is claiming the other principles can be ignored.

Since the examples posted here also related to the latest version. I'm not sure what you are suggesting, other than not to use OpenCart at all.

By the way, the sites you link to, while they may contain useful information, do fail at a couple of the key principles of the GDPR.

www.add-creative.co.uk


Expert Member

Posts

Joined
Sat Jan 14, 2012 1:02 am
Location - United Kingdom

Post by by mona » Mon Aug 09, 2021 6:52 am

@ADD CREATIVE
I beg to differ. The post started to ask for legal implications / advice based on inaccurate conclusions about a legal case.

If you read the second article again you will understand that discussing it on a public forum pretty much defeats the object. I get the conundrum . The article definitely does not state that open source software should not be used, it actually has some nice suggestions for open source users. If it is updated to current regulations I don’t know.

With regards to the positive on this thread, which is a positive and under any other thread, but it has nothing to do with GDPR, it is just another day in Opencart . I am not sure who “as data controllers” you refer to are supposed to be but may the lord help us if DPO’s come on here for legal advice, worse developers start giving legal advice to shop owners.

What I am suggesting is anyone who is concerned about their store and GDPR compliance would be well advised to get legal advice and not the advice from a forum.

DISCLAIMER:
You should not modify core files .. if you would like to donate a cup of coffee I will write it in a modification for you.


https://www.youtube.com/watch?v=zXIxDoCRc84


User avatar
Expert Member

Posts

Joined
Mon Jun 10, 2019 9:31 am

Post by boopbeepforum » Mon Aug 09, 2021 8:32 am

Oh, I found some minor problems with the password_hash() and password_verify(), which could affect the opencart master and the vqmod above in case anyone is thinking to switch to password_hash() from sha1.

For example in:

Code: Select all

password_hash($password, PASSWORD_DEFAULT)
If you accidentally misspell PASSWORD_ARGON2 instead of PASSWORD_ARGON2I then an empty string is written into the database as the password.

That's still ok since password_verify() would normally return false if the hash parameter is null, undefined, or empty. This is still better than comparing with a simple == operator in case both the password and hash are empty.

However, if somehow null or undefined variable is hashed by password_hash() and saved to the database (perhaps a very small chance since validate has a min and max character limit), and you pass a null, empty, or undefined variable password (for example login with an empty password) to password_verify(), then it returns true because null password is a valid comparison to a null hash.

To prevent any chance of password_verify() returning true for whatever reason when there is a null password or hash, I think changing the opencart master to this would be safer:

Code: Select all

if (!empty($password) && !empty($customer_query->row['password']) && password_verify(html_entity_decode($password, ENT_QUOTES, 'UTF-8'), $customer_query->row['password'])) {
https://github.com/opencart/opencart/bl ... er.php#L44

Newbie

Posts

Joined
Tue May 18, 2021 9:40 pm
Who is online

Users browsing this forum: No registered users and 20 guests