On 2020/01/22 9:06, David Fetter wrote:
On Tue, Jan 21, 2020 at 03:27:50PM +0900, Fujii Masao wrote:
Hi,
When I was researching the maximum length of password in PostgreSQL
to answer the question from my customer, I found that there are two
minor issues in .pgpass file.
(1) If the length of a line in .pgpass file is larger than 319B,
libpq silently treats each 319B in the line as a separate
setting line.
This seems like a potentially serious bug. For example, a truncated
password could get retried enough times to raise intruder alarms, and
it wouldn't be easy to track down.
(2) The document explains that a line beginning with # is treated
as a comment in .pgpass. But as far as I read the code,
there is no code doing such special handling.
This is a flat-out bug, as it violates a promise the documentation has
made.
Also if the length of that "comment" line is larger than 319B,
the latter part of the line can be treated as valid setting.
You may think that these unexpected behaviors are not so harmful
in practice because "usually" the length of password setting line is
less than 319B and the hostname beginning with # is less likely to be
used. But the problem exists. And there are people who want to use
large password or to write a long comment (e.g., with multibyte
characters like Japanese) in .pgass, so these may be more harmful
in the near future.
For (1), I think that we should make libpq warn if the length of a line
is larger than 319B, and throw away the remaining part beginning from
320B position. Whether to enlarge the length of a line should be
a separate discussion, I think.
Agreed.
For (2), libpq should treat any lines beginning with # as comments.
Patch attached. This patch does the above (1) and (2).
Would it make sense for lines starting with whitespace and then # to
be treated as comments, too, e.g.:
Could you tell me why you want to treat such a line as comment?
Basically I don't want to change the existing rules for parsing
.pgpass file more thane necessary.
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
diff --git a/src/interfaces/libpq/fe-connect.c
b/src/interfaces/libpq/fe-connect.c
index 408000af83..eb34d2122f 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -6949,6 +6949,7 @@ passwordFromFile(const char *hostname, const char *port,
const char *dbname,
{
FILE *fp;
struct stat stat_buf;
+ int line_number = 0;
#define LINELEN NAMEDATALEN*5
char buf[LINELEN];
@@ -7014,10 +7015,42 @@ passwordFromFile(const char *hostname, const char
*port, const char *dbname,
*p1,
*p2;
int len;
+ int buflen;
if (fgets(buf, sizeof(buf), fp) == NULL)
break;
+ line_number++;
+ buflen = strlen(buf);
+ if (buflen >= sizeof(buf) - 1 && buf[buflen - 1] != '\n')
+ {
+ char tmp[LINELEN];
+ int tmplen;
+
+ /*
+ * Warn if this password setting line is too long,
+ * because it's unexpectedly truncated.
+ */
+ if (buf[0] != '#')
+ fprintf(stderr,
+ libpq_gettext("WARNING: line %d
too long in password file \"%s\"\n"),
+ line_number, pgpassfile);
+
+ /* eat rest of the line */
+ while (!feof(fp) && !ferror(fp))
+ {
+ if (fgets(tmp, sizeof(tmp), fp) == NULL)
+ break;
+ tmplen = strlen(tmp);
+ if (tmplen < sizeof(tmp) -1 || tmp[tmplen - 1]
== '\n')
+ break;
+ }
+ }
+
+ /* ignore comments */
+ if (buf[0] == '#')
+ continue;
+
/* strip trailing newline and carriage return */
len = pg_strip_crlf(buf);
