Skip to content

Commit 59fb391

Browse files
committed
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.
1 parent 1c63665 commit 59fb391

4 files changed

Lines changed: 72 additions & 27 deletions

File tree

dlg_specific.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,9 +687,19 @@ copyConnAttributes(ConnInfo *ci, const char *attribute, const char *value)
687687
}
688688
else if (stricmp(attribute, INI_PQOPT) == 0 || stricmp(attribute, ABBR_PQOPT) == 0)
689689
{
690+
char *hide_str = hide_password(value);
691+
690692
NULL_THE_NAME(ci->pqopt);
691693
ci->pqopt_in_str = TRUE;
692694
ci->pqopt = decode_or_remove_braces(value);
695+
/*
696+
* A pqopt value can embed a libpq 'password=...', so log a copy
697+
* with the password masked and skip the generic logging below.
698+
*/
699+
MYLOG(0, "key='%s' value='%s'\n", attribute, hide_str ? hide_str : "");
700+
if (hide_str)
701+
free(hide_str);
702+
printed = TRUE;
693703
}
694704
else if (stricmp(attribute, INI_UPDATABLECURSORS) == 0 || stricmp(attribute, ABBR_UPDATABLECURSORS) == 0)
695705
ci->allow_keyset = pg_atoi(value);

drvconn.c

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -39,33 +39,6 @@
3939

4040
#define NULL_IF_NULL(a) (a ? a : "(NULL)")
4141

42-
/*
43-
* Mask the password in a connection string before it is written to the log,
44-
* so that debug logs (MyLog/CommLog) can be shared without leaking the
45-
* database credentials. Matches the PWD keyword case-insensitively.
46-
*/
47-
static char * hide_password(const char *str)
48-
{
49-
char *outstr, *pwdp;
50-
51-
if (!str) return NULL;
52-
outstr = strdup(str);
53-
if (!outstr) return NULL;
54-
for (pwdp = outstr; *pwdp; pwdp++)
55-
{
56-
if (strnicmp(pwdp, "PWD=", 4) == 0)
57-
break;
58-
}
59-
if (*pwdp)
60-
{
61-
char *p;
62-
63-
for (p=pwdp + 4; *p && *p != ';'; p++)
64-
*p = 'x';
65-
}
66-
return outstr;
67-
}
68-
6942
/* prototypes */
7043
static BOOL dconn_get_DSN_or_Driver(const char *connect_string, ConnInfo *ci);
7144
static BOOL dconn_get_connect_attributes(const char *connect_string, ConnInfo *ci);

misc.c

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,3 +312,59 @@ quote_table(const pgNAME schema, const pgNAME table, char *buf, int buf_size)
312312

313313
return buf;
314314
}
315+
316+
/*
317+
* Return a malloc'd copy of a connection string with any password value
318+
* masked, so that debug logs (MyLog/CommLog) can be shared without leaking
319+
* the database credentials. Both keywords are matched case-insensitively:
320+
*
321+
* PWD= the ODBC password attribute; its value runs to the next ';'
322+
* password= the libpq password keyword, e.g. inside a pqopt={...} value;
323+
* its value is whitespace-delimited (or single-quoted) and ends
324+
* at the closing brace of the pqopt block.
325+
*
326+
* The caller is responsible for free()ing the returned string.
327+
*/
328+
char *
329+
hide_password(const char *str)
330+
{
331+
char *outstr, *p;
332+
333+
if (!str)
334+
return NULL;
335+
outstr = strdup(str);
336+
if (!outstr)
337+
return NULL;
338+
for (p = outstr; *p; )
339+
{
340+
if (strnicmp(p, "PWD=", 4) == 0)
341+
{
342+
for (p += 4; *p && *p != ';'; p++)
343+
*p = 'x';
344+
}
345+
else if (strnicmp(p, "password=", 9) == 0)
346+
{
347+
p += 9;
348+
if (*p == '\'') /* libpq single-quoted value */
349+
{
350+
for (p++; *p && *p != '\''; p++)
351+
{
352+
if (*p == '\\' && p[1])
353+
*p++ = 'x';
354+
*p = 'x';
355+
}
356+
if (*p == '\'')
357+
p++;
358+
}
359+
else
360+
{
361+
for (; *p && *p != ';' && *p != '}' &&
362+
*p != ' ' && *p != '\t'; p++)
363+
*p = 'x';
364+
}
365+
}
366+
else
367+
p++;
368+
}
369+
return outstr;
370+
}

misc.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ FUNCTION_BEGIN_MACRO \
9898
FUNCTION_END_MACRO
9999

100100

101+
/*
102+
* Return a malloc'd copy of a connection string with password values masked,
103+
* for safe logging. The caller must free() the result.
104+
*/
105+
char *hide_password(const char *str);
106+
101107
#ifdef __cplusplus
102108
}
103109
#endif

0 commit comments

Comments
 (0)