From a2385cac135bb8498a12b1af2cde18244c7be2b1 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 8 Aug 2016 10:07:46 -0400 Subject: [PATCH] Obstruct shell, SQL, and conninfo injection via database and role names. Due to simplistic quoting and confusion of database names with conninfo strings, roles with the CREATEDB or CREATEROLE option could escalate to superuser privileges when a superuser next ran certain maintenance commands. The new coding rule for PQconnectdbParams() calls, documented at conninfo_array_parse(), is to pass expand_dbname=true and wrap literal database names in a trivial connection string. Escape zero-length values in appendConnStrVal(). Back-patch to 9.1 (all supported versions). Nathan Bossart, Michael Paquier, and Noah Misch. Reviewed by Peter Eisentraut. Reported by Nathan Bossart. Security: CVE-2016-5424 --- contrib/pg_upgrade/check.c | 5 +- contrib/pg_upgrade/dump.c | 5 +- contrib/pg_upgrade/option.c | 6 + contrib/pg_upgrade/pg_upgrade.c | 18 +-- contrib/pg_upgrade/pg_upgrade.h | 5 + contrib/pg_upgrade/server.c | 17 ++- contrib/pg_upgrade/util.c | 204 +++++++++++++++++++++++++++ contrib/pg_upgrade/version.c | 10 +- contrib/pg_upgrade/version_old_8_3.c | 34 +++-- src/bin/pg_dump/dumputils.c | 204 +++++++++++++++++++++++++++ src/bin/pg_dump/dumputils.h | 3 + src/bin/pg_dump/pg_backup.h | 2 +- src/bin/pg_dump/pg_backup_archiver.c | 24 +++- src/bin/pg_dump/pg_backup_db.c | 9 +- src/bin/pg_dump/pg_dumpall.c | 155 ++------------------ src/bin/psql/command.c | 15 +- src/bin/scripts/Makefile | 2 +- src/bin/scripts/clusterdb.c | 9 +- src/bin/scripts/reindexdb.c | 14 +- src/bin/scripts/vacuumdb.c | 11 +- src/interfaces/libpq/fe-connect.c | 13 +- 21 files changed, 572 insertions(+), 193 deletions(-) diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c index 82fbd230353..db15a75022d 100644 --- a/contrib/pg_upgrade/check.c +++ b/contrib/pg_upgrade/check.c @@ -218,9 +218,10 @@ issue_warnings(char *sequence_script_file_name) prep_status("Adjusting sequences"); exec_prog(true, SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on " - "--no-psqlrc --port %d --username \"%s\" " + "--no-psqlrc --port %d --username %s " "-f \"%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE, - new_cluster.bindir, new_cluster.port, os_info.user, + new_cluster.bindir, new_cluster.port, + os_info.user_shell_arg, sequence_script_file_name, log_opts.filename); unlink(sequence_script_file_name); check_ok(); diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c index 4ef3e2bd3d1..fef94083199 100644 --- a/contrib/pg_upgrade/dump.c +++ b/contrib/pg_upgrade/dump.c @@ -21,10 +21,11 @@ generate_old_dump(void) * restores the frozenid's for databases and relations. */ exec_prog(true, - SYSTEMQUOTE "\"%s/pg_dumpall\" --port %d --username \"%s\" " + SYSTEMQUOTE "\"%s/pg_dumpall\" --port %d --username %s " "--schema-only --quote-all-identifiers --binary-upgrade " "-f \"%s/" ALL_DUMP_FILE "\"" - SYSTEMQUOTE, new_cluster.bindir, old_cluster.port, os_info.user, os_info.cwd); + SYSTEMQUOTE, new_cluster.bindir, old_cluster.port, + os_info.user_shell_arg, os_info.cwd); check_ok(); } diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c index 735ee6758d1..04cef630e10 100644 --- a/contrib/pg_upgrade/option.c +++ b/contrib/pg_upgrade/option.c @@ -52,6 +52,7 @@ parseCommandLine(int argc, char *argv[]) int option; /* Command line option */ int optindex = 0; /* used by getopt_long */ int os_user_effective_id; + PQExpBufferData userbuf; user_opts.transfer_mode = TRANSFER_MODE_COPY; @@ -211,6 +212,11 @@ parseCommandLine(int argc, char *argv[]) "old cluster data resides"); validateDirectoryOption(&new_cluster.pgdata, "NEWDATADIR", "-D", "new cluster data resides"); + + initPQExpBuffer(&userbuf); + appendShellString(&userbuf, os_info.user); + /* Abandon struct, but keep its buffer until process exit. */ + os_info.user_shell_arg = userbuf.data; } diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c index ad4da56cff2..2e73a25f46d 100644 --- a/contrib/pg_upgrade/pg_upgrade.c +++ b/contrib/pg_upgrade/pg_upgrade.c @@ -359,9 +359,9 @@ prepare_new_cluster(void) */ prep_status("Analyzing all rows in the new cluster"); exec_prog(true, - SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username \"%s\" " + SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username %s " "--all --analyze >> \"%s\" 2>&1" SYSTEMQUOTE, - new_cluster.bindir, new_cluster.port, os_info.user, + new_cluster.bindir, new_cluster.port, os_info.user_shell_arg, #ifndef WIN32 log_opts.filename #else @@ -378,9 +378,9 @@ prepare_new_cluster(void) */ prep_status("Freezing all rows on the new cluster"); exec_prog(true, - SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username \"%s\" " + SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username %s " "--all --freeze >> \"%s\" 2>&1" SYSTEMQUOTE, - new_cluster.bindir, new_cluster.port, os_info.user, + new_cluster.bindir, new_cluster.port, os_info.user_shell_arg, #ifndef WIN32 log_opts.filename #else @@ -421,9 +421,10 @@ prepare_new_databases(void) exec_prog(true, SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on " /* --no-psqlrc prevents AUTOCOMMIT=off */ - "--no-psqlrc --port %d --username \"%s\" " + "--no-psqlrc --port %d --username %s " "-f \"%s/%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE, - new_cluster.bindir, new_cluster.port, os_info.user, os_info.cwd, + new_cluster.bindir, new_cluster.port, + os_info.user_shell_arg, os_info.cwd, GLOBALS_DUMP_FILE, #ifndef WIN32 log_opts.filename @@ -458,9 +459,10 @@ create_new_objects(void) prep_status("Restoring database schema to new cluster"); exec_prog(true, SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on " - "--no-psqlrc --port %d --username \"%s\" " + "--no-psqlrc --port %d --username %s " "-f \"%s/%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE, - new_cluster.bindir, new_cluster.port, os_info.user, os_info.cwd, + new_cluster.bindir, new_cluster.port, + os_info.user_shell_arg, os_info.cwd, DB_DUMP_FILE, #ifndef WIN32 log_opts.filename diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h index 72e2e6acafc..d0de1628ec8 100644 --- a/contrib/pg_upgrade/pg_upgrade.h +++ b/contrib/pg_upgrade/pg_upgrade.h @@ -14,6 +14,7 @@ #include #include "libpq-fe.h" +#include "pqexpbuffer.h" /* Allocate for null byte */ #define USER_NAME_SIZE 128 @@ -236,6 +237,7 @@ typedef struct const char *progname; /* complete pathname for this program */ char *exec_path; /* full path to my executable */ char *user; /* username for clusters */ + char *user_shell_arg; /* the same, with shell quoting */ char cwd[MAXPGPATH]; /* current working directory, used for output */ char **tablespaces; /* tablespaces */ int num_tablespaces; @@ -381,6 +383,9 @@ void check_pghost_envvar(void); /* util.c */ char *quote_identifier(const char *s); +extern void appendShellString(PQExpBuffer buf, const char *str); +extern void appendConnStrVal(PQExpBuffer buf, const char *str); +extern void appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname); int get_user_info(char **user_name); void check_ok(void); void report_status(eLogType type, const char *fmt,...); diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c index a4c2b6d50d1..cb4644bbcdf 100644 --- a/contrib/pg_upgrade/server.c +++ b/contrib/pg_upgrade/server.c @@ -49,13 +49,20 @@ connectToServer(ClusterInfo *cluster, const char *db_name) static PGconn * get_db_conn(ClusterInfo *cluster, const char *db_name) { - char conn_opts[MAXPGPATH]; + PQExpBufferData conn_opts; + PGconn *conn; - snprintf(conn_opts, sizeof(conn_opts), - "dbname = '%s' user = '%s' port = %d", db_name, os_info.user, - cluster->port); + /* Build connection string with proper quoting */ + initPQExpBuffer(&conn_opts); + appendPQExpBufferStr(&conn_opts, "dbname="); + appendConnStrVal(&conn_opts, db_name); + appendPQExpBufferStr(&conn_opts, " user="); + appendConnStrVal(&conn_opts, os_info.user); + appendPQExpBuffer(&conn_opts, " port=%d", cluster->port); - return PQconnectdb(conn_opts); + conn = PQconnectdb(conn_opts.data); + termPQExpBuffer(&conn_opts); + return conn; } diff --git a/contrib/pg_upgrade/util.c b/contrib/pg_upgrade/util.c index c32cfcae645..264357c1d77 100644 --- a/contrib/pg_upgrade/util.c +++ b/contrib/pg_upgrade/util.c @@ -151,6 +151,210 @@ quote_identifier(const char *s) } +/* + * Append the given string to the shell command being built in the buffer, + * with suitable shell-style quoting to create exactly one argument. + * + * Forbid LF or CR characters, which have scant practical use beyond designing + * security breaches. The Windows command shell is unusable as a conduit for + * arguments containing LF or CR characters. A future major release should + * reject those characters in CREATE ROLE and CREATE DATABASE, because use + * there eventually leads to errors here. + */ +void +appendShellString(PQExpBuffer buf, const char *str) +{ + const char *p; + +#ifndef WIN32 + appendPQExpBufferChar(buf, '\''); + for (p = str; *p; p++) + { + if (*p == '\n' || *p == '\r') + { + fprintf(stderr, + _("shell command argument contains a newline or carriage return: \"%s\"\n"), + str); + exit(EXIT_FAILURE); + } + + if (*p == '\'') + appendPQExpBufferStr(buf, "'\"'\"'"); + else + appendPQExpBufferChar(buf, *p); + } + appendPQExpBufferChar(buf, '\''); +#else /* WIN32 */ + int backslash_run_length = 0; + + /* + * A Windows system() argument experiences two layers of interpretation. + * First, cmd.exe interprets the string. Its behavior is undocumented, + * but a caret escapes any byte except LF or CR that would otherwise have + * special meaning. Handling of a caret before LF or CR differs between + * "cmd.exe /c" and other modes, and it is unusable here. + * + * Second, the new process parses its command line to construct argv (see + * https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats + * backslash-double quote sequences specially. + */ + appendPQExpBufferStr(buf, "^\""); + for (p = str; *p; p++) + { + if (*p == '\n' || *p == '\r') + { + fprintf(stderr, + _("shell command argument contains a newline or carriage return: \"%s\"\n"), + str); + exit(EXIT_FAILURE); + } + + /* Change N backslashes before a double quote to 2N+1 backslashes. */ + if (*p == '"') + { + while (backslash_run_length) + { + appendPQExpBufferStr(buf, "^\\"); + backslash_run_length--; + } + appendPQExpBufferStr(buf, "^\\"); + } + else if (*p == '\\') + backslash_run_length++; + else + backslash_run_length = 0; + + /* + * Decline to caret-escape the most mundane characters, to ease + * debugging and lest we approach the command length limit. + */ + if (!((*p >= 'a' && *p <= 'z') || + (*p >= 'A' && *p <= 'Z') || + (*p >= '0' && *p <= '9'))) + appendPQExpBufferChar(buf, '^'); + appendPQExpBufferChar(buf, *p); + } + + /* + * Change N backslashes at end of argument to 2N backslashes, because they + * precede the double quote that terminates the argument. + */ + while (backslash_run_length) + { + appendPQExpBufferStr(buf, "^\\"); + backslash_run_length--; + } + appendPQExpBufferStr(buf, "^\""); +#endif /* WIN32 */ +} + + +/* + * Append the given string to the buffer, with suitable quoting for passing + * the string as a value, in a keyword/pair value in a libpq connection + * string + */ +void +appendConnStrVal(PQExpBuffer buf, const char *str) +{ + const char *s; + bool needquotes; + + /* + * If the string is one or more plain ASCII characters, no need to quote + * it. This is quite conservative, but better safe than sorry. + */ + needquotes = true; + for (s = str; *s; s++) + { + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || + (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) + { + needquotes = true; + break; + } + needquotes = false; + } + + if (needquotes) + { + appendPQExpBufferChar(buf, '\''); + while (*str) + { + /* ' and \ must be escaped by to \' and \\ */ + if (*str == '\'' || *str == '\\') + appendPQExpBufferChar(buf, '\\'); + + appendPQExpBufferChar(buf, *str); + str++; + } + appendPQExpBufferChar(buf, '\''); + } + else + appendPQExpBufferStr(buf, str); +} + + +/* + * Append a psql meta-command that connects to the given database with the + * then-current connection's user, host and port. + */ +void +appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname) +{ + const char *s; + bool complex; + + /* + * If the name is plain ASCII characters, emit a trivial "\connect "foo"". + * For other names, even many not technically requiring it, skip to the + * general case. No database has a zero-length name. + */ + complex = false; + for (s = dbname; *s; s++) + { + if (*s == '\n' || *s == '\r') + { + fprintf(stderr, + _("database name contains a newline or carriage return: \"%s\"\n"), + dbname); + exit(EXIT_FAILURE); + } + + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || + (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) + { + complex = true; + } + } + + appendPQExpBufferStr(buf, "\\connect "); + if (complex) + { + PQExpBufferData connstr; + + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, dbname); + + appendPQExpBuffer(buf, "-reuse-previous=on "); + + /* + * As long as the name does not contain a newline, SQL identifier + * quoting satisfies the psql meta-command parser. Prefer not to + * involve psql-interpreted single quotes, which behaved differently + * before PostgreSQL 9.2. + */ + appendPQExpBufferStr(buf, quote_identifier(connstr.data)); + + termPQExpBuffer(&connstr); + } + else + appendPQExpBufferStr(buf, quote_identifier(dbname)); + appendPQExpBufferChar(buf, '\n'); +} + + /* * get_user_info() * (copied from initdb.c) find the current user diff --git a/contrib/pg_upgrade/version.c b/contrib/pg_upgrade/version.c index 958bcbb80fe..0392d893e88 100644 --- a/contrib/pg_upgrade/version.c +++ b/contrib/pg_upgrade/version.c @@ -48,10 +48,16 @@ new_9_0_populate_pg_largeobject_metadata(ClusterInfo *cluster, bool check_mode) found = true; if (!check_mode) { + PQExpBufferData connectbuf; + if (script == NULL && (script = fopen(output_path, "w")) == NULL) pg_log(PG_FATAL, "could not create necessary file: %s\n", output_path); - fprintf(script, "\\connect %s\n", - quote_identifier(active_db->db_name)); + + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, active_db->db_name); + fputs(connectbuf.data, script); + termPQExpBuffer(&connectbuf); + fprintf(script, "SELECT pg_catalog.lo_create(t.loid)\n" "FROM (SELECT DISTINCT loid FROM pg_catalog.pg_largeobject) AS t;\n"); diff --git a/contrib/pg_upgrade/version_old_8_3.c b/contrib/pg_upgrade/version_old_8_3.c index ffb79adc91a..d3c79f4c05b 100644 --- a/contrib/pg_upgrade/version_old_8_3.c +++ b/contrib/pg_upgrade/version_old_8_3.c @@ -368,8 +368,13 @@ old_8_3_rebuild_tsvector_tables(ClusterInfo *cluster, bool check_mode) pg_log(PG_FATAL, "could not create necessary file: %s\n", output_path); if (!db_used) { - fprintf(script, "\\connect %s\n\n", - quote_identifier(active_db->db_name)); + PQExpBufferData connectbuf; + + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, active_db->db_name); + appendPQExpBufferChar(&connectbuf, '\n'); + fputs(connectbuf.data, script); + termPQExpBuffer(&connectbuf); db_used = true; } @@ -488,8 +493,12 @@ old_8_3_invalidate_hash_gin_indexes(ClusterInfo *cluster, bool check_mode) pg_log(PG_FATAL, "could not create necessary file: %s\n", output_path); if (!db_used) { - fprintf(script, "\\connect %s\n", - quote_identifier(active_db->db_name)); + PQExpBufferData connectbuf; + + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, active_db->db_name); + fputs(connectbuf.data, script); + termPQExpBuffer(&connectbuf); db_used = true; } fprintf(script, "REINDEX INDEX %s.%s;\n", @@ -613,8 +622,12 @@ old_8_3_invalidate_bpchar_pattern_ops_indexes(ClusterInfo *cluster, pg_log(PG_FATAL, "could not create necessary file: %s\n", output_path); if (!db_used) { - fprintf(script, "\\connect %s\n", - quote_identifier(active_db->db_name)); + PQExpBufferData connectbuf; + + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, active_db->db_name); + fputs(connectbuf.data, script); + termPQExpBuffer(&connectbuf); db_used = true; } fprintf(script, "REINDEX INDEX %s.%s;\n", @@ -740,8 +753,13 @@ old_8_3_create_sequence_script(ClusterInfo *cluster) pg_log(PG_FATAL, "could not create necessary file: %s\n", output_path); if (!db_used) { - fprintf(script, "\\connect %s\n\n", - quote_identifier(active_db->db_name)); + PQExpBufferData connectbuf; + + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, active_db->db_name); + appendPQExpBufferChar(&connectbuf, '\n'); + fputs(connectbuf.data, script); + termPQExpBuffer(&connectbuf); db_used = true; } diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index 734066ccfa3..df5a0fb4758 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -330,6 +330,210 @@ appendStringLiteralDQ(PQExpBuffer buf, const char *str, const char *dqprefix) } +/* + * Append the given string to the shell command being built in the buffer, + * with suitable shell-style quoting to create exactly one argument. + * + * Forbid LF or CR characters, which have scant practical use beyond designing + * security breaches. The Windows command shell is unusable as a conduit for + * arguments containing LF or CR characters. A future major release should + * reject those characters in CREATE ROLE and CREATE DATABASE, because use + * there eventually leads to errors here. + */ +void +appendShellString(PQExpBuffer buf, const char *str) +{ + const char *p; + +#ifndef WIN32 + appendPQExpBufferChar(buf, '\''); + for (p = str; *p; p++) + { + if (*p == '\n' || *p == '\r') + { + fprintf(stderr, + _("shell command argument contains a newline or carriage return: \"%s\"\n"), + str); + exit(EXIT_FAILURE); + } + + if (*p == '\'') + appendPQExpBufferStr(buf, "'\"'\"'"); + else + appendPQExpBufferChar(buf, *p); + } + appendPQExpBufferChar(buf, '\''); +#else /* WIN32 */ + int backslash_run_length = 0; + + /* + * A Windows system() argument experiences two layers of interpretation. + * First, cmd.exe interprets the string. Its behavior is undocumented, + * but a caret escapes any byte except LF or CR that would otherwise have + * special meaning. Handling of a caret before LF or CR differs between + * "cmd.exe /c" and other modes, and it is unusable here. + * + * Second, the new process parses its command line to construct argv (see + * https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats + * backslash-double quote sequences specially. + */ + appendPQExpBufferStr(buf, "^\""); + for (p = str; *p; p++) + { + if (*p == '\n' || *p == '\r') + { + fprintf(stderr, + _("shell command argument contains a newline or carriage return: \"%s\"\n"), + str); + exit(EXIT_FAILURE); + } + + /* Change N backslashes before a double quote to 2N+1 backslashes. */ + if (*p == '"') + { + while (backslash_run_length) + { + appendPQExpBufferStr(buf, "^\\"); + backslash_run_length--; + } + appendPQExpBufferStr(buf, "^\\"); + } + else if (*p == '\\') + backslash_run_length++; + else + backslash_run_length = 0; + + /* + * Decline to caret-escape the most mundane characters, to ease + * debugging and lest we approach the command length limit. + */ + if (!((*p >= 'a' && *p <= 'z') || + (*p >= 'A' && *p <= 'Z') || + (*p >= '0' && *p <= '9'))) + appendPQExpBufferChar(buf, '^'); + appendPQExpBufferChar(buf, *p); + } + + /* + * Change N backslashes at end of argument to 2N backslashes, because they + * precede the double quote that terminates the argument. + */ + while (backslash_run_length) + { + appendPQExpBufferStr(buf, "^\\"); + backslash_run_length--; + } + appendPQExpBufferStr(buf, "^\""); +#endif /* WIN32 */ +} + + +/* + * Append the given string to the buffer, with suitable quoting for passing + * the string as a value, in a keyword/pair value in a libpq connection + * string + */ +void +appendConnStrVal(PQExpBuffer buf, const char *str) +{ + const char *s; + bool needquotes; + + /* + * If the string is one or more plain ASCII characters, no need to quote + * it. This is quite conservative, but better safe than sorry. + */ + needquotes = true; + for (s = str; *s; s++) + { + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || + (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) + { + needquotes = true; + break; + } + needquotes = false; + } + + if (needquotes) + { + appendPQExpBufferChar(buf, '\''); + while (*str) + { + /* ' and \ must be escaped by to \' and \\ */ + if (*str == '\'' || *str == '\\') + appendPQExpBufferChar(buf, '\\'); + + appendPQExpBufferChar(buf, *str); + str++; + } + appendPQExpBufferChar(buf, '\''); + } + else + appendPQExpBufferStr(buf, str); +} + + +/* + * Append a psql meta-command that connects to the given database with the + * then-current connection's user, host and port. + */ +void +appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname) +{ + const char *s; + bool complex; + + /* + * If the name is plain ASCII characters, emit a trivial "\connect "foo"". + * For other names, even many not technically requiring it, skip to the + * general case. No database has a zero-length name. + */ + complex = false; + for (s = dbname; *s; s++) + { + if (*s == '\n' || *s == '\r') + { + fprintf(stderr, + _("database name contains a newline or carriage return: \"%s\"\n"), + dbname); + exit(EXIT_FAILURE); + } + + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || + (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) + { + complex = true; + } + } + + appendPQExpBufferStr(buf, "\\connect "); + if (complex) + { + PQExpBufferData connstr; + + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, dbname); + + appendPQExpBuffer(buf, "-reuse-previous=on "); + + /* + * As long as the name does not contain a newline, SQL identifier + * quoting satisfies the psql meta-command parser. Prefer not to + * involve psql-interpreted single quotes, which behaved differently + * before PostgreSQL 9.2. + */ + appendPQExpBufferStr(buf, fmtId(connstr.data)); + + termPQExpBuffer(&connstr); + } + else + appendPQExpBufferStr(buf, fmtId(dbname)); + appendPQExpBufferChar(buf, '\n'); +} + + /* * Convert a bytea value (presented as raw bytes) to an SQL string literal * and append it to the given buffer. We assume the specified diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h index 44d90669b61..7b2706c8614 100644 --- a/src/bin/pg_dump/dumputils.h +++ b/src/bin/pg_dump/dumputils.h @@ -33,6 +33,9 @@ extern void appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length, bool std_strings); extern int parse_version(const char *versionString); +extern void appendShellString(PQExpBuffer buf, const char *str); +extern void appendConnStrVal(PQExpBuffer buf, const char *str); +extern void appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname); extern bool parsePGArray(const char *atext, char ***itemarray, int *nitems); extern bool buildACLCommands(const char *name, const char *subname, const char *type, const char *acls, const char *owner, diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index ce12a41ce3e..3b454e211b7 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -130,7 +130,7 @@ typedef struct _restoreOptions char *triggerNames; int useDB; - char *dbname; + char *dbname; /* subject to expand_dbname */ char *pgport; char *pghost; char *username; diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 4bb28d528c7..3be3ec8e332 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -544,9 +544,16 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, /* If we created a DB, connect to it... */ if (strcmp(te->desc, "DATABASE") == 0) { + PQExpBufferData connstr; + + initPQExpBuffer(&connstr); + appendPQExpBufferStr(&connstr, "dbname="); + appendConnStrVal(&connstr, te->tag); + /* Abandon struct, but keep its buffer until process exit. */ + ahlog(AH, 1, "connecting to new database \"%s\"\n", te->tag); _reconnectToDB(AH, te->tag); - ropt->dbname = strdup(te->tag); + ropt->dbname = connstr.data; } } @@ -2625,12 +2632,17 @@ _reconnectToDB(ArchiveHandle *AH, const char *dbname) ReconnectToServer(AH, dbname, NULL); else { - PQExpBuffer qry = createPQExpBuffer(); + if (dbname) + { + PQExpBufferData connectbuf; - appendPQExpBuffer(qry, "\\connect %s\n\n", - dbname ? fmtId(dbname) : "-"); - ahprintf(AH, "%s", qry->data); - destroyPQExpBuffer(qry); + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, dbname); + ahprintf(AH, "%s\n", connectbuf.data); + termPQExpBuffer(&connectbuf); + } + else + ahprintf(AH, "%s\n", "\\connect -\n"); } /* diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index 5d4d0d40e76..fb5796ef186 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -121,6 +121,7 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username) static PGconn * _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) { + PQExpBufferData connstr; PGconn *newConn; const char *newdb; const char *newuser; @@ -147,6 +148,10 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) die_horribly(AH, modulename, "out of memory\n"); } + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, newdb); + do { #define PARAMS_ARRAY_SIZE 7 @@ -165,7 +170,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) keywords[3] = "password"; values[3] = password; keywords[4] = "dbname"; - values[4] = newdb; + values[4] = connstr.data; keywords[5] = "fallback_application_name"; values[5] = progname; keywords[6] = NULL; @@ -209,6 +214,8 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) AH->savedPassword = password; + termPQExpBuffer(&connstr); + /* check for version mismatch */ _check_database_version(AH); diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index fa186c6c3a1..ad6b43c5541 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -49,8 +49,6 @@ static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem, const char *name2); static void dumpDatabases(PGconn *conn); static void dumpTimestamp(char *msg); -static void appendShellString(PQExpBuffer buf, const char *str); -static void appendConnStrVal(PQExpBuffer buf, const char *str); static int runPgDump(const char *dbname); static PGconn *connectDatabase(const char *dbname, const char *pghost, const char *pgport, @@ -1345,8 +1343,9 @@ dumpCreateDB(PGconn *conn) fdbname, fmtId(dbtablespace)); /* connect to original database */ - appendPQExpBuffer(buf, "%s\\connect %s\n", - binary_upgrade ? " " : "", fdbname); + if (binary_upgrade) + appendPQExpBufferChar(buf, ' '); + appendPsqlMetaConnect(buf, dbname); } if (binary_upgrade) @@ -1569,11 +1568,15 @@ dumpDatabases(PGconn *conn) int ret; char *dbname = PQgetvalue(res, i, 0); + PQExpBufferData connectbuf; if (verbose) fprintf(stderr, _("%s: dumping database \"%s\"...\n"), progname, dbname); - fprintf(OPF, "\\connect %s\n\n", fmtId(dbname)); + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, dbname); + fprintf(OPF, "%s\n", connectbuf.data); + termPQExpBuffer(&connectbuf); /* * Restore will need to write to the target cluster. This connection @@ -1880,145 +1883,3 @@ dumpTimestamp(char *msg) localtime(&now)) != 0) fprintf(OPF, "-- %s %s\n\n", msg, buf); } - - -/* - * Append the given string to the buffer, with suitable quoting for passing - * the string as a value, in a keyword/pair value in a libpq connection - * string - */ -static void -appendConnStrVal(PQExpBuffer buf, const char *str) -{ - const char *s; - bool needquotes; - - /* - * If the string consists entirely of plain ASCII characters, no need to - * quote it. This is quite conservative, but better safe than sorry. - */ - needquotes = false; - for (s = str; *s; s++) - { - if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || - (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) - { - needquotes = true; - break; - } - } - - if (needquotes) - { - appendPQExpBufferChar(buf, '\''); - while (*str) - { - /* ' and \ must be escaped by to \' and \\ */ - if (*str == '\'' || *str == '\\') - appendPQExpBufferChar(buf, '\\'); - - appendPQExpBufferChar(buf, *str); - str++; - } - appendPQExpBufferChar(buf, '\''); - } - else - appendPQExpBufferStr(buf, str); -} - -/* - * Append the given string to the shell command being built in the buffer, - * with suitable shell-style quoting to create exactly one argument. - * - * Forbid LF or CR characters, which have scant practical use beyond designing - * security breaches. The Windows command shell is unusable as a conduit for - * arguments containing LF or CR characters. A future major release should - * reject those characters in CREATE ROLE and CREATE DATABASE, because use - * there eventually leads to errors here. - */ -static void -appendShellString(PQExpBuffer buf, const char *str) -{ - const char *p; - -#ifndef WIN32 - appendPQExpBufferChar(buf, '\''); - for (p = str; *p; p++) - { - if (*p == '\n' || *p == '\r') - { - fprintf(stderr, - _("shell command argument contains a newline or carriage return: \"%s\"\n"), - str); - exit(EXIT_FAILURE); - } - - if (*p == '\'') - appendPQExpBuffer(buf, "'\"'\"'"); - else - appendPQExpBufferChar(buf, *p); - } - appendPQExpBufferChar(buf, '\''); -#else /* WIN32 */ - int backslash_run_length = 0; - - /* - * A Windows system() argument experiences two layers of interpretation. - * First, cmd.exe interprets the string. Its behavior is undocumented, - * but a caret escapes any byte except LF or CR that would otherwise have - * special meaning. Handling of a caret before LF or CR differs between - * "cmd.exe /c" and other modes, and it is unusable here. - * - * Second, the new process parses its command line to construct argv (see - * https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats - * backslash-double quote sequences specially. - */ - appendPQExpBufferStr(buf, "^\""); - for (p = str; *p; p++) - { - if (*p == '\n' || *p == '\r') - { - fprintf(stderr, - _("shell command argument contains a newline or carriage return: \"%s\"\n"), - str); - exit(EXIT_FAILURE); - } - - /* Change N backslashes before a double quote to 2N+1 backslashes. */ - if (*p == '"') - { - while (backslash_run_length) - { - appendPQExpBufferStr(buf, "^\\"); - backslash_run_length--; - } - appendPQExpBufferStr(buf, "^\\"); - } - else if (*p == '\\') - backslash_run_length++; - else - backslash_run_length = 0; - - /* - * Decline to caret-escape the most mundane characters, to ease - * debugging and lest we approach the command length limit. - */ - if (!((*p >= 'a' && *p <= 'z') || - (*p >= 'A' && *p <= 'Z') || - (*p >= '0' && *p <= '9'))) - appendPQExpBufferChar(buf, '^'); - appendPQExpBufferChar(buf, *p); - } - - /* - * Change N backslashes at end of argument to 2N backslashes, because they - * precede the double quote that terminates the argument. - */ - while (backslash_run_length) - { - appendPQExpBufferStr(buf, "^\\"); - backslash_run_length--; - } - appendPQExpBufferStr(buf, "^\""); -#endif /* WIN32 */ -} diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 761f4cee150..cf1950b6ec8 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1485,6 +1485,7 @@ do_connect(enum trivalue reuse_previous_specification, bool keep_password; bool has_connection_string; bool reuse_previous; + PQExpBufferData connstr; has_connection_string = dbname ? recognized_connection_string(dbname) : false; @@ -1536,7 +1537,15 @@ do_connect(enum trivalue reuse_previous_specification, * changes: passwords aren't (usually) database-specific. */ if (!dbname && reuse_previous) - dbname = PQdb(o_conn); + { + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, PQdb(o_conn)); + dbname = connstr.data; + /* has_connection_string=true would be a dead store */ + } + else + connstr.data = NULL; /* * If the user asked to be prompted for a password, ask for one now. If @@ -1641,8 +1650,12 @@ do_connect(enum trivalue reuse_previous_specification, } PQfinish(n_conn); + if (connstr.data) + termPQExpBuffer(&connstr); return false; } + if (connstr.data) + termPQExpBuffer(&connstr); /* * Replace the old connection with the new one, and update diff --git a/src/bin/scripts/Makefile b/src/bin/scripts/Makefile index 0c059665274..0fd21200eb5 100644 --- a/src/bin/scripts/Makefile +++ b/src/bin/scripts/Makefile @@ -32,7 +32,7 @@ dropdb: dropdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq droplang: droplang.o common.o print.o mbprint.o | submake-libpq dropuser: dropuser.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq clusterdb: clusterdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq -vacuumdb: vacuumdb.o common.o | submake-libpq +vacuumdb: vacuumdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq reindexdb: reindexdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq dumputils.c keywords.c: % : $(top_srcdir)/src/bin/pg_dump/% diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c index 8ed839490c8..7088a6cae93 100644 --- a/src/bin/scripts/clusterdb.c +++ b/src/bin/scripts/clusterdb.c @@ -204,12 +204,14 @@ cluster_all_databases(bool verbose, const char *host, const char *port, { PGconn *conn; PGresult *result; + PQExpBufferData connstr; int i; conn = connectDatabase("postgres", host, port, username, prompt_password, progname); result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo); PQfinish(conn); + initPQExpBuffer(&connstr); for (i = 0; i < PQntuples(result); i++) { char *dbname = PQgetvalue(result, i, 0); @@ -220,10 +222,15 @@ cluster_all_databases(bool verbose, const char *host, const char *port, fflush(stdout); } - cluster_one_database(dbname, verbose, NULL, + resetPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, dbname); + + cluster_one_database(connstr.data, verbose, NULL, host, port, username, prompt_password, progname, echo); } + termPQExpBuffer(&connstr); PQclear(result); } diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index 6d7039f8575..4bcec082b99 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -258,12 +258,14 @@ reindex_all_databases(const char *host, const char *port, { PGconn *conn; PGresult *result; + PQExpBufferData connstr; int i; conn = connectDatabase("postgres", host, port, username, prompt_password, progname); result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo); PQfinish(conn); + initPQExpBuffer(&connstr); for (i = 0; i < PQntuples(result); i++) { char *dbname = PQgetvalue(result, i, 0); @@ -274,9 +276,15 @@ reindex_all_databases(const char *host, const char *port, fflush(stdout); } - reindex_one_database(dbname, dbname, "DATABASE", host, port, username, - prompt_password, progname, echo); + resetPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, dbname); + + reindex_one_database(NULL, connstr.data, "DATABASE", host, + port, username, prompt_password, + progname, echo); } + termPQExpBuffer(&connstr); PQclear(result); } @@ -293,7 +301,7 @@ reindex_system_catalogs(const char *dbname, const char *host, const char *port, initPQExpBuffer(&sql); - appendPQExpBuffer(&sql, "REINDEX SYSTEM %s;\n", PQdb(conn)); + appendPQExpBuffer(&sql, "REINDEX SYSTEM %s;\n", fmtId(PQdb(conn))); if (!executeMaintenanceCommand(conn, sql.data, echo)) { diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c index 84a9dcd38e5..5dbc01ec0ac 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -12,6 +12,7 @@ #include "postgres_fe.h" #include "common.h" +#include "dumputils.h" static void vacuum_one_database(const char *dbname, bool full, bool verbose, @@ -296,12 +297,14 @@ vacuum_all_databases(bool full, bool verbose, bool and_analyze, bool analyze_onl { PGconn *conn; PGresult *result; + PQExpBufferData connstr; int i; conn = connectDatabase("postgres", host, port, username, prompt_password, progname); result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo); PQfinish(conn); + initPQExpBuffer(&connstr); for (i = 0; i < PQntuples(result); i++) { char *dbname = PQgetvalue(result, i, 0); @@ -312,10 +315,16 @@ vacuum_all_databases(bool full, bool verbose, bool and_analyze, bool analyze_onl fflush(stdout); } - vacuum_one_database(dbname, full, verbose, and_analyze, analyze_only, + resetPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, PQgetvalue(result, i, 0)); + + vacuum_one_database(connstr.data, full, verbose, and_analyze, + analyze_only, freeze, NULL, host, port, username, prompt_password, progname, echo); } + termPQExpBuffer(&connstr); PQclear(result); } diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 43911a2dea3..bac693273fb 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -4377,10 +4377,15 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage, * Defaults are supplied (from a service file, environment variables, etc) * for unspecified options, but only if use_defaults is TRUE. * - * If expand_dbname is non-zero, and the value passed for keyword "dbname" - * contains an "=", assume it is a conninfo string and process it, - * overriding any previously processed conflicting keywords. Subsequent - * keywords will take precedence, however. + * If expand_dbname is non-zero, and the value passed for the first occurrence + * of "dbname" keyword contains an "=", assume it is a conninfo string and + * process it, overriding any previously processed conflicting + * keywords. Subsequent keywords will take precedence, however. In-tree + * programs generally specify expand_dbname=true, so command-line arguments + * naming a database can use a connection string. Some code acquires + * arbitrary database names from known-literal sources like PQdb(), + * PQconninfoParse() and pg_database.datname. When connecting to such a + * database, in-tree code first wraps the name in a connection string. */ static PQconninfoOption * conninfo_array_parse(const char **keywords, const char **values,