Commit e7a22179 authored by Michael Paquier's avatar Michael Paquier

Parse more strictly integer parameters from connection strings in libpq

The following parameters have been parsed in lossy ways when specified
in a connection string processed by libpq:
- connect_timeout
- keepalives
- keepalives_count
- keepalives_idle
- keepalives_interval
- port

Overflowing values or the presence of incorrect characters were not
properly checked, leading to libpq trying to use such values and fail
with unhelpful error messages.  This commit hardens the parsing of those
parameters so as it is possible to find easily incorrect values.

Author: Fabien Coelho
Reviewed-by: Peter Eisentraut, Michael Paquier
Discussion: https://postgr.es/m/alpine.DEB.2.21.1808171206180.20841@lancre
parent 0d45cd96
...@@ -1587,6 +1587,34 @@ useKeepalives(PGconn *conn) ...@@ -1587,6 +1587,34 @@ useKeepalives(PGconn *conn)
return val != 0 ? 1 : 0; return val != 0 ? 1 : 0;
} }
/*
* Parse and try to interpret "value" as an integer value, and if successful,
* store it in *result, complaining if there is any trailing garbage or an
* overflow.
*/
static bool
parse_int_param(const char *value, int *result, PGconn *conn,
const char *context)
{
char *end;
long numval;
*result = 0;
errno = 0;
numval = strtol(value, &end, 10);
if (errno == 0 && *end == '\0' && numval == (int) numval)
{
*result = numval;
return true;
}
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("invalid integer value \"%s\" for keyword \"%s\"\n"),
value, context);
return false;
}
#ifndef WIN32 #ifndef WIN32
/* /*
* Set the keepalive idle timer. * Set the keepalive idle timer.
...@@ -1599,7 +1627,9 @@ setKeepalivesIdle(PGconn *conn) ...@@ -1599,7 +1627,9 @@ setKeepalivesIdle(PGconn *conn)
if (conn->keepalives_idle == NULL) if (conn->keepalives_idle == NULL)
return 1; return 1;
idle = atoi(conn->keepalives_idle); if (!parse_int_param(conn->keepalives_idle, &idle, conn,
"keepalives_idle"))
return 0;
if (idle < 0) if (idle < 0)
idle = 0; idle = 0;
...@@ -1631,7 +1661,9 @@ setKeepalivesInterval(PGconn *conn) ...@@ -1631,7 +1661,9 @@ setKeepalivesInterval(PGconn *conn)
if (conn->keepalives_interval == NULL) if (conn->keepalives_interval == NULL)
return 1; return 1;
interval = atoi(conn->keepalives_interval); if (!parse_int_param(conn->keepalives_interval, &interval, conn,
"keepalives_interval"))
return 0;
if (interval < 0) if (interval < 0)
interval = 0; interval = 0;
...@@ -1664,7 +1696,9 @@ setKeepalivesCount(PGconn *conn) ...@@ -1664,7 +1696,9 @@ setKeepalivesCount(PGconn *conn)
if (conn->keepalives_count == NULL) if (conn->keepalives_count == NULL)
return 1; return 1;
count = atoi(conn->keepalives_count); if (!parse_int_param(conn->keepalives_count, &count, conn,
"keepalives_count"))
return 0;
if (count < 0) if (count < 0)
count = 0; count = 0;
...@@ -1698,13 +1732,17 @@ setKeepalivesWin32(PGconn *conn) ...@@ -1698,13 +1732,17 @@ setKeepalivesWin32(PGconn *conn)
int idle = 0; int idle = 0;
int interval = 0; int interval = 0;
if (conn->keepalives_idle) if (conn->keepalives_idle &&
idle = atoi(conn->keepalives_idle); !parse_int_param(conn->keepalives_idle, &idle, conn,
"keepalives_idle"))
return 0;
if (idle <= 0) if (idle <= 0)
idle = 2 * 60 * 60; /* 2 hours = default */ idle = 2 * 60 * 60; /* 2 hours = default */
if (conn->keepalives_interval) if (conn->keepalives_interval &&
interval = atoi(conn->keepalives_interval); !parse_int_param(conn->keepalives_interval, &interval, conn,
"keepalives_interval"))
return 0;
if (interval <= 0) if (interval <= 0)
interval = 1; /* 1 second = default */ interval = 1; /* 1 second = default */
...@@ -1831,7 +1869,10 @@ connectDBComplete(PGconn *conn) ...@@ -1831,7 +1869,10 @@ connectDBComplete(PGconn *conn)
*/ */
if (conn->connect_timeout != NULL) if (conn->connect_timeout != NULL)
{ {
timeout = atoi(conn->connect_timeout); if (!parse_int_param(conn->connect_timeout, &timeout, conn,
"connect_timeout"))
return 0;
if (timeout > 0) if (timeout > 0)
{ {
/* /*
...@@ -1842,6 +1883,8 @@ connectDBComplete(PGconn *conn) ...@@ -1842,6 +1883,8 @@ connectDBComplete(PGconn *conn)
if (timeout < 2) if (timeout < 2)
timeout = 2; timeout = 2;
} }
else /* negative means 0 */
timeout = 0;
} }
for (;;) for (;;)
...@@ -2108,7 +2151,9 @@ keep_going: /* We will come back to here until there is ...@@ -2108,7 +2151,9 @@ keep_going: /* We will come back to here until there is
thisport = DEF_PGPORT; thisport = DEF_PGPORT;
else else
{ {
thisport = atoi(ch->port); if (!parse_int_param(ch->port, &thisport, conn, "port"))
goto error_return;
if (thisport < 1 || thisport > 65535) if (thisport < 1 || thisport > 65535)
{ {
appendPQExpBuffer(&conn->errorMessage, appendPQExpBuffer(&conn->errorMessage,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment