Database class exposes user & password on error

Kirby 2 Issues 🐛
1

If the database class’ constructor fails it exposes the connection string which includes the username and password for the mysql server.

This occurs in kirby toolkit 2.3.2

Uncaught exception ‘PDOException’ with message ‘SQLSTATE[42000] [1049] Unknown database ‘database’’ in /var/www/kirby/toolkit/lib/database.php:124 Stack trace: #0 /var/www/kirby/toolkit/lib/database.php(124): PDO->__construct(‘mysql:host=loca…’, ‘username’, ‘password’) #1 /var/www/kirby/toolkit/lib/database.php(69): Database->connect(Array) #2 /var/www/kirby/toolkit/lib/db.php(46): Database->__construct(Array) #3 /var/www/kirby/toolkit/lib/db.php(78): DB::connect() #4 /var/www/site/controllers/home.php(13): DB::query(‘select count(id…’) #5 [internal function]: Kirby\Registry\Controller::{closure}(Object(Site), Object(Children), Object(Page), Array) #6 /var/www/kirby/core/page.php(1424): call_user_func_array(Object(Closure), Array) #7

2

Oh, that’s not good. However I don’t know of a way to avoid this besides catching and suppressing the error (which is not great). It’s rather something PHP needs to fix if the fix should be proper.

Maybe you have another idea?

PS: You should always disable error reporting on production systems. Displaying errors there can give attackers a lot of insights (just like in this case). And it probably makes sense to connect to the database via a local port or socket that’s blocked for external access as MySQL is generally unencrypted anyway.

3

Catching the exception would be enough to fix the exposure.
You could trigger a custom error after catching the exception:
http://php.net/manual/en/function.set-error-handler.php

I agree with you that error reporting should be disabled but it would be better to not have people find this out the hard way :slight_smile:

4

Oh and you could use trigger_error also to send out the error message without the stacktrace:

http://php.net/manual/en/function.trigger-error.php