Hello Michael,
Hmmm. It does apply on a test I just did right know...
That's weird, it does not apply for me either with patch -p1 and there
is on conflict in fe-connect.c, which looks easy enough to fix by the
way.
Weird indeed. However, even if the patch applied cleanly for me, it was
not compiling anymore. Attached an updated version, in which I also tried
to improve the error message on trailing garbage.
--
Fabien
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 5e7931ba90..a6cbbea655 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1591,6 +1591,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</varlistentry>
</variablelist>
</para>
+
+ <para>
+ Integer values expected for keywords <literal>port</literal>,
+ <literal>connect_timeout</literal>,
+ <literal>keepalives_idle</literal>,
+ <literal>keepalives_interval</literal> and
+ <literal>keepalives_timeout</literal> are parsed strictly.
+ Versions of <application>libpq</application> before
+ <product>PostgreSQL 12</product> accepted trailing garbage or overflows.
+ </para>
</sect2>
</sect1>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index db5aacd0e9..5ea51d5c55 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1586,6 +1586,35 @@ useKeepalives(PGconn *conn)
return val != 0 ? 1 : 0;
}
+/*
+ * Parse integer, report any syntax error, and tell if all is well.
+ */
+static bool
+strict_atoi(const char *str, int *val, PGconn *conn, const char *context)
+{
+ char *endptr;
+
+ errno = 0;
+ *val = strtol(str, &endptr, 10);
+
+ if (errno != 0)
+ {
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid integer \"%s\" for keyword \"%s\": %s\n"),
+ str, context, strerror(errno));
+ return false;
+ }
+ else if (*endptr != '\0')
+ {
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid integer \"%s\" for keyword \"%s\": trailing garbage\n"),
+ str, context);
+ return false;
+ }
+
+ return true;
+}
+
#ifndef WIN32
/*
* Set the keepalive idle timer.
@@ -1598,7 +1627,9 @@ setKeepalivesIdle(PGconn *conn)
if (conn->keepalives_idle == NULL)
return 1;
- idle = atoi(conn->keepalives_idle);
+ if (!strict_atoi(conn->keepalives_idle, &idle, conn, "keepalives_idle"))
+ return 0;
+
if (idle < 0)
idle = 0;
@@ -1630,7 +1661,9 @@ setKeepalivesInterval(PGconn *conn)
if (conn->keepalives_interval == NULL)
return 1;
- interval = atoi(conn->keepalives_interval);
+ if (!strict_atoi(conn->keepalives_interval, &interval, conn, "keepalives_interval"))
+ return 0;
+
if (interval < 0)
interval = 0;
@@ -1663,7 +1696,9 @@ setKeepalivesCount(PGconn *conn)
if (conn->keepalives_count == NULL)
return 1;
- count = atoi(conn->keepalives_count);
+ if (!strict_atoi(conn->keepalives_count, &count, conn, "keepalives_count"))
+ return 0;
+
if (count < 0)
count = 0;
@@ -1697,13 +1732,17 @@ setKeepalivesWin32(PGconn *conn)
int idle = 0;
int interval = 0;
- if (conn->keepalives_idle)
- idle = atoi(conn->keepalives_idle);
+ if (conn->keepalives_idle &&
+ !strict_atoi(conn->keepalives_idle, &idle, conn, "keepalives_idle"))
+ return 0;
+
+ if (conn->keepalives_interval &&
+ !strict_atoi(conn->keepalives_interval, &interval, conn, "keepalives_interval"))
+ return 0;
+
if (idle <= 0)
idle = 2 * 60 * 60; /* 2 hours = default */
- if (conn->keepalives_interval)
- interval = atoi(conn->keepalives_interval);
if (interval <= 0)
interval = 1; /* 1 second = default */
@@ -1817,7 +1856,9 @@ connectDBComplete(PGconn *conn)
*/
if (conn->connect_timeout != NULL)
{
- timeout = atoi(conn->connect_timeout);
+ if (!strict_atoi(conn->connect_timeout, &timeout, conn, "connect_timeout"))
+ return 0;
+
if (timeout > 0)
{
/*
@@ -1828,6 +1869,8 @@ connectDBComplete(PGconn *conn)
if (timeout < 2)
timeout = 2;
}
+ else /* negative means 0 */
+ timeout = 0;
}
for (;;)
@@ -2094,7 +2137,9 @@ keep_going: /* We will come back to here until there is
thisport = DEF_PGPORT;
else
{
- thisport = atoi(ch->port);
+ if (!strict_atoi(ch->port, &thisport, conn, "port"))
+ goto error_return;
+
if (thisport < 1 || thisport > 65535)
{
appendPQExpBuffer(&conn->errorMessage,
