58

I have a simple website where I establish a connection to a MySQL server using PDO.

$dbh = new PDO('mysql:host=localhost;dbname=DB;port=3306',
               'USER',
               'SECRET', 
               array(PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8"));

I had some traffic on my site and the server's connection limit was reached, and the website throws this error, with my plain password in it!

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[08004] [1040] Too many connections' in /home/domain/html/index.php:xxx Stack trace: #0 /home/domain/html/index.php(64): PDO->__construct('mysql:host=loca...', 'USER', 'SECRET', Array) #1 {main} thrown in /home/domain/html/index.php on line 64

Ironically I switched to PDO for security reasons, so this really shocked me, because this exact error is something you can provoke very easily on most sites using simple HTTP flooding.

I have now wrapped my connection in a try/catch block, but still I think this is catastrophic!

I am new to PDO and so my question is: what do I have to do to consider to be safe? How do I establish a connection in a secure way? Are there other known security holes like this one that I have to be aware of?

5
  • See: stackoverflow.com/questions/5811834/… For a hole when using dynamic table/db/column names, and how to plug that hole.
    – Johan
    Jun 23, 2011 at 13:47
  • I wholeheartedly agree with turning off errors in production, try/catch and stuff like that but consider if you are having an offshore "team" of progammers where the password should not be known to "junior" programmers, this is, as you said a "catastrophic" security leak. Not to mention, novice programmers who don't care about turning off errors at all. With that said, I am baffled by this decision to reveal the password on error.
    – IMB
    May 29, 2012 at 13:43
  • 1
    Holy moly this is SICK! OH MY GOD! This is absolutely outrageous! You need more upvotes just for keeping your cool and not going into CAPS RAGE.
    – Sharky
    Jun 8, 2013 at 17:40
  • Possible duplicate: Uncaught PDOException reveals username and password Apr 21, 2020 at 16:46
  • The thing that none of these answers mention is that it's a bad practice to even let this information touch the logs (see OWASP logging cheat sheet). So you wouldn't want to simply catch the exception, give a generic message, and log the real PDO message, because then the credentials are in your logs. At the very least do a str_replace to redact that info before logging it.
    – Ben Y
    Jan 2, 2022 at 19:07

4 Answers 4

22

You should have display_errors = off in your PHP.ini anyway to avoid this problem. Errors that reveal details like these come from many places, in addition to PDO.

Yes, you should also have it in a try/catch block.

You can also $pdo->setAttribute(PDO::ERRMODE_SILENT), but then you need to be checking the error codes manually rather than using a try/catch block. See http://php.net/manual/en/pdo.setattribute.php for more error constants.

2
  • 3
    @Joe, then you should take that up with the PDO developers. I see no issue with it returning information from the stack like this. Once you are aware of it, it isn't an issue. Of course, the try/catch will be forgotten on some applications... inevitably it will be a problem for some... you're right about that.
    – Brad
    Jun 23, 2011 at 14:23
  • 2
    Both of these suggestions don't seem to work for me. When the connection fails, the error stack is printed to the screen with the plain text password visible. I am using PHP ActiveRecord. Jun 30, 2015 at 20:24
16

A simple workaround is to catch the PDOException thrown by the PDO constructor:

try {
    $dbh  =  new PDO('mysql:host=localhost;dbname=DB;port=3306', 'USER',
    'SECRET', array(PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8"));
} catch (PDOException $e) {
    throw new Exception($e->getMessage());
}
6
  • 3
    ...that is leaving a developer without an error message that otherwise would help them to solve a problem. Jun 2, 2016 at 6:00
  • 2
    Does getting exception "Could not connect to database" leave developer with no info? I don't think so
    – Matthias
    Jun 2, 2016 at 14:03
  • 12
    The base concept is bad. Nobody should place a plain password into an object and keep it in memory. The correct solution would be to change PDO code, not to put the plain password into the message of an exception. I consider this as a bug and a security hole in PDO itself.
    – nagylzs
    Apr 11, 2017 at 12:00
  • @YourCommonSense That's easily fixed (and I've included how in an edit).
    – user149341
    Aug 21, 2018 at 20:53
  • @duskwuff thank you a lot. It really does solve a problem. I already updated all my tutorials. The only thing I have to add is that if a function is used to wrap the connection code (as it usually is), it shouldn't accept raw credentials as its parameters, but accept them as an array or an object or retrieve in the body. Otherwise the problem will be moved one level up but persists. Apr 1, 2019 at 10:23
8

Ok, this made me giggle a little. The usage of error reporting is for debugging purposes, and it allows you to quickly find and fix issues.

When you're within a live environment your server should be configured for internal logging only, and not direct output, so basically you will need to turn off the output of errors within your php.ini.

display_errors = Off

But while you're within your test environment, this stack is merely a tool to help you and is configurable.

When errors occur within a live environment they would be logged, so you should always be checking your log files and then fix accordingly.

People may specify that you can manage errors within your PHP application, but by personal preference I think this is the wrong way to go about it. Configuring the INI and configuration files for your web-server and MySQL / SQL Server will result in more acute management.

If your application is a public application then it would also be a good idea to handle errors within the application as a large percentage of clients may be on shared hosting and not have full access to server configurations.

0
2

We use encoded username and passwords, and decode those in the PDO constructor. Then we catch the PDOException and throw a new PDOException with the old exception its message, so that the trace will show only the encoded username and password.

A good encryption library for PHP is defuse/php-encryption.

Example code:

<?php
class myPDOWrapper extends PDO
    {

        public function __construct(string $dns, string $encodedUser, string $encodedPassword)
        {
            try {
                parent::__construct($dns, $this->decodeFunction($encodedUser), $this->decodeFunction($encodedPassword),
                    [
                        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
                    ]
                );
            }
            catch (PDOException $exception) {
                throw new PDOException($exception->getMessage());
            }
        }

        private function decodeFunction(string $encoded): string
        {
            return \Defuse\Crypto\Crypto::decrypt($encoded, $this->decodeKey());
        }

        private function decodeKey(): \Defuse\Crypto\Key
        {
            static $key = null;

            if(null === $key) {
                $key = \Defuse\Crypto\Key::loadFromAsciiSafeString(getenv('MY_PDO_DECODE_KEY'));
            }

            return $key;
        }
    }

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.