Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log message for database connection exposes the full URL that may contain plain text credentials #5339

Closed
jean553 opened this issue Mar 31, 2022 · 5 comments
Milestone

Comments

@jean553
Copy link

jean553 commented Mar 31, 2022

Q A
Version 3.3.4

The doctrine channel records the following log when a connection is established to the database:

Connecting with parameters array{
    "url":"postgresql://username:PLAIN_TEXT_PASSWORD@hostname/database_name",
    "driver":"pdo_pgsql",
    "host":"hostname",
    "port":null,
    "user":"username",
    "password":"<redacted>",
    "driverOptions":[],
    "serverVersion":"12",
    "defaultTableOptions":[],
    "dbname":"database_name",
    "charset":"utf8"
}

Although the password field value is not visible (<redacted> instead), the password is visible in the url field (https://github.com/doctrine/dbal/blob/3.3.x/src/Logging/Driver.php#L31). Using an URL instead of individual DB parameters is valid though (https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html#getting-a-connection).

This can be a security issue regarding where the logs are stored and who has access to those logs.

@allan-simon
Copy link

the url should be redacted (or at least the password part) or the log set to debug instead of info ?

@GCth
Copy link

GCth commented Apr 4, 2022

Using URL is actually a recommended configuration for recent Symfony releases, so this will affect a large number of users.

Ideally, the passwords should never appear in logs, no matter the logging level.

Also I do not think this information belongs to the info level, as during normal application operation, it's unnecessary, filling the logs with repetitive contents. Therefore I recommend lowering it to debug.

@greg0ire
Copy link
Member

greg0ire commented Apr 5, 2022

Do we know what versions are affected? I suspect it affects many versions, but was revealed only recently by doctrine/DoctrineBundle#1456

IMO a good first step would be to unset url if present. Anybody up to the task? This could be done in

private function maskPassword(array $params): array
{
if (isset($params['password'])) {
$params['password'] = '<redacted>';
}
return $params;
}

and tested in

public function testConnectAndDisconnect(): void
{
$this->logger->expects(self::exactly(2))
->method('info')
->withConsecutive(
[
'Connecting with parameters {params}',
[
'params' => [
'username' => 'admin',
'password' => '<redacted>',
],
],
],
['Disconnecting', []],
);
$this->driver->connect([
'username' => 'admin',
'password' => 'Passw0rd!',
]);
}

@stof
Copy link
Member

stof commented Apr 5, 2022

Do we know what versions are affected? I suspect it affects many versions, but was revealed only recently by doctrine/DoctrineBundle#1456

I suspect it affects only the new logging middleware, which would explain why it was revealed by that DoctrineBundle PR starting to use it in Symfony projects.

andrew-demb added a commit to andrew-demb/dbal that referenced this issue Apr 5, 2022
derrabus added a commit to derrabus/dbal that referenced this issue Apr 5, 2022
* 3.4.x:
  doctrine#5339 Redact connection URL from logs as it may contain sensitive data (password)
  Update PHPStan to 1.5.3
  Mark DBAL 2 as no longer maintained
  Remove documentation bits only relevant to DBAL 2
  Leverage int-mask-of to make types more precise
  Support TEXT/BLOB default values on MariaDB
@derrabus derrabus closed this as completed Apr 5, 2022
@morozov morozov added this to the 3.3.5 milestone Jul 15, 2022
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants