2

I'm just learning PDO and need to know if this connection (it works) is really secure. I put my connections in an inaccessible folder outside of the root.

Is this well formed or can I improve it?

Do I need the $conn->setAttribute...EXCEPTION?

$user = 'JoeUser';
$pass = 'JoesPassword';

try {
  $conn = new PDO("mysql:host=myWebHost.com;dbname=MydataBase", $user, $pass);
  $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
} 
catch(PDOException $e) {
  echo 'ERROR: ' . $e->getMessage();
}
Peter Mortensen
  • 28,342
  • 21
  • 95
  • 123
AllThisOnAnACER
  • 331
  • 4
  • 17
  • secure or not, it depends on your security requirements. Since your password is stored in plain text, anybody has access rights to server still can read the password. Read this: http://stackoverflow.com/questions/97984/how-to-secure-database-passwords-in-php – Raptor Mar 10 '14 at 03:10
  • Good article recommendation. This is in fact on ssl. Are you saying that, even though this resides outside of the public folder, I should still put my username and password into a separate file and bring them in with an include.php? – AllThisOnAnACER Mar 10 '14 at 03:16
  • 1
    Write your password in a config file, and put the file outside web root & source control folders is already enough for general practice. – Raptor Mar 10 '14 at 03:17

2 Answers2

3

if this connection (it works) is really secure.

Not quite.

You are revealing a database error to the whole world. And it could contain some sensitive details such as database structure. One can call it anything but secure.

Do I need the $conn->setAttribute...EXCEPTION?

Yes but that's a bit irrelevant.

can I improve it?

You are assigning only the username and password to variables. What about the database? Other configurable stuff?

But in the first place you should really avoid echoing the database error information. As a rule, you just shouldn't catch an error of this level. It should just let your application die. However, in this particular case of a database connection error, the stack trace would contain the database credentials and you don't want to see them even in the error log, least displayed on the screen. Hence, you got to catch this error and then throw a brand new exception that wold contain only the message but not the stack trace.

$user    = 'JoeUser';
$pass    = 'JoesPassword';
$host    = 'myWebHost.com';
$db      = 'MydataBase';
$charset = 'utf8mb4';

try {
    $conn = new PDO("mysql:host=$host;dbname=$db;charset=$charset", $user, $pass);
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
} catch (\PDOException $e) {
     throw new \PDOException($e->getMessage(), (int)$e->getCode());
}

This way, in case of the connection error we will have a proper exception, but without any sensitive details such as database password. And this exception, in turn, will be hanlded according the the site-wide configuration - either displayed on screen, logged, or handled by a dedicated error handler.

As of the configuration options, they can be detached and stored in some configuration file.

Your Common Sense
  • 152,517
  • 33
  • 193
  • 313
  • Well, gee Dad, I appreciate the information, but, is the paddle really necessary this time. Promise, won't do it again. :-) Honestly, thanks for the tune up. I'm trying to work with all this new info coming at me and at times a good slap is necessary. – AllThisOnAnACER Mar 10 '14 at 14:28
  • But removing the try-catch may now reveal the first 16 characters of the database password (if the PDO constructor fails, like *"getaddrinfo failed: Temporary failure in name resolution"*)? Perhaps add some details (or reference) about these scenarios and how they can be avoided? Related question: *[Why does PDO print my password when the connection fails?](https://stackoverflow.com/questions/6455018)* – Peter Mortensen Apr 21 '20 at 07:16
  • @PeterMortensen thanks, I was editing the answer to address this very issue :) – Your Common Sense Apr 21 '20 at 07:21
0

In itself it's fine, but keep in mind that an insecure part of the application can change that. If you have any file access methods called within the application and don't have adequate input sanitization then even something above the root can sometimes be read by an attacker.

You'll want to ensure that permissions for that file are appropriately restricted, especially if you're on a shared host.

The error mode thing is not necessary for what you're asking, but I'd recommend it anyway.

The simple echo in the catch is not recommended. Try to set up an error/exception logging system that you can monitor as a developer. It's insecure and unprofessional to simply write raw error messages to the page.

Please wrap all code in classes. Global scope variables are a nightmare, and global scope code WILL make your life harder.

Hans
  • 2,120
  • 22
  • 21