See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by iplocker » Sun Oct 11, 2020 2:26 am

Hello.
I m facing a weird issue (actually its been years which I am facing this issue ) when I m going something to checkout I see other customers details at the fields .
This time I was even logged in with the customer account !!!
I cant reproduce the issue so my report to Journal Support has not solution .
Is this Opencart issue or Journal ?
EDIT: I see now I have report years ago the same issue: viewtopic.php?t=186673 :(
Thanks
iplocker
Active Member
Posts
201
Joined
Sun May 26, 2013 6:39 pm
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by sw!tch » Sun Oct 11, 2020 2:38 am

A lot of troubleshooting was already listed in that thread.

What have you tried?

Can you duplicate in a different browser?
How big is your session table?

Any caching enabled with your hosting? Check your hosting control panel.

Backup and learn how to recover before you make any changes!

sw!tch
Active Member
Posts
1470
Joined
Sat Apr 28, 2012 2:32 pm
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by khnaz35 » Sun Oct 11, 2020 9:26 am

sw!tch wrote: ↑
Sun Oct 11, 2020 2:38 am
 A lot of troubleshooting was already listed in that thread.

What have you tried?

Can you duplicate in a different browser?
How big is your session table?

Any caching enabled with your hosting? Check your hosting control panel.
Looks like might be quite big already since the OP has issue over the years and didn't take care of it.


Or the other solution could be do a fresh install on the same server with same setting in different directory and see if you are able to reproduce the issue.

Got an urgent question that’s keeping you up at night? There might just be a magical inbox ready to help: khnaz35@gmail.com
Enjoy nature ;) :) :-*

User avatar
khnaz35
Active Member
Posts
1445
Joined
Mon Aug 27, 2018 11:30 pm
Location - Malaysia
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by letxobnav » Mon Oct 12, 2020 12:37 pm

This can only happen when you share a session_id.
Under normal, proper site operations (^), that can only happen when you are issueing/using the same session_id while that session_id is already given.

Chances of that are very very small but not zero.

The change is increased when:
1) you have a huge backlog of used session records in your database (which is common in default OC) or file system. Remember, bots get a new session id on every request so that db table can fill pretty fast if not cleaned properly.
2) hackers who send requests with random/edited session cookies in the hope one is active, very rare, with virtually no chance but very difficult to counter (*).
3) Users who never close their browser and thus retain their session_id indefinitely which now may have been issued to someone else or some "remember me" implementation where the session cookie is no longer a session cookie but has a defined lifetime (*).

(*) many sites (incl OC) simply use the session_id presented in the cookie even if that session_id was not issued by the server nor whether that session is actually active on the server. As long as it is a valid format, that session_id is used.
(^) excludes unstable/experimental one-page checkout implementations

Item 1. you can counter by:
a) making sure your session garbage collection works properly which is the preferred method with the least performance impact.
b) checking whether a new session_id is already active before issueing/using it which adds extra effort, be it a minute one.

a)
For those sites we use the database for session storage we added this function to the file system/library/session/db.php

Code: Select all

	public function __destruct() {
		if (ini_get('session.gc_divisor')) $gc_divisor = ini_get('session.gc_divisor');
		else $gc_divisor = 1;
		if (ini_get('session.gc_probability')) $gc_probability = ini_get('session.gc_probability');
		else $gc_probability = 1;
		if ((rand() % $gc_divisor) < $gc_probability) {
			$expire_dt = date('Y-m-d H:i:s', time() - ini_get('session.gc_maxlifetime'));
			$this->db->query("DELETE FROM `" . DB_PREFIX . "session` WHERE expire < '".$expire_dt."'");
		}
	}
That should take better care of the session garbage collection, the default gc() function is crap as it compares a timestamp with a date-time value apart from the fact that it is never called. The file storage class already has a __destruct() function and works fine.

b)
we changed the start function in system/library/session.php to:

Code: Select all

public function start($session_id = false, $no_clash = true) {
		if ($session_id) {
			// determine if session id is valid format
			if (!preg_match('/^[a-zA-Z0-9,\-]{22,52}$/', $session_id)) {
				error_log('Error: invalid session ID given:'.$session_id.' by '.$_SERVER['REMOTE_ADDR'].', issueing a new one...');
				$session_id = false;
			}
		}
		if (!$session_id) {
			// issue new session id
			$session_id_active = true;
			while ($session_id_active) {
				// generate a new session id
				if (function_exists('random_bytes')) $session_id = substr(bin2hex(random_bytes(32)), 0, 32);
				else $session_id = substr(bin2hex(openssl_random_pseudo_bytes(32)), 0, 32);
				// exit loop if no_clash is false or session id not active
				if (!$no_clash) $session_id_active = false;
				elseif (!$this->adaptor->session_active($session_id)) $session_id_active = false;
			}
		}
		$this->session_id = $session_id;
		$this->data = $this->adaptor->read($session_id);
		return $session_id;
	}
We added to system/library/session/db.php

Code: Select all

	public function session_active ($session_id) {
		// determine if session_id is present in the session table
		$this->db->query("SELECT session_id FROM " . DB_PREFIX . "session WHERE session_id = '" . $this->db->escape($session_id) . "'");
		if (!$query->num_rows) return false;
		return true;
	}
We added to system/library/session/file.php

Code: Select all

	public function session_active ($session_id) {
		// determine if session_id is present in the file system
		$file = DIR_SESSION . 'sess_' . basename($session_id);
		clearstatcache(true, $file);
		if (!file_exists($file)) return false;
		return true;
	}
These function will check whether a newly generated session_id is already active server-side and generate a new one if it is.
Both for db and file stored sessions. You can toggle that check with the $no_clash parameter.

In addition, as stated (*), OC will use any session_id given to it via the cookie as long as it has the right format, which is not good.
We check whether the given session_id actually is active server-side or that given session_id is ignored.

catalog/controller/startup/session.php
change:

Code: Select all

			if (isset($_COOKIE[$this->config->get('session_name')])) {
				$session_id = $_COOKIE[$this->config->get('session_name')];
			} else {
				$session_id = '';
			}
			$this->session->start($session_id);
to:

Code: Select all

		$session_id = '';
			if (!empty($_COOKIE[$this->config->get('session_name')])) {
				if (SESSION_VERIFY) {
					if ($this->config->get('session_engine') == 'db') {
						$query = $this->db->query("SELECT session_id FROM " . DB_PREFIX . "session WHERE session_id = '" . $this->db->escape($_COOKIE[$this->config->get('session_name')]) . "'");
						if ($query->num_rows) $session_id = $_COOKIE[$this->config->get('session_name')];
					} else {
						$session_file = DIR_SESSION.'sess_'.$_COOKIE[$this->config->get('session_name')];
						if (is_file($session_file))	$session_id = $_COOKIE[$this->config->get('session_name')];
					}
				} else {
					$session_id = $_COOKIE[$this->config->get('session_name')];
				}
			}
			$this->session->start($session_id,SESSION_NOCLASH);
system/framework.php
change:

Code: Select all

	if (isset($_COOKIE[$config->get('session_name')])) {
		$session_id = $_COOKIE[$config->get('session_name')];
	} else {
		$session_id = '';
	}
	$this->session->start($session_id);
to:

Code: Select all

	$session_id = false;
	if (isset($_COOKIE[$config->get('session_name')])) {
		if (SESSION_VERIFY) {
			if ($config->get('session_engine') == 'db') {
				$query = $db->query("SELECT session_id FROM " . DB_PREFIX . "session WHERE session_id = '" . $db->escape($_COOKIE[$config->get('session_name')]) . "'");
				if ($query->num_rows) $session_id = $_COOKIE[$config->get('session_name')];
			} else {
				$session_file = DIR_SESSION.'sess_'.$_COOKIE[$config->get('session_name')];
				if (is_file($session_file))	$session_id = $_COOKIE[$config->get('session_name')];
			}
		} else {
			$session_id = $_COOKIE[$config->get('session_name')];
		}
	}
	$this->session->start($session_id,SESSION_NOCLASH);

change:

Code: Select all

// Database
if ($config->get('db_autostart')) {
	$registry->set('db', new DB($config->get('db_engine'), $config->get('db_hostname'), $config->get('db_username'), $config->get('db_password'), $config->get('db_database'), $config->get('db_port')));
}
to:

Code: Select all

// Database
if ($config->get('db_autostart')) {
	$db = new DB($config->get('db_engine'), $config->get('db_hostname'), $config->get('db_username'), $config->get('db_password'), $config->get('db_database'), $config->get('db_port'));
	$registry->set('db', $db);
}
to be able to access the database from here.
(Turning verify on in framework and session controller makes the format check redundant as an invalid formatted session id would never be found in the database or file system)

define the switches in config
config.php

Code: Select all

define('SESSION_VERIFY',	true); // verify if the given session id exists server side
define('SESSION_NOCLASH',	false); // prevent issueing new session id which already exists
As said, under proper operations, the chance of clashing sessions is very small but not zero and when it does happen, the impact/embarassment is rather large.
So make your own choice on whether the yield is worth the extra effort.

Crystal Light Centrum Taiwan
Extensions: MailQueue | SUKHR | VBoces

“Data security is paramount at [...], and we are committed to protecting the privacy of anyone who is associated with our [...]. We’ve made a lot of improvements and will continue to make them.”
When you know your life savings are gone.

User avatar
letxobnav
Expert Member
Posts
3264
Joined
Fri Aug 18, 2017 4:35 pm
Location - Taiwan
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by paulfeakins » Mon Oct 12, 2020 6:40 pm

iplocker wrote: ↑
Sun Oct 11, 2020 2:26 am
 I cant reproduce the issue so my report to Journal Support has not solution .
You will need to be able to reproduce the issue before anyone can help you.

UK OpenCart Hosting | OpenCart Audits | OpenCart Support - please email info@antropy.co.uk

User avatar
paulfeakins
Legendary Member
Online
Posts
10053
Joined
Mon Aug 22, 2011 11:01 pm
Location - London Gatwick, United Kingdom
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by JNeuhoff » Mon Oct 12, 2020 7:08 pm

@letxobnav : Would you be able for these changes to be submitted as a pull-request on github? I think your changes could make the OpenCart session handling more reliable.

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig

User avatar
JNeuhoff
Guru Member
Posts
7482
Joined
Wed Dec 05, 2007 3:38 am
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by letxobnav » Tue Oct 13, 2020 12:12 am

@letxobnav : Would you be able for these changes to be submitted as a pull-request on github? I think your changes could make the OpenCart session handling more reliable.
Sure but then you would have to explain to me how to do a pull-request.

One thing I forgot to add:
as mentioned, bots do not retain sessions and as such they invoke a new session generation on each request.
That is why the vast majority of your sessions in your session table or file system are for bots (you thought you had a lot of customers).

Since bots have no use for them beyond the lifetime of the script, those sessions can be removed immediately after script end.
(in the olden days it was common practice to not issue sessions to bots in the first place but that is not a good practice as the script itself and the content it produces relies on variables stored and used in the session during execution)

We do this by identifying public bots (those identifying themselves as such via the user agent, assuming everybody knows how to do that) and direct the session class to delete the session upon final session close (script end).

In catalog/controller/startup/startup.php we identify if the request comes from a public bot.
If it is, we set a session variable:

Code: Select all

$this->data['force_remove_session'] = false;
if ($bot) $this->data['force_remove_session'] = true;
in system/library/session.php we change the shutdown function.
change:

Code: Select all

	public function close() {
		$this->adaptor->write($this->session_id, $this->data);
	}
to:

Code: Select all

	public function close() {
		$this->adaptor->write($this->session_id, $this->data);
		if (isset($this->data['force_remove_session']) && $this->data['force_remove_session']) {
			$this->adaptor->destroy($this->session_id);
		}
	}
That way any session given to a bot is deleted after the script for that request ends and that will reduce the session table/directory to a large extend not waiting for the normal session garbage collection to kick in.

if you want to double check if it really is deleting the session of a public bot, add this:

Code: Select all

error_log('Bot session force delete: '.$this->session_id.' '.$_SERVER['REMOTE_ADDR'].' '.$_SERVER['HTTP_USER_AGENT']);
before this:

Code: Select all

$this->adaptor->destroy($this->session_id);
This scheme adds a query/file operation but who cares, its a bot.

you can of course also just increase the character count of the session id from 32 to say 50.
That also reduces the chance of clashes but it still is not zero (just like the lottery) and as such it is more like an arms-race.

Crystal Light Centrum Taiwan
Extensions: MailQueue | SUKHR | VBoces

“Data security is paramount at [...], and we are committed to protecting the privacy of anyone who is associated with our [...]. We’ve made a lot of improvements and will continue to make them.”
When you know your life savings are gone.

User avatar
letxobnav
Expert Member
Posts
3264
Joined
Fri Aug 18, 2017 4:35 pm
Location - Taiwan
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by JNeuhoff » Tue Oct 13, 2020 5:59 pm

letxobnav wrote: ↑
Tue Oct 13, 2020 12:12 am
@letxobnav : Would you be able for these changes to be submitted as a pull-request on github? I think your changes could make the OpenCart session handling more reliable.
Sure but then you would have to explain to me how to do a pull-request.
https://docs.github.com/en/free-pro-tea ... ll-request

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig

User avatar
JNeuhoff
Guru Member
Posts
7482
Joined
Wed Dec 05, 2007 3:38 am
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by xxvirusxx » Tue Oct 13, 2020 6:54 pm

letxobnav wrote: ↑
Tue Oct 13, 2020 12:12 am
@letxobnav : Would you be able for these changes to be submitted as a pull-request on github? I think your changes could make the OpenCart session handling more reliable.
Sure but then you would have to explain to me how to do a pull-request.
https://docs.github.com/en/free-pro-tea ... ll-request

If you make changes on your repo for multiple files or other modification.
If you use linux or on Windows (need to dowload Git)

1. Fork a repo (Fork option from Right side)
2. Clone your forked repo on PC

Code: Select all

git clone https://github.com/letxobnav/opencart
Now you make changes on files, directories etc...(local opencart).
You need to cd-in opencart folder from your cloned location then you can run:

3. Prepare to upload local repo to your github repo. From Terminal or Git (Windows)

Code: Select all

git init
git remote add origin https://github.com/letxobnav/opencart
git add .
git commit -m "I have made some changes"
git push origin master
Here you need to enter your github username and password. Then all changes from local repo will be on your github repo.

After that you can create a Pull request from your repo, to oficial opencart repo.

Upgrade Service | OC 2.3.0.2 PHP 8 | My Custom OC 3.0.3.8 | Buy me a beer

User avatar
xxvirusxx
Expert Member
Posts
4157
Joined
Tue Jul 17, 2012 10:35 pm
Location - România
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by iplocker » Tue Oct 13, 2020 7:54 pm

Hello.
I am alone in the pc which is at my office, no one is working on it except me so not chance of I took other session which works before me .
Here is my session table at the db : http://prntscr.com/uyfayy

Anyway as I cant reproduce the issue I cant say more.
But it seems letxobnav has a phd at the issue ;D
So it will be nice his code to be adapted to OC .

Thanks
iplocker
Active Member
Posts
201
Joined
Sun May 26, 2013 6:39 pm
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by khnaz35 » Tue Oct 13, 2020 7:57 pm

iplocker wrote: ↑
Tue Oct 13, 2020 7:54 pm
 But it seems letxobnav has a phd at the issue ;D
Certainly he does ;) he had posted code regarding session many times on different forums to put OP in right direction +1 from my side too

Got an urgent question that’s keeping you up at night? There might just be a magical inbox ready to help: khnaz35@gmail.com
Enjoy nature ;) :) :-*

User avatar
khnaz35
Active Member
Posts
1445
Joined
Mon Aug 27, 2018 11:30 pm
Location - Malaysia
Top

Re: See other customer details at checkout OC 3.0.3.6 Journal 3.1.4

Quote

Post by by mona » Thu Oct 22, 2020 10:39 pm

The __destruct function in the class will not work for the database solution as the database connection is already gone.

Garbage collection for DB stored sessions.
system/library/session/db.php:

replace the __construct function with:

Code: Select all

public function __construct($registry) {
$this->db = $registry->get('db');

// session expire using php.ini, defaults to 1 hour
$this->expire = ini_get('session.gc_maxlifetime') ?? 3600;

// session garbage collection using php.ini, defaults to 1% probability
$gc_probability = ini_get('session.gc_probability') ?? 1;
$gc_divisor = ini_get('session.gc_divisor') ?? 100;
if ((rand() % $gc_divisor) < $gc_probability) $this->gc();
}
That will make sure the gc is actually called and sets the right divisor/probability settings

replace the gc function with:

Code: Select all

public function gc() {
$dt = date('Y-m-d H:i:s',((int)time()));
$this->db->query("DELETE FROM " . DB_PREFIX . "session WHERE expire < '".$dt."'");
}
That will make sure that the query compares datetime with datetime and not timestamp with datetime.

replace the read function with:

Code: Select all

public function read($session_id) {
$query = $this->db->query("SELECT data FROM " . DB_PREFIX . "session WHERE session_id = '" . $this->db->escape($session_id) . "'");
if ($query->num_rows) return json_decode($query->row['data'], true);
else return false;
}
That will remove the expire condition from the query as it slows the query down comparing timestamp with datetime.

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
by mona
Expert Member
Posts
3295
Joined
Mon Jun 10, 2019 9:31 am
Top
Post Reply
Return to “Themes”
Who is online

Users browsing this forum: No registered users and 8 guests