Skip to content

Redact the DB password in connection strings written to the debug log - #206

Open
davecramer wants to merge 1 commit into
mainfrom
fix-password-in-debug-log
Open

Redact the DB password in connection strings written to the debug log#206
davecramer wants to merge 1 commit into
mainfrom
fix-password-in-debug-log

Conversation

@davecramer

Copy link
Copy Markdown
Contributor

Problem

hide_password() exists to mask the PWD value in a connection string before it is written to the log, so that MyLog/CommLog output can be shared for debugging without leaking the database credentials.

It was compiled out by an unconditional #define FORCE_PASSWORD_DISPLAY (present since 2013). With MyLog (Debug) enabled, the driver therefore logged the full connection string -- including PWD=... in cleartext -- at the connStrIn, szConnStrOut and our_connect_string log sites.

Reproduced against PostgreSQL 18: connecting with ...;PWD=HORSEBATTERY42; and MyLog enabled wrote PWD=HORSEBATTERY42 verbatim into the log.

Fix

  • Remove the FORCE_PASSWORD_DISPLAY define so the existing redaction branches are actually used.
  • Make the PWD match case-insensitive, so an application-supplied Pwd=/pwd= is masked as well as the driver's own generated PWD=.
  • Fix the pointer-sign and format (ssize_t) warnings in the szConnStrOut redaction branch that were previously never compiled.

After the fix, the same connection logs PWD=xxxxxxxxxxxxxx. Verified with PWD=, pwd= and Pwd= variants -- all redacted, no cleartext.

Scope / severity

Logging is off by default (mylog_on = 0; the MYLOG macro is a no-op and no log file is created unless debug logging is explicitly enabled), so this only affected users who turned on debug tracing, and additionally required access to the resulting log file. There is no default-on exposure and no remote vector. This is a low-severity hardening fix (CWE-532), not assigned a CVE.

Follow-up (not in this PR)

hide_password() masks the top-level PWD= only. A password embedded in a pqopt={...} value is redacted elsewhere (log_redacted_pqopt in connection.c) for the pqopt-specific logging, but the raw connStrIn/szConnStrOut logging could still surface it. Worth a separate look.

Reported-by: @Alpop12

hide_password() masks the PWD value in a connection string before it is
logged, so MyLog/CommLog output can be shared for debugging without
leaking the database credentials.  It was compiled out by an
unconditional '#define FORCE_PASSWORD_DISPLAY' (present since 2013), so
with MyLog enabled the driver logged the full connection string --
including PWD=... in cleartext -- at the connStrIn, szConnStrOut and
our_connect_string log sites.

Remove the FORCE_PASSWORD_DISPLAY define so the existing redaction is
actually used, and make the PWD match case-insensitive so an
application-supplied 'Pwd='/'pwd=' is masked as well as the driver's own
'PWD='.  Also fixes the pointer-sign and format warnings in the
szConnStrOut redaction branch that were previously never compiled.

Logging is off by default, so this only affected users who explicitly
enabled debug logging; no default-on exposure.

Reported-by: Alpop12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant