Check return value of strdup() in libpq connection option parsing.

An out-of-memory in most of these would lead to strange behavior, like
connecting to a different database than intended, but some would lead to
an outright segfault.

Alex Shulgin and me. Backpatch to all supported versions.

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 8088bdac9833abc1b5d19fcca94af86a9fe09d28..8ac9dab2580012dcbd977c59858a4e8cd7c79deb 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -292,7 +292,7 @@ static int  connectDBStart(PGconn *conn);
 static int connectDBComplete(PGconn *conn);
 static PGPing internal_ping(PGconn *conn);
 static PGconn *makeEmptyPGconn(void);
-static void fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
+static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
 static void freePGconn(PGconn *conn);
 static void closePGconn(PGconn *conn);
 static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage);
@@ -522,7 +522,11 @@ PQconnectStartParams(const char *const * keywords,
    /*
     * Move option values into conn structure
     */
-   fillPGconn(conn, connOptions);
+   if (!fillPGconn(conn, connOptions))
+   {
+       PQconninfoFree(connOptions);
+       return conn;
+   }
 
    /*
     * Free the option info - all is in conn now
@@ -602,63 +606,55 @@ PQconnectStart(const char *conninfo)
    return conn;
 }
 
-static void
+/*
+ * Move option values into conn structure
+ *
+ * Don't put anything cute here --- intelligence should be in
+ * connectOptions2 ...
+ *
+ * Returns true on success. On failure, returns false and sets error message.
+ */
+#define FILL_CONN_OPTION(dst, name) \
+   do \
+   { \
+       tmp = conninfo_getval(connOptions, name); \
+       if (tmp) \
+       { \
+           dst = strdup(tmp); \
+           if (dst == NULL) \
+               goto oom_error; \
+       } \
+       else \
+           dst = NULL; \
+   } while(0)
+
+static bool
 fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
 {
    const char *tmp;
 
-   /*
-    * Move option values into conn structure
-    *
-    * Don't put anything cute here --- intelligence should be in
-    * connectOptions2 ...
-    *
-    * XXX: probably worth checking strdup() return value here...
-    */
-   tmp = conninfo_getval(connOptions, "hostaddr");
-   conn->pghostaddr = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "host");
-   conn->pghost = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "port");
-   conn->pgport = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "tty");
-   conn->pgtty = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "options");
-   conn->pgoptions = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "application_name");
-   conn->appname = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "fallback_application_name");
-   conn->fbappname = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "dbname");
-   conn->dbName = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "user");
-   conn->pguser = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "password");
-   conn->pgpass = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "connect_timeout");
-   conn->connect_timeout = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "client_encoding");
-   conn->client_encoding_initial = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "keepalives");
-   conn->keepalives = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "keepalives_idle");
-   conn->keepalives_idle = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "keepalives_interval");
-   conn->keepalives_interval = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "keepalives_count");
-   conn->keepalives_count = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "sslmode");
-   conn->sslmode = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "sslcompression");
-   conn->sslcompression = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "sslkey");
-   conn->sslkey = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "sslcert");
-   conn->sslcert = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "sslrootcert");
-   conn->sslrootcert = tmp ? strdup(tmp) : NULL;
-   tmp = conninfo_getval(connOptions, "sslcrl");
-   conn->sslcrl = tmp ? strdup(tmp) : NULL;
+   FILL_CONN_OPTION(conn->pghostaddr, "hostaddr");
+   FILL_CONN_OPTION(conn->pghost, "host");
+   FILL_CONN_OPTION(conn->pgport, "port");
+   FILL_CONN_OPTION(conn->pgtty, "tty");
+   FILL_CONN_OPTION(conn->pgoptions, "options");
+   FILL_CONN_OPTION(conn->appname, "application_name");
+   FILL_CONN_OPTION(conn->fbappname, "fallback_application_name");
+   FILL_CONN_OPTION(conn->dbName, "dbname");
+   FILL_CONN_OPTION(conn->pguser, "user");
+   FILL_CONN_OPTION(conn->pgpass, "password");
+   FILL_CONN_OPTION(conn->connect_timeout, "connect_timeout");
+   FILL_CONN_OPTION(conn->client_encoding_initial, "client_encoding");
+   FILL_CONN_OPTION(conn->keepalives, "keepalives");
+   FILL_CONN_OPTION(conn->keepalives_idle, "keepalives_idle");
+   FILL_CONN_OPTION(conn->keepalives_interval, "keepalives_interval");
+   FILL_CONN_OPTION(conn->keepalives_count, "keepalives_count");
+   FILL_CONN_OPTION(conn->sslmode, "sslmode");
+   FILL_CONN_OPTION(conn->sslcompression, "sslcompression");
+   FILL_CONN_OPTION(conn->sslkey, "sslkey");
+   FILL_CONN_OPTION(conn->sslcert, "sslcert");
+   FILL_CONN_OPTION(conn->sslrootcert, "sslrootcert");
+   FILL_CONN_OPTION(conn->sslcrl, "sslcrl");
 #ifdef USE_SSL
    tmp = conninfo_getval(connOptions, "requiressl");
    if (tmp && tmp[0] == '1')
@@ -667,20 +663,25 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
        if (conn->sslmode)
            free(conn->sslmode);
        conn->sslmode = strdup("require");
+       if (!conn->sslmode)
+           return false;
    }
 #endif
-   tmp = conninfo_getval(connOptions, "requirepeer");
-   conn->requirepeer = tmp ? strdup(tmp) : NULL;
+   FILL_CONN_OPTION(conn->requirepeer, "requirepeer");
 #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
-   tmp = conninfo_getval(connOptions, "krbsrvname");
-   conn->krbsrvname = tmp ? strdup(tmp) : NULL;
+   FILL_CONN_OPTION(conn->krbsrvname, "krbsrvname");
 #endif
 #if defined(ENABLE_GSS) && defined(ENABLE_SSPI)
-   tmp = conninfo_getval(connOptions, "gsslib");
-   conn->gsslib = tmp ? strdup(tmp) : NULL;
+   FILL_CONN_OPTION(conn->gsslib, "gsslib");
 #endif
-   tmp = conninfo_getval(connOptions, "replication");
-   conn->replication = tmp ? strdup(tmp) : NULL;
+   FILL_CONN_OPTION(conn->replication, "replication");
+
+   return true;
+
+oom_error:
+   printfPQExpBuffer(&conn->errorMessage,
+                     libpq_gettext("out of memory\n"));
+   return false;
 }
 
 /*
@@ -713,7 +714,12 @@ connectOptions1(PGconn *conn, const char *conninfo)
    /*
     * Move option values into conn structure
     */
-   fillPGconn(conn, connOptions);
+   if (!fillPGconn(conn, connOptions))
+   {
+       conn->status = CONNECTION_BAD;
+       PQconninfoFree(connOptions);
+       return false;
+   }
 
    /*
     * Free the option info - all is in conn now
@@ -743,6 +749,8 @@ connectOptions2(PGconn *conn)
        if (conn->dbName)
            free(conn->dbName);
        conn->dbName = strdup(conn->pguser);
+       if (!conn->dbName)
+           goto oom_error;
    }
 
    /*
@@ -755,7 +763,12 @@ connectOptions2(PGconn *conn)
        conn->pgpass = PasswordFromFile(conn->pghost, conn->pgport,
                                        conn->dbName, conn->pguser);
        if (conn->pgpass == NULL)
+       {
            conn->pgpass = strdup(DefaultPassword);
+           if (!conn->pgpass)
+               goto oom_error;
+
+       }
        else
            conn->dot_pgpass_used = true;
    }
@@ -813,7 +826,11 @@ connectOptions2(PGconn *conn)
 #endif
    }
    else
+   {
        conn->sslmode = strdup(DefaultSSLMode);
+       if (!conn->sslmode)
+           goto oom_error;
+   }
 
    /*
     * Resolve special "auto" client_encoding from the locale
@@ -823,6 +840,8 @@ connectOptions2(PGconn *conn)
    {
        free(conn->client_encoding_initial);
        conn->client_encoding_initial = strdup(pg_encoding_to_char(pg_get_encoding_from_locale(NULL, true)));
+       if (!conn->client_encoding_initial)
+           goto oom_error;
    }
 
    /*
@@ -833,6 +852,12 @@ connectOptions2(PGconn *conn)
    conn->options_valid = true;
 
    return true;
+
+oom_error:
+   conn->status = CONNECTION_BAD;
+   printfPQExpBuffer(&conn->errorMessage,
+                     libpq_gettext("out of memory\n"));
+   return false;
 }
 
 /*
@@ -926,6 +951,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
            if (conn->dbName)
                free(conn->dbName);
            conn->dbName = strdup(dbName);
+           if (!conn->dbName)
+               goto oom_error;
        }
    }
 
@@ -938,6 +965,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
        if (conn->pghost)
            free(conn->pghost);
        conn->pghost = strdup(pghost);
+       if (!conn->pghost)
+           goto oom_error;
    }
 
    if (pgport && pgport[0] != '\0')
@@ -945,6 +974,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
        if (conn->pgport)
            free(conn->pgport);
        conn->pgport = strdup(pgport);
+       if (!conn->pgport)
+           goto oom_error;
    }
 
    if (pgoptions && pgoptions[0] != '\0')
@@ -952,6 +983,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
        if (conn->pgoptions)
            free(conn->pgoptions);
        conn->pgoptions = strdup(pgoptions);
+       if (!conn->pgoptions)
+           goto oom_error;
    }
 
    if (pgtty && pgtty[0] != '\0')
@@ -959,6 +992,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
        if (conn->pgtty)
            free(conn->pgtty);
        conn->pgtty = strdup(pgtty);
+       if (!conn->pgtty)
+           goto oom_error;
    }
 
    if (login && login[0] != '\0')
@@ -966,6 +1001,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
        if (conn->pguser)
            free(conn->pguser);
        conn->pguser = strdup(login);
+       if (!conn->pguser)
+           goto oom_error;
    }
 
    if (pwd && pwd[0] != '\0')
@@ -973,6 +1010,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
        if (conn->pgpass)
            free(conn->pgpass);
        conn->pgpass = strdup(pwd);
+       if (!conn->pgpass)
+           goto oom_error;
    }
 
    /*
@@ -988,6 +1027,12 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
        (void) connectDBComplete(conn);
 
    return conn;
+
+oom_error:
+   conn->status = CONNECTION_BAD;
+   printfPQExpBuffer(&conn->errorMessage,
+                     libpq_gettext("out of memory\n"));
+   return conn;
 }
 
 
@@ -3811,7 +3856,16 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
                if (strcmp(options[i].keyword, optname) == 0)
                {
                    if (options[i].val == NULL)
+                   {
                        options[i].val = strdup(optval);
+                       if (!options[i].val)
+                       {
+                           printfPQExpBuffer(errorMessage,
+                                           libpq_gettext("out of memory\n"));
+                           free(result);
+                           return 3;
+                       }
+                   }
                    found_keyword = true;
                    break;
                }
@@ -4034,6 +4088,13 @@ parseServiceFile(const char *serviceFile,
                    {
                        if (options[i].val == NULL)
                            options[i].val = strdup(val);
+                       if (!options[i].val)
+                       {
+                           printfPQExpBuffer(errorMessage,
+                                           libpq_gettext("out of memory\n"));
+                           fclose(f);
+                           return 3;
+                       }
                        found_keyword = true;
                        break;
                    }
@@ -4439,6 +4500,14 @@ conninfo_array_parse(const char *const * keywords, const char *const * values,
                                if (options[k].val)
                                    free(options[k].val);
                                options[k].val = strdup(str_option->val);
+                               if (!options[k].val)
+                               {
+                                   printfPQExpBuffer(errorMessage,
+                                                     libpq_gettext("out of memory\n"));
+                                   PQconninfoFree(options);
+                                   PQconninfoFree(dbname_options);
+                                   return NULL;
+                               }
                                break;
                            }
                        }
@@ -4622,20 +4691,22 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
 {
    int         prefix_len;
    char       *p;
-   char       *buf = strdup(uri);      /* need a modifiable copy of the input
-                                        * URI */
-   char       *start = buf;
+   char       *buf;
+   char       *start;
    char        prevchar = '\0';
    char       *user = NULL;
    char       *host = NULL;
    bool        retval = false;
 
+   /* need a modifiable copy of the input URI */
+   buf = strdup(uri);
    if (buf == NULL)
    {
        printfPQExpBuffer(errorMessage,
                          libpq_gettext("out of memory\n"));
        return false;
    }
+   start = buf;
 
    /* Skip the URI prefix */
    prefix_len = uri_prefix_length(uri);
@@ -4977,15 +5048,17 @@ conninfo_uri_parse_params(char *params,
 static char *
 conninfo_uri_decode(const char *str, PQExpBuffer errorMessage)
 {
-   char       *buf = malloc(strlen(str) + 1);
-   char       *p = buf;
+   char       *buf;
+   char       *p;
    const char *q = str;
 
+   buf = malloc(strlen(str) + 1);
    if (buf == NULL)
    {
        printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
        return NULL;
    }
+   p = buf;
 
    for (;;)
    {
@@ -5116,7 +5189,6 @@ conninfo_storeval(PQconninfoOption *connOptions,
    else
    {
        value_copy = strdup(value);
-
        if (value_copy == NULL)
        {
            printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
@@ -5632,6 +5704,12 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
        ret = strdup(t);
        fclose(fp);
 
+       if (!ret)
+       {
+           /* Out of memory. XXX: an error message would be nice. */
+           return NULL;
+       }
+
        /* De-escape password. */
        for (p1 = p2 = ret; *p1 != ':' && *p1 != '\0'; ++p1, ++p2)
        {