Redact the DB password (ODBC PWD and libpq pqopt password) in debug logs - #206
Open
davecramer wants to merge 2 commits into
Open
Redact the DB password (ODBC PWD and libpq pqopt password) in debug logs#206davecramer wants to merge 2 commits into
davecramer wants to merge 2 commits into
Conversation
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
hide_password() only masked the ODBC 'PWD=' attribute, so a password
embedded in a libpq pqopt value (pqopt={... password=secret}) still
leaked into MyLog via szConnStrOut (drvconn.c) and via the per-attribute
logging in copyConnAttributes (dlg_specific.c).
Move hide_password() to misc.c so it can be shared, and teach it to mask
the libpq 'password=' keyword as well: the value is whitespace-delimited
(or single-quoted, honoring backslash escapes) and ends at the closing
brace of the pqopt block. Use it to redact the pqopt value where
copyConnAttributes logs it, and skip the generic raw logging for that
key. Non-secret pqopt fields (host, sslmode, application_name, ...)
remain visible for debugging.
Verified against PostgreSQL 18: ODBC PWD, unquoted pqopt password and
single-quoted pqopt password are all masked; no cleartext remains.
davecramer
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
hide_password()exists to mask the password 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 the password in cleartext.Two forms of password leaked:
PWD=attribute (connStrIn,szConnStrOut,our_connect_stringlog sites in drvconn.c).password=embedded in apqopt={...}value -- viaszConnStrOutand via the per-attribute logging incopyConnAttributes(dlg_specific.c).hide_password()did not know about the libpqpassword=keyword.Reproduced against PostgreSQL 18 for both forms, including a single-quoted
pqopt={... password='my secret'}.Fix
FORCE_PASSWORD_DISPLAYdefine so the existing redaction branches are actually used.hide_password()tomisc.cso it can be shared, and teach it to mask both keywords, case-insensitively:PWD=-- ODBC attribute; value runs to the next;.password=-- libpq keyword (e.g. insidepqopt); value is whitespace-delimited or single-quoted (backslash escapes honored) and ends at the closing brace.pqoptvalue wherecopyConnAttributeslogs it, and skip the generic raw logging for that key. Non-secret pqopt fields (host, sslmode, application_name, ...) stay visible for debugging.ssize_t) warnings in theszConnStrOutredaction branch that were previously never compiled.After the fix the same connections log
PWD=xxxx,password=xxxx, andpassword='xxxx'(quoted) -- verified withPWD=/pwd=/Pwd=and unquoted/quoted pqopt passwords; no cleartext remains.Scope / severity
Logging is off by default (
mylog_on = 0; theMYLOGmacro 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. No default-on exposure, no remote vector. Low-severity hardening (CWE-532); no CVE.Reported-by: @Alpop12