database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-28 23:42:10 +03:00
Code Activity

Obstruct shell, SQL, and conninfo injection via database and role names.

Browse Source 
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
This commit is contained in:
Noah Misch
2016-08-08 10:07:46 -04:00
parent c1b048f498
commit 95a6855c55
21 changed files with 665 additions and 202 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
contrib/pg_upgrade/dump.c
View File

@ -46,6 +46,15 @@ generate_old_dump(void)
char sql_file_name[MAXPGPATH],
log_file_name[MAXPGPATH];
DbInfo *old_db = &old_cluster.dbarr.dbs[dbnum];
PQExpBufferData connstr,
escaped_connstr;
initPQExpBuffer(&connstr);
appendPQExpBuffer(&connstr, "dbname=");
appendConnStrVal(&connstr, old_db->db_name);
initPQExpBuffer(&escaped_connstr);
appendShellString(&escaped_connstr, connstr.data);
termPQExpBuffer(&connstr);
pg_log(PG_STATUS, "%s", old_db->db_name);
snprintf(sql_file_name, sizeof(sql_file_name), DB_DUMP_FILE_MASK, old_db->db_oid);
@ -53,10 +62,12 @@ generate_old_dump(void)
parallel_exec_prog(log_file_name, NULL,
"\"%s/pg_dump\" %s --schema-only --quote-all-identifiers "
"--binary-upgrade --format=custom %s --file=\"%s\" \"%s\"",
"--binary-upgrade --format=custom %s --file=\"%s\" %s",
new_cluster.bindir, cluster_conn_opts(&old_cluster),
log_opts.verbose ? "--verbose" : "",
sql_file_name, old_db->db_name);
sql_file_name, escaped_connstr.data);
termPQExpBuffer(&escaped_connstr);
}
/* reap all children */

15
contrib/pg_upgrade/pg_upgrade.c
View File

@ -498,6 +498,15 @@ create_new_objects(void)
char sql_file_name[MAXPGPATH],
log_file_name[MAXPGPATH];
DbInfo *old_db = &old_cluster.dbarr.dbs[dbnum];
PQExpBufferData connstr,
escaped_connstr;
initPQExpBuffer(&connstr);
appendPQExpBuffer(&connstr, "dbname=");
appendConnStrVal(&connstr, old_db->db_name);
initPQExpBuffer(&escaped_connstr);
appendShellString(&escaped_connstr, connstr.data);
termPQExpBuffer(&connstr);
pg_log(PG_STATUS, "%s", old_db->db_name);
snprintf(sql_file_name, sizeof(sql_file_name), DB_DUMP_FILE_MASK, old_db->db_oid);
@ -509,11 +518,13 @@ create_new_objects(void)
*/
parallel_exec_prog(log_file_name,
NULL,
"\"%s/pg_restore\" %s --exit-on-error --verbose --dbname \"%s\" \"%s\"",
"\"%s/pg_restore\" %s --exit-on-error --verbose --dbname %s \"%s\"",
new_cluster.bindir,
cluster_conn_opts(&new_cluster),
old_db->db_name,
escaped_connstr.data,
sql_file_name);
termPQExpBuffer(&escaped_connstr);
}
/* reap all children */

4
contrib/pg_upgrade/pg_upgrade.h
View File

@ -11,6 +11,7 @@
#include <sys/time.h>
#include "libpq-fe.h"
#include "pqexpbuffer.h"
/* Use port in the private/dynamic port number range */
#define DEF_PGUPORT 50432
@ -440,6 +441,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

50
contrib/pg_upgrade/server.c
View File

@ -51,18 +51,25 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
static PGconn *
get_db_conn(ClusterInfo *cluster, const char *db_name)
{
char conn_opts[2 * NAMEDATALEN + MAXPGPATH + 100];
PQExpBufferData conn_opts;
PGconn *conn;
/* 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);
if (cluster->sockdir)
snprintf(conn_opts, sizeof(conn_opts),
"dbname = '%s' user = '%s' host = '%s' port = %d",
db_name, os_info.user, cluster->sockdir, cluster->port);
else
snprintf(conn_opts, sizeof(conn_opts),
"dbname = '%s' user = '%s' port = %d",
db_name, os_info.user, cluster->port);
{
appendPQExpBufferStr(&conn_opts, " host=");
appendConnStrVal(&conn_opts, cluster->sockdir);
}
return PQconnectdb(conn_opts);
conn = PQconnectdb(conn_opts.data);
termPQExpBuffer(&conn_opts);
return conn;
}
@ -74,23 +81,28 @@ get_db_conn(ClusterInfo *cluster, const char *db_name)
* sets, but the utilities we need aren't very consistent about the treatment
* of database name options, so we leave that out.
*
* Note result is in static storage, so use it right away.
* Result is valid until the next call to this function.
*/
char *
cluster_conn_opts(ClusterInfo *cluster)
{
static char conn_opts[MAXPGPATH + NAMEDATALEN + 100];
static PQExpBuffer buf;
if (buf == NULL)
buf = createPQExpBuffer();
else
resetPQExpBuffer(buf);
if (cluster->sockdir)
snprintf(conn_opts, sizeof(conn_opts),
"--host \"%s\" --port %d --username \"%s\"",
cluster->sockdir, cluster->port, os_info.user);
else
snprintf(conn_opts, sizeof(conn_opts),
"--port %d --username \"%s\"",
cluster->port, os_info.user);
{
appendPQExpBufferStr(buf, "--host ");
appendShellString(buf, cluster->sockdir);
appendPQExpBufferChar(buf, ' ');
}
appendPQExpBuffer(buf, "--port %d --username ", cluster->port);
appendShellString(buf, os_info.user);
return conn_opts;
return buf->data;
}

17
contrib/pg_upgrade/test.sh
View File

@ -155,6 +155,20 @@ set -x
standard_initdb "$oldbindir"/initdb
$oldbindir/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
# Create databases with names covering the ASCII bytes other than NUL, BEL,
# LF, or CR. BEL would ring the terminal bell in the course of this test, and
# it is not otherwise a special case. PostgreSQL doesn't support the rest.
dbname1=`awk 'BEGIN { for (i= 1; i < 46; i++)
if (i != 7 && i != 10 && i != 13) printf "%c", i }' </dev/null`
# Exercise backslashes adjacent to double quotes, a Windows special case.
dbname1='\"\'$dbname1'\\"\\\'
dbname2=`awk 'BEGIN { for (i = 46; i < 91; i++) printf "%c", i }' </dev/null`
dbname3=`awk 'BEGIN { for (i = 91; i < 128; i++) printf "%c", i }' </dev/null`
createdb "$dbname1" || createdb_status=$?
createdb "$dbname2" || createdb_status=$?
createdb "$dbname3" || createdb_status=$?
if "$MAKE" -C "$oldsrc" installcheck; then
pg_dumpall -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
if [ "$newsrc" != "$oldsrc" ]; then
@ -180,6 +194,9 @@ else
make_installcheck_status=$?
fi
$oldbindir/pg_ctl -m fast stop
if [ -n "$createdb_status" ]; then
exit 1
fi
if [ -n "$make_installcheck_status" ]; then
exit 1
fi

204
contrib/pg_upgrade/util.c
View File

@ -180,6 +180,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

10
contrib/pg_upgrade/version.c
View File

@ -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_priv(output_path, "w")) == NULL)
pg_log(PG_FATAL, "could not open file \"%s\": %s\n", output_path, getErrorText(errno));
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");

34
contrib/pg_upgrade/version_old_8_3.c
View File

@ -367,8 +367,13 @@ old_8_3_rebuild_tsvector_tables(ClusterInfo *cluster, bool check_mode)
pg_log(PG_FATAL, "could not open file \"%s\": %s\n", output_path, getErrorText(errno));
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;
}
@ -483,8 +488,12 @@ old_8_3_invalidate_hash_gin_indexes(ClusterInfo *cluster, bool check_mode)
pg_log(PG_FATAL, "could not open file \"%s\": %s\n", output_path, getErrorText(errno));
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",
@ -602,8 +611,12 @@ old_8_3_invalidate_bpchar_pattern_ops_indexes(ClusterInfo *cluster,
pg_log(PG_FATAL, "could not open file \"%s\": %s\n", output_path, getErrorText(errno));
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",
@ -724,8 +737,13 @@ old_8_3_create_sequence_script(ClusterInfo *cluster)
pg_log(PG_FATAL, "could not open file \"%s\": %s\n", output_path, getErrorText(errno));
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;
}