33

I am having pet-peeve with PHP's PDO class. As you can see on the Warning note: it reveals database password by default if an exception was not caught (when display errors is on which is a probable situation in a production server for novice webmasters).

Here's my argument with a PHP Dev

Me:

It is quite unusual to reveal passwords since MySQL itself conceals passwords for debugging or when error occurs. For example the typical output of MySQL is "Access denied for user 'root'@'localhost' (using password: YES)" it never reveals password.

As we know many novice programmers or end-users who uses third-party software that rely on PDO probably don't care about display_errors being turned off.

I think this a security risk. For example, a malicious user can run a spider and search for websites with the string "Fatal error: Uncaught exception 'PDOException'" and they can now harvest passwords.

I hope you consider this.

PHP Core Dev:

Yes, running production server with display_errors=on and printing out backtraces with argument is a security risk. That's why you should never do it.

It appears that the PHP Core Dev would rather rely on the end-user to turn off error displays. Granting they made a warning, still it seems unacceptable given that MySQL itself (the one PDO is abstracting) is not revealing the password and yet PDO prefers to reveal it.

Do you think the PHP Core Devs are right on revealing passwords on error?

11
  • 8
    I think we all agree this is unacceptable. The question now is how can we persuade them to do something about it? The conversation I pasted above was already my attempt to suggest this change. It was done in php.net's bug/suggestion area and as you've read in their reply, they don't seem to take it seriously. Anybody knows other ways to put this message across?
    – IMB
    Commented May 30, 2012 at 10:44
  • 1
    I'm siding with the PHP guys - for security to be compromised then it requires that 1) errors are not being managed properly and 2) that the database is then accessible either directly or via another vulnerability (e.g. PHP code injection). The flip side is that code complexity in error handling should be minimal, and that supplying the wrong auth details can be a common cause for connection failures in development.
    – symcbean
    Commented May 30, 2012 at 11:24
  • 3
    @symcbean heavy traffic is another probable cause of this in a production server. Example: stackoverflow.com/questions/6455018/…
    – IMB
    Commented May 30, 2012 at 14:42
  • 3
    The bug report appears to be here, but it is marked private, alas.
    – D.W.
    Commented Jun 3, 2012 at 5:11
  • 1
    @D.W. The problem is even older than that: bugs.php.net/bug.php?id=56552
    – Walf
    Commented Dec 12, 2021 at 23:03

5 Answers 5

31

This question is loaded, but I completely agree on the answer: no, passwords should not be revealed in error messages. Passwords should never be written out anywhere without the explicit consent of the user. Consider the following roles and how they might access the wrong password that the user just typed:

  • The user. That's the only role who should be able to know what she just typed. Fine.
  • Someone shoulder-surfing. Most password entry forms obscure the password into ****; showing a password on-screen defeats this protection. This is irrelevant if the password is not shown in a production environment (but more on this below).
  • An administrator of the system where the password was. A rogue admin can generally obtain the password by inserting some surreptitious logging code, however this is an active attack, at risk of being detected. Even if the password is only shown in development configurations, it means the code is there and can be reactivated with an innocent-looking change of many configuration variables at once. It is common practice to hide passwords from administrators: they are not logged by any non-web general-public software that I've ever seen; system administrators and IT support staff routinely avert their eyes when a user is typing a password in their presence.
  • A developer of the application to whom a bug report has been made. That role never should have access to passwords. Including passwords in error traces, even if they are only shown to those who have the most use from the traces, is not good.
  • An attacker stealing a backup of error traces, or being able to see a backtrace du to a misconfiguration (such as accidentally setting display_errors=on).

The value of the wrong password as an asset depends on what it is. If it is a typo on the correct password, the wrong password is practically as valuable as the correct password. If the password is a password for another site (oops, I typed my production site password in the user login form on the test environment), it is practically as valuable as a credential to that site. Revealing the wrong password has high risk of disclosing a high-value asset.

The developer's response is deeply unsatisfactory:

Yes, running production server with display_errors=on and printing out backtraces with argument is a security risk. That's why you should never do it.

First, there is a strong security concern even in a development environment, even if the logs are only ever shown to those who would legitimately see them.

Second, everything “is a security risk”, but some risks are worse than others. Backtraces can reveal confidential information that may be assets in themselves or may become one step is an attack path. That's not as bad as handing out passwords on a silver platter.

Zeroth and foremost, this response shows a very narrow view of security: “if you use my software correctly, you won't have any direct risk”. Even if that was true (it isn't), security is a holistic concern. It is difficult to ensure that every component of a larger system is used precisely as the author of that component intended. Hence components must be robust — this is also known as “defense in depth”. A rule like “never log passwords” is simpler than “don't show backtraces to those (who?) who shouldn't see them, and turn them off anyway (and if you need them, too bad)”.

There might be a difficulty in recognizing what is a password and what isn't. It can be argued that to hide passwords creates the expectation that passwords will always be hidden, which is a bad thing if they aren't. I think the rate of success is more than enough to justify doing best effort here.

1
  • 7
    Now, weigh your list of reasons why you should not display the password against the list of reasons why you should: the only diagnostic value of displaying the password is to help you determine if you mis-typed the password. But how difficult is that problem to diagnose otherwise? I, personally, tend to only make that particular mistake once per project (early on), and whenever I have any kind of database connectivity issue, my first instinct is always to double-check the connection strings anyway. So, yeah, upvote for "don't display the password." Commented May 30, 2012 at 14:39
11

I'd NEVER hand credentials back for any reason during an exception, it's up to the developer to do something like a print of var_dump to verify what they're passing in, anything else can leak data that shouldn't be.

Of course it's best practice to never show exception messages to users, and under other circumstances it can easily be used to further exploit the site, but this is too easy and I don't see a clear reason WHY you would pass that sort of detail back.

MySQL's errors are much better (user@address, password yes/no), informative but not too much data leakage (though it could be argued I don't want people to know internal addresses, or my authentication mechanism).

PHP is also known for not really holding your hand at all, and throwing you to the sharks when you do something wrong (because it's poorly documented), so seems par for the course though.

10

The way PDO does it is absolutely wrong, and there is no excuse for it.

Error messages, even on a properly-configured system, do end up in all sorts of places - error logs, user-facing error messages, etc. People make mistakes, and the risk of exposing database credentials is just way too high, while the benefit (pinpointing a bad password) is marginal at best. When such an error occurs, the culprit may be something like a invalid SQL, a constraint violation, network problems, or a corrupt DB server. Even if the password is indeed wrong, there is no reason to show it; just throwing with a message along the lines of "invalid database credentials" or "access to database denied" would be enough; if you suspect that the password isn't what you want it to be, there are plenty of ways to debug this specifically. Even if there were no risk at all, including the password in the error message would still be unnecessary.

If you browse the PHP bug repository, I'm sure you will find plenty of discussion about this issue and similar ones. To keep it short: the issue has been discussed to death, and the responsible developers do not seem to agree that the current behavior is problematic, so it is highly unlikely that it will be changed anytime soon.

This means that you'll have to come up with a solution yourself. A last-straw solution that works is to configure a global exception handler that just str_replaces the real password with a series of asterisks. Alternatively, you could force PDO to not throw, but then you lose the comfort of exception-based error handling, and you still have to sanitize PDO's error messages (only you now have to tediousy peel them out of the PDO's and PDOStatements yourself).

1

Just hide the password using str_replace().

// Connect using PDO
try 
{   
    $this->dbh = new PDO('mysql:host='.DB_HOST.';dbname='.DB_NAME , DB_USER , DB_PASS); 
    $this->dbh->setAttribute(PDO::ATTR_ERRMODE , PDO::ERRMODE_EXCEPTION);           
}
catch(PDOException $e)
{
    echo str_replace(DB_PASS, ' *** LOOK @CONFIG FOR YOUR SECRET PASSWORD! *** ' , $e);
}
1
  • 1
    It's functional, but I'm not sure it's the best of methods. Also, if the error included any user input, the user could use that error message to brute force attack the password.
    – Jeff Ferland
    Commented Sep 23, 2012 at 22:51
-3

Normally, when an exception is triggered, it means some undefined error happened. And what follows is the why and what of the exception. PHP Core Developers are Absolutely right to display passwords depending on the kind of error which triggered the exception.

And of course, the display of passwords for whatsoever reason is unsafe. It is the task of production developers to conceal passwords and eventually hide error messages.

When you develop an application to be used by others, you take care to be as explicit as possible. For that reason, almost all apps are configured for minimal and efficient default options, and it is the responsibility of the final user to make sure the configurations are optimal for his application.

2
  • 2
    passwords should never be revealed anywhere. period.
    – Casey
    Commented Oct 20, 2012 at 21:35
  • 2
    It is sufficient to say "Bad password" if that is the case. Otherwise the password must remain 100% secret, including obsfucated in RAM. Dumping the password to PHP logs, memory dumps or php://stdout is seriously stupid. The password could be shared with other credentials to other systems, or who knows, maybe the server is a public server.
    – oxygen
    Commented Dec 12, 2014 at 15:47

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .