Skip to content

Commit

Permalink
Parse more strictly integer parameters from connection strings in libpq
Browse files Browse the repository at this point in the history
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
  • Loading branch information
michaelpq committed Sep 11, 2018
1 parent 0d45cd9 commit e7a2217
Showing 1 changed file with 54 additions and 9 deletions.
63 changes: 54 additions & 9 deletions src/interfaces/libpq/fe-connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -1587,6 +1587,34 @@ useKeepalives(PGconn *conn)
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
/*
* Set the keepalive idle timer.
Expand All @@ -1599,7 +1627,9 @@ setKeepalivesIdle(PGconn *conn)
if (conn->keepalives_idle == NULL)
return 1;

idle = atoi(conn->keepalives_idle);
if (!parse_int_param(conn->keepalives_idle, &idle, conn,
"keepalives_idle"))
return 0;
if (idle < 0)
idle = 0;

Expand Down Expand Up @@ -1631,7 +1661,9 @@ setKeepalivesInterval(PGconn *conn)
if (conn->keepalives_interval == NULL)
return 1;

interval = atoi(conn->keepalives_interval);
if (!parse_int_param(conn->keepalives_interval, &interval, conn,
"keepalives_interval"))
return 0;
if (interval < 0)
interval = 0;

Expand Down Expand Up @@ -1664,7 +1696,9 @@ setKeepalivesCount(PGconn *conn)
if (conn->keepalives_count == NULL)
return 1;

count = atoi(conn->keepalives_count);
if (!parse_int_param(conn->keepalives_count, &count, conn,
"keepalives_count"))
return 0;
if (count < 0)
count = 0;

Expand Down Expand Up @@ -1698,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 &&
!parse_int_param(conn->keepalives_idle, &idle, conn,
"keepalives_idle"))
return 0;
if (idle <= 0)
idle = 2 * 60 * 60; /* 2 hours = default */

if (conn->keepalives_interval)
interval = atoi(conn->keepalives_interval);
if (conn->keepalives_interval &&
!parse_int_param(conn->keepalives_interval, &interval, conn,
"keepalives_interval"))
return 0;
if (interval <= 0)
interval = 1; /* 1 second = default */

Expand Down Expand Up @@ -1831,7 +1869,10 @@ connectDBComplete(PGconn *conn)
*/
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)
{
/*
Expand All @@ -1842,6 +1883,8 @@ connectDBComplete(PGconn *conn)
if (timeout < 2)
timeout = 2;
}
else /* negative means 0 */
timeout = 0;
}

for (;;)
Expand Down Expand Up @@ -2108,7 +2151,9 @@ PQconnectPoll(PGconn *conn)
thisport = DEF_PGPORT;
else
{
thisport = atoi(ch->port);
if (!parse_int_param(ch->port, &thisport, conn, "port"))
goto error_return;

if (thisport < 1 || thisport > 65535)
{
appendPQExpBuffer(&conn->errorMessage,
Expand Down

0 comments on commit e7a2217

Please sign in to comment.