From 30bf4689a96cd283af33edcdd6b7210df3f20cd8 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 25 Nov 2014 12:55:00 +0200 Subject: [PATCH] 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. --- src/interfaces/libpq/fe-connect.c | 132 +++++++++++++++++++++++++----- 1 file changed, 113 insertions(+), 19 deletions(-) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 3fe8c21639..6663c48352 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -333,7 +333,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); @@ -585,7 +585,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 @@ -665,19 +669,19 @@ 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. + */ +static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions) { const internalPQconninfoOption *option; - /* - * 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... - */ for (option = PQconninfoOptions; option->keyword; option++) { const char *tmp = conninfo_getval(connOptions, option->keyword); @@ -688,9 +692,22 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions) if (*connmember) free(*connmember); - *connmember = tmp ? strdup(tmp) : NULL; + if (tmp) + { + *connmember = strdup(tmp); + if (*connmember == NULL) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("out of memory\n")); + return false; + } + } + else + *connmember = NULL; } } + + return true; } /* @@ -723,7 +740,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 @@ -753,6 +775,8 @@ connectOptions2(PGconn *conn) if (conn->dbName) free(conn->dbName); conn->dbName = strdup(conn->pguser); + if (!conn->dbName) + goto oom_error; } /* @@ -765,7 +789,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; } @@ -823,7 +852,11 @@ connectOptions2(PGconn *conn) #endif } else + { conn->sslmode = strdup(DefaultSSLMode); + if (!conn->sslmode) + goto oom_error; + } /* * Resolve special "auto" client_encoding from the locale @@ -833,6 +866,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; } /* @@ -843,6 +878,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; } /* @@ -937,6 +978,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; } } @@ -949,6 +992,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') @@ -956,6 +1001,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') @@ -963,6 +1010,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') @@ -970,6 +1019,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') @@ -977,6 +1028,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') @@ -984,6 +1037,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; } /* @@ -999,6 +1054,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; } @@ -3760,7 +3821,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; } @@ -3983,6 +4053,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; } @@ -4402,6 +4479,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; } } @@ -4596,20 +4681,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); @@ -4951,15 +5038,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 (;;) { @@ -5104,7 +5193,6 @@ conninfo_storeval(PQconninfoOption *connOptions, else { value_copy = strdup(value); - if (value_copy == NULL) { printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n")); @@ -5672,6 +5760,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) {