Hardening CC Database Access

[bsandall]

Typically when I'm working on a website with a database, I keep the database credentials in a file stored outside of the public web root directory so that it is impossible to access via URL, and I also set up different database users for regular users (e.g. customers) and those with elevated privileges (e.g. admins).

The first part can be done simply by editing the global.inc.php file to include a different file stored somewhere else:

<?php require CC_ROOT_DIR . '/../secret/path/my_hidden_db_credentials.php'; ?>

For additional security, I prefer not to track global.inc.php or any hidden files with the rest of my version controlled files.

However, for the second part, I'm wonder where the best place is to reestablish the database connection using elevated admin credentials; specifically, I would like to know when, exactly, it is determined that a user is an admin and that they are fully authenticated.

As far as I can tell after a little investigation, this would be right after the call to Sanitize::checkToken() in controller.admin.pre_session.inc.php - is that correct?

//Check security token
Sanitize::checkToken();
// Re-initialize Database class with elevated privileges now that identity is established (is it?)
//$GLOBALS['db']->close(); // this crashes for some reason
$GLOBALS['db'] = Database::getInstance(array(
		'dbhost'    =>$glob['dbhost'],
		'dbusername'=>DB_ADMIN_USER, // defined in hidden file
		'dbpassword'=>DB_ADMIN_PASS, // defined in hidden file
		'dbdatabase'=>$glob['dbdatabase'],
		'dbprefix'  =>$glob['dbprefix'],
	)
);

Also, my default database user has only the 4 basic permissions required for CRUD operations: Select, Insert, Update, Delete - I presume this will be sufficient for anything that a normal user is capable of doing while shopping and managing their account, but does anyone happen to know otherwise?

[bsmither]

I would like to know of your analysis of what database privileges that an admin would have over a customer where vulnerabilities would be avoided by not giving those privileges to the customer.

So far, all I can imagine is CREATE/ALTER, INDEX, and DROP/TRUNCATE. I can see inexplicable loss of data from mucking about with the schema, but I'm not seeing any deterrence against bad data getting into the table columns.

I see your approach with globals.inc.php, but it is a PHP file and will usually not output anything if in the URL of an HTTP request. The web browser will show the file if it is not configured to use a PHP parser.

Sanitize::checkToken() does not deal with admin/user distinction. It is to verify that the POSTed data includes the same hash that was previously sent to the one who requested the page. This must match -- as well as the cookie to maintain session state. Otherwise, POST and GET is discarded (but not REQUEST nor FILES).

[bsmither]

"I would like to know when, exactly, it is determined that a user is an admin and that they are fully authenticated."

In admin.php, a call is made to Admin::getInstance()->is(). The function getInstance() will instantiate the Admin class and run __contruct().

In __construct(), a call is made to _authenticate() with the POSTed username and password. If there is no username and password, _authenticate() is skipped.

In _authenticate(), Cubecart_admin_users is first queried for the enabled username. If no record found, _authenticate() returns false.

Then a call to _load() is made.

If a record was found (with just the username), the password is hashed and a query is made using the username and the hash.

If a record was found, and assuming this username is not on the 'blocked' list, the Admin class property _logged_in is set to true and the admin's admin_id is saved in the Session. Then a call to _load() is made.

In _load(), if the admin_id from Session looks like a zero, false is returned. Otherwise, a query is made using the enabled admin_id from Session. If a record was found, _logged_in is set to true. This function returns true or false, but is not used in this sequence.

Back in _authenticate(), assuming this session is not blocked, CubeCart 302's to admin.php where the process starts all over.

This time, _authenticate() is skipped, going straight to _load() with Session having a good admin_id.

Back in __construct(), with nothing else to do, the Admin class executes the is() function. Admin->is() simply returns the value of the Admin class property _logged_in.

The cookie contains the session ident, the session ident is validated with PHP, the POST is sanitized and the token is checked, and CubeCart loads the session data from the database. If that session data has an enabled admin_id, the visitor who sent the cookie in admin.php is logged in as an admin.

[havenswift-hosting]

Hi

While attempts at increasing security should always be applauded, I am not sure this solves any real problems or actually increases security much.

I don't believe that restricting the database access for non admin users will help at all but also cannot see any issues with restricting it to those permissions but would require reasonable changes to core code to achieve this with little gain so honestly cannot see it is worth the coding.

Preventing people from knowing the database credentials has always got to be good but the only way they can be used is if there are other, much more major, security issues with the account.  External access to the databases should always be IP restricted and tied down by the firewall and to access the database from localhost requires file access in which case the "hidden" included file with these details would be accessible anyway

Ian

[bsandall]

Security in layers, folks

Moving the database credentials out of the public directory is just one more layer - what if there is ever a vulnerability found in your firewall software, for example?

As for restricting database permissions, the principle of least privilege is a standard approach to a more secure system, so why the opposition? It's the same reason if you run a server you don't just give every user root access - maybe they wouldn't break anything on purpose, and maybe your other settings would prevent them if they tried, but why take the chance?

Normally I would grant granular permissions at the table level so non-admin users can only affect tables that they would reasonably be accessing / modifying, but unfortunately my current hosting provider does not allow permissions at that level (even though MySQL supports it natively), so I have to make do with what I have.

Besides, it hardly takes any changes at all to core code - literally a handful of lines in 2 files - so that's not an issue for me.

EDIT: Actually, it also requires one more change to allow a new database instance:

public static function getInstance($config = '', $create_new = false) {
	if (!(self::$_instance instanceof self) || $create_new) {
		self::$_instance = new self($config);
	}

	return self::$_instance;
}

Thanks @bhsmither for tracing the admin login path - saved me some time this morning!

Edited December 22, 2015 by bsandall
Additional info

[havenswift-hosting]

Security in layers, folks

Moving the database credentials out of the public directory is just one more layer - what if there is ever a vulnerability found in your firewall software, for example?

As for restricting database permissions, the principle of least privilege is a standard approach to a more secure system, so why the opposition? It's the same reason if you run a server you don't just give every user root access - maybe they wouldn't break anything on purpose, and maybe your other settings would prevent them if they tried, but why take the chance?

Security by Obfuscation is not security at all but what Brian and I have said is not blind opposition, just pointing out that although your measures may help in a small way, in order to exploit these areas, your account needs to have already been exploited in a far greater and more dangerous way.

There is absolutely nothing wrong in having separate database connections for admin and non admin users and if it can easily be achieved (I havent even looked and to be fair, you two are probably far better coders than myself) and the code fed back into core then great, go for it

[bsandall]

Security by Obfuscation is not security at all

I totally agree, but this is not security by obscurity, it is preventing your credentials from having any URL at all so they are completely inaccessible* from the web no matter what. Here's a StackOverflow question I searched up for you that discusses this.

However as you said, someone with root / direct access to the server would still be able to read your file, but there isn't a whole lot you can do about that.

Security by obscurity would be renaming the admin folder to '21097590GOihEA907' or running your secure server from a non-standard port - they do add some measure of security in the sense that you are less likely to be attacked (which can be very significant), but you still need to have real security measures in place if you ever are attacked.

* Unless someone can upload and run a .php file or other script to your server, at which point it is trivial to read the contents of any file in your home directory whether public or not, but then you have already been compromised.

[havenswift-hosting]

Security by Obfuscation is not security at all

I totally agree, but this is not security by obscurity, it is preventing your credentials from having any URL at all so they are completely inaccessible* from the web no matter what. Here's a StackOverflow question I searched up for you that discusses this.

Hi

Would putting the config file above the publicly accessible level make it marginally more secure - maybe.  Is it complex to do - not at all.  Why do so few mainstream packages do it - because the risk is negligible to none, except on such a poorly configured server that this would actually be the least of your worries.  Given that you know the rules and structure of a CubeCart site and so know the location of the global.inc.php file - what good does that do you ?  If I published the database credentials for a specific site here, there is no access to that database externally. If you had a normal hosting account on the same server where you knew these details, as there is no ability to access across accounts, it does you no good.

Does it make it more secure, I suppose you could argue that it is marginally more secure, but in order to exploit this insecurity, the server security has to be so poor as to make it worthless.  Ensure your website is hosted with a hosting company that takes security seriously.  Is any site 100% secure, of course not, but the script kiddies that target sites like this, do so through software  holes (as in the recent but thankfully rare incident with CubeCart) rather than OS level issues.

Ian

[bsandall]

I don't see why you are so vehemently opposed to me moving my credentials outside of the root directory, yet on Github you raise the issue of forcing users to rename the admin folder... well, anyone with a strong password shouldn't have to worry about that, right?

Whether an attack vector is likely or not is irrelevant - no one is likely to break into my house, but I still lock the door, and advising people to do otherwise is sort of like malpractice for web security. Both moving the credentials out of the root directory and renaming the admin folder are simple steps that address fairly unlikely attacks.

Unless you host your own server (which you do), then you never know what the people managing your server are going to do with it. Maybe they hire someone new, or some setting gets bungled in an upgrade, etc, so why not take a minute to take some simple precautions, and why argue against it?

[havenswift-hosting]

I don't see why you are so vehemently opposed to me moving my credentials outside of the root directory

Vehemently opposed ?  I give up