From 1c636657f0c68dfb9a7a960c820d903e6da6ea80 Mon Sep 17 00:00:00 2001 From: Dave Cramer Date: Thu, 27 Aug 2026 18:19:37 -0400 Subject: [PATCH 1/2] Redact the password in connection strings written to the log 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 --- drvconn.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drvconn.c b/drvconn.c index 58ff2d86..53fafd9e 100644 --- a/drvconn.c +++ b/drvconn.c @@ -37,10 +37,13 @@ #include "dlg_specific.h" -#define FORCE_PASSWORD_DISPLAY #define NULL_IF_NULL(a) (a ? a : "(NULL)") -#ifndef FORCE_PASSWORD_DISPLAY +/* + * Mask the password in a connection string before it is written to the log, + * so that debug logs (MyLog/CommLog) can be shared without leaking the + * database credentials. Matches the PWD keyword case-insensitively. + */ static char * hide_password(const char *str) { char *outstr, *pwdp; @@ -48,9 +51,12 @@ static char * hide_password(const char *str) if (!str) return NULL; outstr = strdup(str); if (!outstr) return NULL; - if (pwdp = strstr(outstr, "PWD="), !pwdp) - pwdp = strstr(outstr, "pwd="); - if (pwdp) + for (pwdp = outstr; *pwdp; pwdp++) + { + if (strnicmp(pwdp, "PWD=", 4) == 0) + break; + } + if (*pwdp) { char *p; @@ -59,7 +65,6 @@ static char * hide_password(const char *str) } return outstr; } -#endif /* prototypes */ static BOOL dconn_get_DSN_or_Driver(const char *connect_string, ConnInfo *ci); @@ -308,8 +313,8 @@ MYLOG(DETAIL_LOG_LEVEL, "before CC_connect\n"); char *hide_str = NULL; if (cbConnStrOutMax > 0) - hide_str = hide_password(szConnStrOut); - MYLOG(0, "szConnStrOut = '%s' len=%d,%d\n", NULL_IF_NULL(hide_str), len, cbConnStrOutMax); + hide_str = hide_password((char *) szConnStrOut); + MYLOG(0, "szConnStrOut = '%s' len=" FORMAT_SSIZE_T ",%d\n", NULL_IF_NULL(hide_str), len, cbConnStrOutMax); if (hide_str) free(hide_str); } From 59fb3911c3a42682ecb1df6eae0fa52fa2c8b05b Mon Sep 17 00:00:00 2001 From: Dave Cramer Date: Fri, 28 Aug 2026 18:32:27 -0400 Subject: [PATCH 2/2] Also redact libpq 'password=' (e.g. inside pqopt) in logged conn strings 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. --- dlg_specific.c | 10 +++++++++ drvconn.c | 27 ------------------------ misc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ misc.h | 6 ++++++ 4 files changed, 72 insertions(+), 27 deletions(-) diff --git a/dlg_specific.c b/dlg_specific.c index 7f8ad79e..c252ee06 100644 --- a/dlg_specific.c +++ b/dlg_specific.c @@ -687,9 +687,19 @@ copyConnAttributes(ConnInfo *ci, const char *attribute, const char *value) } else if (stricmp(attribute, INI_PQOPT) == 0 || stricmp(attribute, ABBR_PQOPT) == 0) { + char *hide_str = hide_password(value); + NULL_THE_NAME(ci->pqopt); ci->pqopt_in_str = TRUE; ci->pqopt = decode_or_remove_braces(value); + /* + * A pqopt value can embed a libpq 'password=...', so log a copy + * with the password masked and skip the generic logging below. + */ + MYLOG(0, "key='%s' value='%s'\n", attribute, hide_str ? hide_str : ""); + if (hide_str) + free(hide_str); + printed = TRUE; } else if (stricmp(attribute, INI_UPDATABLECURSORS) == 0 || stricmp(attribute, ABBR_UPDATABLECURSORS) == 0) ci->allow_keyset = pg_atoi(value); diff --git a/drvconn.c b/drvconn.c index 53fafd9e..35d6258e 100644 --- a/drvconn.c +++ b/drvconn.c @@ -39,33 +39,6 @@ #define NULL_IF_NULL(a) (a ? a : "(NULL)") -/* - * Mask the password in a connection string before it is written to the log, - * so that debug logs (MyLog/CommLog) can be shared without leaking the - * database credentials. Matches the PWD keyword case-insensitively. - */ -static char * hide_password(const char *str) -{ - char *outstr, *pwdp; - - if (!str) return NULL; - outstr = strdup(str); - if (!outstr) return NULL; - for (pwdp = outstr; *pwdp; pwdp++) - { - if (strnicmp(pwdp, "PWD=", 4) == 0) - break; - } - if (*pwdp) - { - char *p; - - for (p=pwdp + 4; *p && *p != ';'; p++) - *p = 'x'; - } - return outstr; -} - /* prototypes */ static BOOL dconn_get_DSN_or_Driver(const char *connect_string, ConnInfo *ci); static BOOL dconn_get_connect_attributes(const char *connect_string, ConnInfo *ci); diff --git a/misc.c b/misc.c index 66a88573..7dd24508 100644 --- a/misc.c +++ b/misc.c @@ -312,3 +312,59 @@ quote_table(const pgNAME schema, const pgNAME table, char *buf, int buf_size) return buf; } + +/* + * Return a malloc'd copy of a connection string with any password value + * masked, so that debug logs (MyLog/CommLog) can be shared without leaking + * the database credentials. Both keywords are matched case-insensitively: + * + * PWD= the ODBC password attribute; its value runs to the next ';' + * password= the libpq password keyword, e.g. inside a pqopt={...} value; + * its value is whitespace-delimited (or single-quoted) and ends + * at the closing brace of the pqopt block. + * + * The caller is responsible for free()ing the returned string. + */ +char * +hide_password(const char *str) +{ + char *outstr, *p; + + if (!str) + return NULL; + outstr = strdup(str); + if (!outstr) + return NULL; + for (p = outstr; *p; ) + { + if (strnicmp(p, "PWD=", 4) == 0) + { + for (p += 4; *p && *p != ';'; p++) + *p = 'x'; + } + else if (strnicmp(p, "password=", 9) == 0) + { + p += 9; + if (*p == '\'') /* libpq single-quoted value */ + { + for (p++; *p && *p != '\''; p++) + { + if (*p == '\\' && p[1]) + *p++ = 'x'; + *p = 'x'; + } + if (*p == '\'') + p++; + } + else + { + for (; *p && *p != ';' && *p != '}' && + *p != ' ' && *p != '\t'; p++) + *p = 'x'; + } + } + else + p++; + } + return outstr; +} diff --git a/misc.h b/misc.h index da8cdfce..ff76d9b6 100644 --- a/misc.h +++ b/misc.h @@ -98,6 +98,12 @@ FUNCTION_BEGIN_MACRO \ FUNCTION_END_MACRO +/* + * Return a malloc'd copy of a connection string with password values masked, + * for safe logging. The caller must free() the result. + */ +char *hide_password(const char *str); + #ifdef __cplusplus } #endif