mikemccaffrey
commented
8 years ago Well, this certainly looks like a bug that should be addressed!
Open quicksketch opened 8 years ago
mikemccaffrey
commented
8 years ago Well, this certainly looks like a bug that should be addressed!
jlfranklin
commented
6 years ago The patch redirects the user to the installer when the active configuration is not yet setup (fixes the "but it should have" in the original issue), and tries to connect to the database before telling the user the site is already installed.
Now, when there is a bad database URL string, the user will get:
SQLSTATE[HY000] [1044] Access denied for user 'dbuser'@'localhost' to database 'backdrop-x'Maybe not the most friendly, but it gives the user a clue to the real problem.
jlfranklin
commented
6 years ago Moving this back to "needs work" as it will disclose the database user ID and database name to any visitor if the database ever becomes unavailable. This isn't enough to allow an attacker to get access to the database, but it does make it a lot easier. Returning a simpler, more generic "Cannot connect to the database, check the credentials" would be safer.
jlfranklin
commented
6 years ago This is going to be a judgement call for @quicksketch and @jenlampton.
The patch causes an error message to be displayed to any visitor that discloses the username, hostname, and database name of the intended connection, but not the password. From a conservative security perspective, that's a lot of information to disclose to a non-admin. The error will appear anytime the site cannot find the database, which includes:
The first two are not a concern, IMHO. They would generally happen before the site is installed, and therefore the time window would be limited.
It's the last one that bugs me. A database outage (including maintenance on the database server), a glitch in the network between web server and database (including fat-fingered firewall rules), or an overloaded database that cannot accept new connections would all trigger that error message, and that would go out to however many visitors per minute they have times the duration of the outage.
On the other hand, just spitting an error "can't connect to the database, please check the database connection credentials" may be difficult for a rookie web developer to diagnose. Maybe a longer error message would be better, or a link to a help page on b.o, directing them to check a list of things and then contact their hosting provider for more assistance?
Nevermind, I convinced myself while writing this comment. The error message can't have that much database connection information in it. It should show a link to a "common problems" page somewhere under https://backdropcms.org/support with details on how to fix it.
klonos
commented
6 years ago From a conservative security perspective, that's a lot of information to disclose to a non-admin.
It is a lot of info indeed. If there was a way to tell that the site has not been installed yet (via checking config for example), then I guess it would be slightly safer to assume that this is an installation problem and that this info needs to be conveyed to the person performing the installation. Otherwise yeah, it's too risky to be disclosing all that info.
jenlampton
commented
5 years ago This issue still needs more feedback, removing milestone, but adding milestone candidate tag.
This issue is very similar to the already closed issue at https://github.com/backdrop/backdrop-issues/issues/413.
Steps:
/core/install.php. Visiting it manually now gives me:After I create the database, both the automatic redirection and the installer work properly. This seems like an usual situation where a user would enter in database credentials in settings.php without actually having made the database first, but these things happen.