packages/postgresql
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!37 fix CVE-2020-25694 CVE-2020-25695 CVE-2020-25696

Browse Source 
From: @wangxiao65
Reviewed-by: @miao_kaibo
Signed-off-by: @miao_kaibo
This commit is contained in:
openeuler-ci-bot 2020-12-15 11:59:33 +08:00 committed by Gitee
commit a2dc495906
9 changed files with 2926 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

492
CVE-2020-25694-1.patch Normal file
View File

@ -0,0 +1,492 @@
From a062fb822382398c7282810029016400feecf644 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 21 Oct 2020 16:18:41 -0400
Subject: [PATCH] Fix connection string handling in psql's \connect command.
psql's \connect claims to be able to re-use previous connection
parameters, but in fact it only re-uses the database name, user name,
host name (and possibly hostaddr, depending on version), and port.
This is problematic for assorted use cases. Notably, pg_dump[all]
emits "\connect databasename" commands which we would like to have
re-use all other parameters. If such a script is loaded in a psql run
that initially had "-d connstring" with some non-default parameters,
those other parameters would be lost, potentially causing connection
failure. (Thus, this is the same kind of bug addressed in commits
a45bc8a4f and 8e5793ab6, although the details are much different.)
To fix, redesign do_connect() so that it pulls out all properties
of the old PGconn using PQconninfo(), and then replaces individual
properties in that array. In the case where we don't wish to re-use
anything, get libpq's default settings using PQconndefaults() and
replace entries in that, so that we don't need different code paths
for the two cases.
This does result in an additional behavioral change for cases where
the original connection parameters allowed multiple hosts, say
"psql -h host1,host2", and the \connect request allows re-use of the
host setting. Because the previous coding relied on PQhost(), it
would only permit reconnection to the same host originally selected.
Although one can think of scenarios where that's a good thing, there
are others where it is not. Moreover, that behavior doesn't seem to
meet the principle of least surprise, nor was it documented; nor is
it even clear it was intended, since that coding long pre-dates the
addition of multi-host support to libpq. Hence, this patch is content
to drop it and re-use the host list as given.
Per Peter Eisentraut's comments on bug #16604. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
---
doc/src/sgml/ref/psql-ref.sgml | 44 ++++--
src/bin/psql/command.c | 272 ++++++++++++++++++++++++---------
2 files changed, 230 insertions(+), 86 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index c8388513b8..7fda949f3b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -875,21 +875,17 @@ testdb=&gt;
<term><literal>\c</literal> or <literal>\connect [ -reuse-previous=<replaceable class="parameter">on|off</replaceable> ] [ <replaceable class="parameter">dbname</replaceable> [ <replaceable class="parameter">username</replaceable> ] [ <replaceable class="parameter">host</replaceable> ] [ <replaceable class="parameter">port</replaceable> ] | <replaceable class="parameter">conninfo</replaceable> ]</literal></term>
<listitem>
<para>
- Establishes a new connection to a <productname>PostgreSQL</>
+ Establishes a new connection to a <productname>PostgreSQL</productname>
server. The connection parameters to use can be specified either
- using a positional syntax, or using <replaceable>conninfo</> connection
- strings as detailed in <xref linkend="libpq-connstring">.
+ using a positional syntax (one or more of database name, user,
+ host, and port), or using a <replaceable>conninfo</replaceable>
+ connection string as detailed in
+ <xref linkend="libpq-connstring">. If no arguments are given, a
+ new connection is made using the same parameters as before.
</para>
<para>
- Where the command omits database name, user, host, or port, the new
- connection can reuse values from the previous connection. By default,
- values from the previous connection are reused except when processing
- a <replaceable>conninfo</> string. Passing a first argument
- of <literal>-reuse-previous=on</>
- or <literal>-reuse-previous=off</literal> overrides that default.
- When the command neither specifies nor reuses a particular parameter,
- the <application>libpq</application> default is used. Specifying any
+ Specifying any
of <replaceable class="parameter">dbname</replaceable>,
<replaceable class="parameter">username</replaceable>,
<replaceable class="parameter">host</replaceable> or
@@ -897,12 +893,31 @@ testdb=&gt;
as <literal>-</literal> is equivalent to omitting that parameter.
</para>
+ <para>
+ The new connection can re-use connection parameters from the previous
+ connection; not only database name, user, host, and port, but other
+ settings such as <replaceable>sslmode</replaceable>. By default,
+ parameters are re-used in the positional syntax, but not when
+ a <replaceable>conninfo</replaceable> string is given. Passing a
+ first argument of <literal>-reuse-previous=on</literal>
+ or <literal>-reuse-previous=off</literal> overrides that default. If
+ parameters are re-used, then any parameter not explicitly specified as
+ a positional parameter or in the <replaceable>conninfo</replaceable>
+ string is taken from the existing connection's parameters. An
+ exception is that if the <replaceable>host</replaceable> setting
+ is changed from its previous value using the positional syntax,
+ any <replaceable>hostaddr</replaceable> setting present in the
+ existing connection's parameters is dropped.
+ When the command neither specifies nor reuses a particular parameter,
+ the <application>libpq</application> default is used.
+ </para>
+
<para>
If the new connection is successfully made, the previous
connection is closed.
- If the connection attempt failed (wrong user name, access
- denied, etc.), the previous connection will only be kept if
- <application>psql</application> is in interactive mode. When
+ If the connection attempt fails (wrong user name, access
+ denied, etc.), the previous connection will be kept if
+ <application>psql</application> is in interactive mode. But when
executing a non-interactive script, processing will
immediately stop with an error. This distinction was chosen as
a user convenience against typos on the one hand, and a safety
@@ -917,6 +932,7 @@ testdb=&gt;
=&gt; \c mydb myuser host.dom 6432
=&gt; \c service=foo
=&gt; \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable"
+=&gt; \c -reuse-previous=on sslmode=require -- changes only sslmode
=&gt; \c postgresql://tom@localhost/mydb?application_name=myapp
</programlisting>
</listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a30a5f8ea0..91f7f0adc3 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2979,12 +2979,15 @@ do_connect(enum trivalue reuse_previous_specification,
char *dbname, char *user, char *host, char *port)
{
PGconn *o_conn = pset.db,
- *n_conn;
+ *n_conn = NULL;
+ PQconninfoOption *cinfo;
+ int nconnopts = 0;
+ bool same_host = false;
char *password = NULL;
- bool keep_password;
+ bool success = true;
+ bool keep_password = true;
bool has_connection_string;
bool reuse_previous;
- PQExpBufferData connstr;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -3012,6 +3015,11 @@ do_connect(enum trivalue reuse_previous_specification,
reuse_previous = !has_connection_string;
break;
}
+
+ /* If the old connection does not exist, there is nothing to reuse. */
+ if (!o_conn)
+ reuse_previous = false;
+
/* Silently ignore arguments subsequent to a connection string. */
if (has_connection_string)
{
@@ -3020,41 +3028,126 @@ do_connect(enum trivalue reuse_previous_specification,
port = NULL;
}
- /* grab missing values from the old connection */
- if (!user && reuse_previous)
- user = PQuser(o_conn);
- if (!host && reuse_previous)
- host = PQhost(o_conn);
- if (!port && reuse_previous)
- port = PQport(o_conn);
-
/*
- * Any change in the parameters read above makes us discard the password.
- * We also discard it if we're to use a conninfo rather than the
- * positional syntax.
+ * If we intend to re-use connection parameters, collect them out of the
+ * old connection, then replace individual values as necessary. Otherwise,
+ * obtain a PQconninfoOption array containing libpq's defaults, and modify
+ * that. Note this function assumes that PQconninfo, PQconndefaults, and
+ * PQconninfoParse will all produce arrays containing the same options in
+ * the same order.
*/
- if (has_connection_string)
- keep_password = false;
+ if (reuse_previous)
+ cinfo = PQconninfo(o_conn);
else
- keep_password =
- (user && PQuser(o_conn) && strcmp(user, PQuser(o_conn)) == 0) &&
- (host && PQhost(o_conn) && strcmp(host, PQhost(o_conn)) == 0) &&
- (port && PQport(o_conn) && strcmp(port, PQport(o_conn)) == 0);
+ cinfo = PQconndefaults();
- /*
- * Grab missing dbname from old connection. No password discard if this
- * changes: passwords aren't (usually) database-specific.
- */
- if (!dbname && reuse_previous)
+ if (cinfo)
{
- initPQExpBuffer(&connstr);
- appendPQExpBuffer(&connstr, "dbname=");
- appendConnStrVal(&connstr, PQdb(o_conn));
- dbname = connstr.data;
- /* has_connection_string=true would be a dead store */
+ if (has_connection_string)
+ {
+ /* Parse the connstring and insert values into cinfo */
+ PQconninfoOption *replcinfo;
+ char *errmsg;
+
+ replcinfo = PQconninfoParse(dbname, &errmsg);
+ if (replcinfo)
+ {
+ PQconninfoOption *ci;
+ PQconninfoOption *replci;
+
+ for (ci = cinfo, replci = replcinfo;
+ ci->keyword && replci->keyword;
+ ci++, replci++)
+ {
+ Assert(strcmp(ci->keyword, replci->keyword) == 0);
+ /* Insert value from connstring if one was provided */
+ if (replci->val)
+ {
+ /*
+ * We know that both val strings were allocated by
+ * libpq, so the least messy way to avoid memory leaks
+ * is to swap them.
+ */
+ char *swap = replci->val;
+
+ replci->val = ci->val;
+ ci->val = swap;
+ }
+ }
+ Assert(ci->keyword == NULL && replci->keyword == NULL);
+
+ /* While here, determine how many option slots there are */
+ nconnopts = ci - cinfo;
+
+ PQconninfoFree(replcinfo);
+
+ /* We never re-use a password with a conninfo string. */
+ keep_password = false;
+
+ /* Don't let code below try to inject dbname into params. */
+ dbname = NULL;
+ }
+ else
+ {
+ /* PQconninfoParse failed */
+ if (errmsg)
+ {
+ psql_error("%s", errmsg);
+ PQfreemem(errmsg);
+ }
+ else
+ psql_error("out of memory\n");
+ success = false;
+ }
+ }
+ else
+ {
+ /*
+ * If dbname isn't a connection string, then we'll inject it and
+ * the other parameters into the keyword array below. (We can't
+ * easily insert them into the cinfo array because of memory
+ * management issues: PQconninfoFree would misbehave on Windows.)
+ * However, to avoid dependencies on the order in which parameters
+ * appear in the array, make a preliminary scan to set
+ * keep_password and same_host correctly.
+ *
+ * While any change in user, host, or port causes us to ignore the
+ * old connection's password, we don't force that for dbname,
+ * since passwords aren't database-specific.
+ */
+ PQconninfoOption *ci;
+
+ for (ci = cinfo; ci->keyword; ci++)
+ {
+ if (user && strcmp(ci->keyword, "user") == 0)
+ {
+ if (!(ci->val && strcmp(user, ci->val) == 0))
+ keep_password = false;
+ }
+ else if (host && strcmp(ci->keyword, "host") == 0)
+ {
+ if (ci->val && strcmp(host, ci->val) == 0)
+ same_host = true;
+ else
+ keep_password = false;
+ }
+ else if (port && strcmp(ci->keyword, "port") == 0)
+ {
+ if (!(ci->val && strcmp(port, ci->val) == 0))
+ keep_password = false;
+ }
+ }
+
+ /* While here, determine how many option slots there are */
+ nconnopts = ci - cinfo;
+ }
}
else
- connstr.data = NULL;
+ {
+ /* We failed to create the cinfo structure */
+ psql_error("out of memory\n");
+ success = false;
+ }
/*
* If the user asked to be prompted for a password, ask for one now. If
@@ -3066,7 +3159,7 @@ do_connect(enum trivalue reuse_previous_specification,
* the postmaster's log. But libpq offers no API that would let us obtain
* a password and then continue with the first connection attempt.
*/
- if (pset.getPassword == TRI_YES)
+ if (pset.getPassword == TRI_YES && success)
{
password = prompt_for_password(user);
}
@@ -3079,52 +3172,60 @@ do_connect(enum trivalue reuse_previous_specification,
password = NULL;
}
- while (true)
+ /* Loop till we have a connection or fail, which we might've already */
+ while (success)
{
-#define PARAMS_ARRAY_SIZE 8
- const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
- const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
- int paramnum = -1;
-
- keywords[++paramnum] = "host";
- values[paramnum] = host;
- keywords[++paramnum] = "port";
- values[paramnum] = port;
- keywords[++paramnum] = "user";
- values[paramnum] = user;
+ const char **keywords = pg_malloc((nconnopts + 1) * sizeof(*keywords));
+ const char **values = pg_malloc((nconnopts + 1) * sizeof(*values));
+ int paramnum = 0;
+ PQconninfoOption *ci;
/*
- * Position in the array matters when the dbname is a connection
- * string, because settings in a connection string override earlier
- * array entries only. Thus, user= in the connection string always
- * takes effect, but client_encoding= often will not.
+ * Copy non-default settings into the PQconnectdbParams parameter
+ * arrays; but override any values specified old-style, as well as the
+ * password and a couple of fields we want to set forcibly.
*
- * If you change this code, also change the initial-connection code in
+ * If you change this code, see also the initial-connection code in
* main(). For no good reason, a connection string password= takes
* precedence in main() but not here.
*/
- keywords[++paramnum] = "dbname";
- values[paramnum] = dbname;
- keywords[++paramnum] = "password";
- values[paramnum] = password;
- keywords[++paramnum] = "fallback_application_name";
- values[paramnum] = pset.progname;
- keywords[++paramnum] = "client_encoding";
- values[paramnum] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
-
+ for (ci = cinfo; ci->keyword; ci++)
+ {
+ keywords[paramnum] = ci->keyword;
+
+ if (dbname && strcmp(ci->keyword, "dbname") == 0)
+ values[paramnum++] = dbname;
+ else if (user && strcmp(ci->keyword, "user") == 0)
+ values[paramnum++] = user;
+ else if (host && strcmp(ci->keyword, "host") == 0)
+ values[paramnum++] = host;
+ else if (host && !same_host && strcmp(ci->keyword, "hostaddr") == 0)
+ {
+ /* If we're changing the host value, drop any old hostaddr */
+ values[paramnum++] = NULL;
+ }
+ else if (port && strcmp(ci->keyword, "port") == 0)
+ values[paramnum++] = port;
+ else if (strcmp(ci->keyword, "password") == 0)
+ values[paramnum++] = password;
+ else if (strcmp(ci->keyword, "fallback_application_name") == 0)
+ values[paramnum++] = pset.progname;
+ else if (strcmp(ci->keyword, "client_encoding") == 0)
+ values[paramnum++] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
+ else if (ci->val)
+ values[paramnum++] = ci->val;
+ /* else, don't bother making libpq parse this keyword */
+ }
/* add array terminator */
- keywords[++paramnum] = NULL;
+ keywords[paramnum] = NULL;
values[paramnum] = NULL;
- n_conn = PQconnectdbParams(keywords, values, true);
+ /* Note we do not want libpq to re-expand the dbname parameter */
+ n_conn = PQconnectdbParams(keywords, values, false);
pg_free(keywords);
pg_free(values);
- /* We can immediately discard the password -- no longer needed */
- if (password)
- pg_free(password);
-
if (PQstatus(n_conn) == CONNECTION_OK)
break;
@@ -3134,11 +3235,34 @@ do_connect(enum trivalue reuse_previous_specification,
*/
if (!password && PQconnectionNeedsPassword(n_conn) && pset.getPassword != TRI_NO)
{
+ /*
+ * Prompt for password using the username we actually connected
+ * with --- it might've come out of "dbname" rather than "user".
+ */
+ password = prompt_for_password(PQuser(n_conn));
PQfinish(n_conn);
- password = prompt_for_password(user);
+ n_conn = NULL;
continue;
}
+ /*
+ * We'll report the error below ... unless n_conn is NULL, indicating
+ * that libpq didn't have enough memory to make a PGconn.
+ */
+ if (n_conn == NULL)
+ psql_error("out of memory\n");
+
+ success = false;
+ } /* end retry loop */
+
+ /* Release locally allocated data, whether we succeeded or not */
+ if (password)
+ pg_free(password);
+ if (cinfo)
+ PQconninfoFree(cinfo);
+
+ if (!success)
+ {
/*
* Failed to connect to the database. In interactive mode, keep the
* previous connection to the DB; in scripting mode, close our
@@ -3146,7 +3270,11 @@ do_connect(enum trivalue reuse_previous_specification,
*/
if (pset.cur_cmd_interactive)
{
- psql_error("%s", PQerrorMessage(n_conn));
+ if (n_conn)
+ {
+ psql_error("%s", PQerrorMessage(n_conn));
+ PQfinish(n_conn);
+ }
/* pset.db is left unmodified */
if (o_conn)
@@ -3154,7 +3282,12 @@ do_connect(enum trivalue reuse_previous_specification,
}
else
{
- psql_error("\\connect: %s", PQerrorMessage(n_conn));
+ if (n_conn)
+ {
+ psql_error("\\connect: %s", PQerrorMessage(n_conn));
+ PQfinish(n_conn);
+ }
+
if (o_conn)
{
PQfinish(o_conn);
@@ -3162,13 +3295,8 @@ 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
--
2.23.0

1076
CVE-2020-25694-2.patch Normal file
View File

File diff suppressed because it is too large Load Diff

782
CVE-2020-25694-3.patch Normal file
View File

@ -0,0 +1,782 @@
From bb3ab8bfb690721923e987d2dfab683b462c1e88 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 24 Sep 2020 18:19:39 -0400
Subject: [PATCH] Fix handling of -d "connection string" in pg_dump/pg_restore.
Parallel pg_dump failed if its -d parameter was a connection string
containing any essential information other than host, port, or username.
The same was true for pg_restore with --create.
The reason is that these scenarios failed to preserve the connection
string from the command line; the code felt free to replace that with
just the database name when reconnecting from a pg_dump parallel worker
or after creating the target database. By chance, parallel pg_restore
did not suffer this defect, as long as you didn't say --create.
In practice it seems that the error would be obvious only if the
connstring included essential, non-default SSL or GSS parameters.
This may explain why it took us so long to notice. (It also makes
it very difficult to craft a regression test case illustrating the
problem, since the test would fail in builds without those options.)
Fix by refactoring so that ConnectDatabase always receives all the
relevant options directly from the command line, rather than
reconstructed values. Inject a different database name, when necessary,
by relying on libpq's rules for handling multiple "dbname" parameters.
While here, let's get rid of the essentially duplicate _connectDB
function, as well as some obsolete nearby cruft.
Per bug #16604 from Zsolt Ero. Back-patch to all supported branches.
Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
---
src/bin/pg_dump/pg_backup.h | 36 ++--
src/bin/pg_dump/pg_backup_archiver.c | 96 +++--------
src/bin/pg_dump/pg_backup_archiver.h | 3 +-
src/bin/pg_dump/pg_backup_db.c | 249 +++++++--------------------
src/bin/pg_dump/pg_dump.c | 24 +--
src/bin/pg_dump/pg_restore.c | 14 +-
6 files changed, 131 insertions(+), 291 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 560de01884..72c6a8db6c 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -58,6 +58,20 @@ typedef enum _teSection
SECTION_POST_DATA /* stuff to be processed after data */
} teSection;
+/* Parameters needed by ConnectDatabase; same for dump and restore */
+typedef struct _connParams
+{
+ /* These fields record the actual command line parameters */
+ char *dbname; /* this may be a connstring! */
+ char *pgport;
+ char *pghost;
+ char *username;
+ trivalue promptPassword;
+ /* If not NULL, this overrides the dbname obtained from command line */
+ /* (but *only* the DB name, not anything else in the connstring) */
+ char *override_dbname;
+} ConnParams;
+
typedef struct _restoreOptions
{
int createDB; /* Issue commands to create the database */
@@ -106,12 +120,9 @@ typedef struct _restoreOptions
SimpleStringList tableNames;
int useDB;
- char *dbname; /* subject to expand_dbname */
- char *pgport;
- char *pghost;
- char *username;
+ ConnParams cparams; /* parameters to use if useDB */
+
int noDataForFailedTables;
- trivalue promptPassword;
int exit_on_error;
int compression;
int suppressDumpWarnings; /* Suppress output of WARNING entries
@@ -126,10 +137,8 @@ typedef struct _restoreOptions
typedef struct _dumpOptions
{
- const char *dbname; /* subject to expand_dbname */
- const char *pghost;
- const char *pgport;
- const char *username;
+ ConnParams cparams;
+
bool oids;
int binary_upgrade;
@@ -239,12 +248,9 @@ typedef void (*SetupWorkerPtrType) (Archive *AH);
* Main archiver interface.
*/
-extern void ConnectDatabase(Archive *AH,
- const char *dbname,
- const char *pghost,
- const char *pgport,
- const char *username,
- trivalue prompt_password);
+extern void ConnectDatabase(Archive *AHX,
+ const ConnParams *cparams,
+ bool isReconnect);
extern void DisconnectDatabase(Archive *AHX);
extern PGconn *GetConnection(Archive *AHX);
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 8892b17790..4003ab3b13 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -144,6 +144,7 @@ InitDumpOptions(DumpOptions *opts)
memset(opts, 0, sizeof(DumpOptions));
/* set any fields that shouldn't default to zeroes */
opts->include_everything = true;
+ opts->cparams.promptPassword = TRI_DEFAULT;
opts->dumpSections = DUMP_UNSECTIONED;
}
@@ -157,6 +158,11 @@ dumpOptionsFromRestoreOptions(RestoreOptions *ropt)
DumpOptions *dopt = NewDumpOptions();
/* this is the inverse of what's at the end of pg_dump.c's main() */
+ dopt->cparams.dbname = ropt->cparams.dbname ? pg_strdup(ropt->cparams.dbname) : NULL;
+ dopt->cparams.pgport = ropt->cparams.pgport ? pg_strdup(ropt->cparams.pgport) : NULL;
+ dopt->cparams.pghost = ropt->cparams.pghost ? pg_strdup(ropt->cparams.pghost) : NULL;
+ dopt->cparams.username = ropt->cparams.username ? pg_strdup(ropt->cparams.username) : NULL;
+ dopt->cparams.promptPassword = ropt->cparams.promptPassword;
dopt->outputClean = ropt->dropSchema;
dopt->dataOnly = ropt->dataOnly;
dopt->schemaOnly = ropt->schemaOnly;
@@ -401,9 +407,7 @@ RestoreArchive(Archive *AHX)
AHX->minRemoteVersion = 0;
AHX->maxRemoteVersion = 9999999;
- ConnectDatabase(AHX, ropt->dbname,
- ropt->pghost, ropt->pgport, ropt->username,
- ropt->promptPassword);
+ ConnectDatabase(AHX, &ropt->cparams, false);
/*
* If we're talking to the DB directly, don't send comments since they
@@ -823,16 +827,8 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
/* 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 = connstr.data;
}
}
@@ -966,7 +962,7 @@ NewRestoreOptions(void)
/* set any fields that shouldn't default to zeroes */
opts->format = archUnknown;
- opts->promptPassword = TRI_DEFAULT;
+ opts->cparams.promptPassword = TRI_DEFAULT;
opts->dumpSections = DUMP_UNSECTIONED;
return opts;
@@ -2388,8 +2384,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
else
AH->format = fmt;
- AH->promptPassword = TRI_DEFAULT;
-
switch (AH->format)
{
case archCustom:
@@ -3157,27 +3151,20 @@ _doSetWithOids(ArchiveHandle *AH, const bool withOids)
* If we're currently restoring right into a database, this will
* actually establish a connection. Otherwise it puts a \connect into
* the script output.
- *
- * NULL dbname implies reconnecting to the current DB (pretty useless).
*/
static void
_reconnectToDB(ArchiveHandle *AH, const char *dbname)
{
if (RestoringToDB(AH))
- ReconnectToServer(AH, dbname, NULL);
+ ReconnectToServer(AH, dbname);
else
{
- if (dbname)
- {
- PQExpBufferData connectbuf;
+ PQExpBufferData connectbuf;
- initPQExpBuffer(&connectbuf);
- appendPsqlMetaConnect(&connectbuf, dbname);
- ahprintf(AH, "%s\n", connectbuf.data);
- termPQExpBuffer(&connectbuf);
- }
- else
- ahprintf(AH, "%s\n", "\\connect -\n");
+ initPQExpBuffer(&connectbuf);
+ appendPsqlMetaConnect(&connectbuf, dbname);
+ ahprintf(AH, "%s\n", connectbuf.data);
+ termPQExpBuffer(&connectbuf);
}
/*
@@ -4107,9 +4094,7 @@ restore_toc_entries_postfork(ArchiveHandle *AH, TocEntry *pending_list)
/*
* Now reconnect the single parent connection.
*/
- ConnectDatabase((Archive *) AH, ropt->dbname,
- ropt->pghost, ropt->pgport, ropt->username,
- ropt->promptPassword);
+ ConnectDatabase((Archive *) AH, &ropt->cparams, true);
/* re-establish fixed state */
_doSetFixedOutputState(AH);
@@ -4690,54 +4675,15 @@ CloneArchive(ArchiveHandle *AH)
clone->public.n_errors = 0;
/*
- * Connect our new clone object to the database: In parallel restore the
- * parent is already disconnected, because we can connect the worker
- * processes independently to the database (no snapshot sync required). In
- * parallel backup we clone the parent's existing connection.
+ * Connect our new clone object to the database, using the same connection
+ * parameters used for the original connection.
*/
- if (AH->mode == archModeRead)
- {
- RestoreOptions *ropt = AH->public.ropt;
-
- Assert(AH->connection == NULL);
-
- /* this also sets clone->connection */
- ConnectDatabase((Archive *) clone, ropt->dbname,
- ropt->pghost, ropt->pgport, ropt->username,
- ropt->promptPassword);
+ ConnectDatabase((Archive *) clone, &clone->public.ropt->cparams, true);
- /* re-establish fixed state */
+ /* re-establish fixed state */
+ if (AH->mode == archModeRead)
_doSetFixedOutputState(clone);
- }
- else
- {
- PQExpBufferData connstr;
- char *pghost;
- char *pgport;
- char *username;
-
- Assert(AH->connection != NULL);
-
- /*
- * Even though we are technically accessing the parent's database
- * object here, these functions are fine to be called like that
- * because all just return a pointer and do not actually send/receive
- * any data to/from the database.
- */
- initPQExpBuffer(&connstr);
- appendPQExpBuffer(&connstr, "dbname=");
- appendConnStrVal(&connstr, PQdb(AH->connection));
- pghost = PQhost(AH->connection);
- pgport = PQport(AH->connection);
- username = PQuser(AH->connection);
-
- /* this also sets clone->connection */
- ConnectDatabase((Archive *) clone, connstr.data,
- pghost, pgport, username, TRI_NO);
-
- termPQExpBuffer(&connstr);
- /* setupDumpWorker will fix up connection state */
- }
+ /* in write case, setupDumpWorker will fix up connection state */
/* Let the format-specific code have a chance too */
(clone->ClonePtr) (clone);
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index 3b69868359..50829c4b2e 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -304,7 +304,6 @@ struct _archiveHandle
/* Stuff for direct DB connection */
char *archdbname; /* DB name *read* from archive */
- trivalue promptPassword;
char *savedPassword; /* password for ropt->username, if known */
char *use_role;
PGconn *connection;
@@ -450,7 +449,7 @@ extern void InitArchiveFmt_Tar(ArchiveHandle *AH);
extern bool isValidTarHeader(char *header);
-extern int ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *newUser);
+extern void ReconnectToServer(ArchiveHandle *AH, const char *dbname);
extern void DropBlobIfExists(ArchiveHandle *AH, Oid oid);
void ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH);
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 43f5941f93..4eaea4af4f 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -30,7 +30,6 @@
static const char *modulename = gettext_noop("archiver (db)");
static void _check_database_version(ArchiveHandle *AH);
-static PGconn *_connectDB(ArchiveHandle *AH, const char *newdbname, const char *newUser);
static void notice_processor(void *arg, const char *message);
static void
@@ -76,186 +75,51 @@ _check_database_version(ArchiveHandle *AH)
/*
* Reconnect to the server. If dbname is not NULL, use that database,
- * else the one associated with the archive handle. If username is
- * not NULL, use that user name, else the one from the handle. If
- * both the database and the user match the existing connection already,
- * nothing will be done.
- *
- * Returns 1 in any case.
+ * else the one associated with the archive handle.
*/
-int
-ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username)
-{
- PGconn *newConn;
- const char *newdbname;
- const char *newusername;
-
- if (!dbname)
- newdbname = PQdb(AH->connection);
- else
- newdbname = dbname;
-
- if (!username)
- newusername = PQuser(AH->connection);
- else
- newusername = username;
-
- /* Let's see if the request is already satisfied */
- if (strcmp(newdbname, PQdb(AH->connection)) == 0 &&
- strcmp(newusername, PQuser(AH->connection)) == 0)
- return 1;
-
- newConn = _connectDB(AH, newdbname, newusername);
-
- /* Update ArchiveHandle's connCancel before closing old connection */
- set_archive_cancel_info(AH, newConn);
-
- PQfinish(AH->connection);
- AH->connection = newConn;
-
- /* Start strict; later phases may override this. */
- PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
- ALWAYS_SECURE_SEARCH_PATH_SQL));
-
- return 1;
-}
-
-/*
- * Connect to the db again.
- *
- * Note: it's not really all that sensible to use a single-entry password
- * cache if the username keeps changing. In current usage, however, the
- * username never does change, so one savedPassword is sufficient. We do
- * update the cache on the off chance that the password has changed since the
- * start of the run.
- */
-static PGconn *
-_connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
+void
+ReconnectToServer(ArchiveHandle *AH, const char *dbname)
{
- PQExpBufferData connstr;
- PGconn *newConn;
- const char *newdb;
- const char *newuser;
- char *password;
- char passbuf[100];
- bool new_pass;
-
- if (!reqdb)
- newdb = PQdb(AH->connection);
- else
- newdb = reqdb;
-
- if (!requser || strlen(requser) == 0)
- newuser = PQuser(AH->connection);
- else
- newuser = requser;
-
- ahlog(AH, 1, "connecting to database \"%s\" as user \"%s\"\n",
- newdb, newuser);
-
- password = AH->savedPassword;
-
- if (AH->promptPassword == TRI_YES && password == NULL)
- {
- simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
- password = passbuf;
- }
-
- initPQExpBuffer(&connstr);
- appendPQExpBuffer(&connstr, "dbname=");
- appendConnStrVal(&connstr, newdb);
-
- do
- {
- const char *keywords[7];
- const char *values[7];
-
- keywords[0] = "host";
- values[0] = PQhost(AH->connection);
- keywords[1] = "port";
- values[1] = PQport(AH->connection);
- keywords[2] = "user";
- values[2] = newuser;
- keywords[3] = "password";
- values[3] = password;
- keywords[4] = "dbname";
- values[4] = connstr.data;
- keywords[5] = "fallback_application_name";
- values[5] = progname;
- keywords[6] = NULL;
- values[6] = NULL;
-
- new_pass = false;
- newConn = PQconnectdbParams(keywords, values, true);
-
- if (!newConn)
- exit_horribly(modulename, "failed to reconnect to database\n");
-
- if (PQstatus(newConn) == CONNECTION_BAD)
- {
- if (!PQconnectionNeedsPassword(newConn))
- exit_horribly(modulename, "could not reconnect to database: %s",
- PQerrorMessage(newConn));
- PQfinish(newConn);
-
- if (password)
- fprintf(stderr, "Password incorrect\n");
-
- fprintf(stderr, "Connecting to %s as %s\n",
- newdb, newuser);
-
- if (AH->promptPassword != TRI_NO)
- {
- simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
- password = passbuf;
- }
- else
- exit_horribly(modulename, "connection needs password\n");
-
- new_pass = true;
- }
- } while (new_pass);
+ PGconn *oldConn = AH->connection;
+ RestoreOptions *ropt = AH->public.ropt;
/*
- * We want to remember connection's actual password, whether or not we got
- * it by prompting. So we don't just store the password variable.
+ * Save the dbname, if given, in override_dbname so that it will also
+ * affect any later reconnection attempt.
*/
- if (PQconnectionUsedPassword(newConn))
- {
- if (AH->savedPassword)
- free(AH->savedPassword);
- AH->savedPassword = pg_strdup(PQpass(newConn));
- }
+ if (dbname)
+ ropt->cparams.override_dbname = pg_strdup(dbname);
- termPQExpBuffer(&connstr);
-
- /* check for version mismatch */
- _check_database_version(AH);
+ /*
+ * Note: we want to establish the new connection, and in particular update
+ * ArchiveHandle's connCancel, before closing old connection. Otherwise
+ * an ill-timed SIGINT could try to access a dead connection.
+ */
+ AH->connection = NULL; /* dodge error check in ConnectDatabase */
- PQsetNoticeProcessor(newConn, notice_processor, NULL);
+ ConnectDatabase((Archive *) AH, &ropt->cparams, true);
- return newConn;
+ PQfinish(oldConn);
}
-
/*
- * Make a database connection with the given parameters. The
- * connection handle is returned, the parameters are stored in AHX.
- * An interactive password prompt is automatically issued if required.
+ * Make, or remake, a database connection with the given parameters.
+ *
+ * The resulting connection handle is stored in AHX->connection.
*
+ * An interactive password prompt is automatically issued if required.
+ * We store the results of that in AHX->savedPassword.
* Note: it's not really all that sensible to use a single-entry password
* cache if the username keeps changing. In current usage, however, the
* username never does change, so one savedPassword is sufficient.
*/
void
ConnectDatabase(Archive *AHX,
- const char *dbname,
- const char *pghost,
- const char *pgport,
- const char *username,
- trivalue prompt_password)
+ const ConnParams *cparams,
+ bool isReconnect)
{
ArchiveHandle *AH = (ArchiveHandle *) AHX;
+ trivalue prompt_password;
char *password;
char passbuf[100];
bool new_pass;
@@ -263,6 +127,9 @@ ConnectDatabase(Archive *AHX,
if (AH->connection)
exit_horribly(modulename, "already connected to a database\n");
+ /* Never prompt for a password during a reconnection */
+ prompt_password = isReconnect ? TRI_NO : cparams->promptPassword;
+
password = AH->savedPassword;
if (prompt_password == TRI_YES && password == NULL)
@@ -270,7 +137,6 @@ ConnectDatabase(Archive *AHX,
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
}
- AH->promptPassword = prompt_password;
/*
* Start the connection. Loop until we have a password if requested by
@@ -278,23 +144,35 @@ ConnectDatabase(Archive *AHX,
*/
do
{
- const char *keywords[7];
- const char *values[7];
-
- keywords[0] = "host";
- values[0] = pghost;
- keywords[1] = "port";
- values[1] = pgport;
- keywords[2] = "user";
- values[2] = username;
- keywords[3] = "password";
- values[3] = password;
- keywords[4] = "dbname";
- values[4] = dbname;
- keywords[5] = "fallback_application_name";
- values[5] = progname;
- keywords[6] = NULL;
- values[6] = NULL;
+ const char *keywords[8];
+ const char *values[8];
+ int i = 0;
+
+ /*
+ * If dbname is a connstring, its entries can override the other
+ * values obtained from cparams; but in turn, override_dbname can
+ * override the dbname component of it.
+ */
+ keywords[i] = "host";
+ values[i++] = cparams->pghost;
+ keywords[i] = "port";
+ values[i++] = cparams->pgport;
+ keywords[i] = "user";
+ values[i++] = cparams->username;
+ keywords[i] = "password";
+ values[i++] = password;
+ keywords[i] = "dbname";
+ values[i++] = cparams->dbname;
+ if (cparams->override_dbname)
+ {
+ keywords[i] = "dbname";
+ values[i++] = cparams->override_dbname;
+ }
+ keywords[i] = "fallback_application_name";
+ values[i++] = progname;
+ keywords[i] = NULL;
+ values[i++] = NULL;
+ Assert(i <= lengthof(keywords));
new_pass = false;
AH->connection = PQconnectdbParams(keywords, values, true);
@@ -316,9 +194,16 @@ ConnectDatabase(Archive *AHX,
/* check to see that the backend connection was successfully made */
if (PQstatus(AH->connection) == CONNECTION_BAD)
- exit_horribly(modulename, "connection to database \"%s\" failed: %s",
- PQdb(AH->connection) ? PQdb(AH->connection) : "",
- PQerrorMessage(AH->connection));
+ {
+ if (isReconnect)
+ exit_horribly(modulename, "reconnection to database \"%s\" failed: %s",
+ PQdb(AH->connection) ? PQdb(AH->connection) : "",
+ PQerrorMessage(AH->connection));
+ else
+ exit_horribly(modulename, "connection to database \"%s\" failed: %s",
+ PQdb(AH->connection) ? PQdb(AH->connection) : "",
+ PQerrorMessage(AH->connection));
+ }
/* Start strict; later phases may override this. */
PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8080155a95..abcee89d10 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -302,7 +302,6 @@ main(int argc, char **argv)
const char *dumpsnapshot = NULL;
char *use_role = NULL;
int numWorkers = 1;
- trivalue prompt_password = TRI_DEFAULT;
int compressLevel = -1;
int plainText = 0;
ArchiveFormat archiveFormat = archUnknown;
@@ -431,7 +430,7 @@ main(int argc, char **argv)
break;
case 'd': /* database name */
- dopt.dbname = pg_strdup(optarg);
+ dopt.cparams.dbname = pg_strdup(optarg);
break;
case 'E': /* Dump encoding */
@@ -447,7 +446,7 @@ main(int argc, char **argv)
break;
case 'h': /* server host */
- dopt.pghost = pg_strdup(optarg);
+ dopt.cparams.pghost = pg_strdup(optarg);
break;
case 'j': /* number of dump jobs */
@@ -472,7 +471,7 @@ main(int argc, char **argv)
break;
case 'p': /* server port */
- dopt.pgport = pg_strdup(optarg);
+ dopt.cparams.pgport = pg_strdup(optarg);
break;
case 'R':
@@ -497,7 +496,7 @@ main(int argc, char **argv)
break;
case 'U':
- dopt.username = pg_strdup(optarg);
+ dopt.cparams.username = pg_strdup(optarg);
break;
case 'v': /* verbose */
@@ -505,11 +504,11 @@ main(int argc, char **argv)
break;
case 'w':
- prompt_password = TRI_NO;
+ dopt.cparams.promptPassword = TRI_NO;
break;
case 'W':
- prompt_password = TRI_YES;
+ dopt.cparams.promptPassword = TRI_YES;
break;
case 'x': /* skip ACL dump */
@@ -563,8 +562,8 @@ main(int argc, char **argv)
* Non-option argument specifies database name as long as it wasn't
* already specified with -d / --dbname
*/
- if (optind < argc && dopt.dbname == NULL)
- dopt.dbname = argv[optind++];
+ if (optind < argc && dopt.cparams.dbname == NULL)
+ dopt.cparams.dbname = argv[optind++];
/* Complain if any arguments remain */
if (optind < argc)
@@ -677,7 +676,7 @@ main(int argc, char **argv)
* Open the database using the Archiver, so it knows about it. Errors mean
* death.
*/
- ConnectDatabase(fout, dopt.dbname, dopt.pghost, dopt.pgport, dopt.username, prompt_password);
+ ConnectDatabase(fout, &dopt.cparams, false);
setup_connection(fout, dumpencoding, dumpsnapshot, use_role);
/*
@@ -860,6 +859,11 @@ main(int argc, char **argv)
ropt->filename = filename;
/* if you change this list, see dumpOptionsFromRestoreOptions */
+ ropt->cparams.dbname = dopt.cparams.dbname ? pg_strdup(dopt.cparams.dbname) : NULL;
+ ropt->cparams.pgport = dopt.cparams.pgport ? pg_strdup(dopt.cparams.pgport) : NULL;
+ ropt->cparams.pghost = dopt.cparams.pghost ? pg_strdup(dopt.cparams.pghost) : NULL;
+ ropt->cparams.username = dopt.cparams.username ? pg_strdup(dopt.cparams.username) : NULL;
+ ropt->cparams.promptPassword = dopt.cparams.promptPassword;
ropt->dropSchema = dopt.outputClean;
ropt->dataOnly = dopt.dataOnly;
ropt->schemaOnly = dopt.schemaOnly;
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 860a211a3c..ce03ded8ec 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -163,7 +163,7 @@ main(int argc, char **argv)
opts->createDB = 1;
break;
case 'd':
- opts->dbname = pg_strdup(optarg);
+ opts->cparams.dbname = pg_strdup(optarg);
break;
case 'e':
opts->exit_on_error = true;
@@ -177,7 +177,7 @@ main(int argc, char **argv)
break;
case 'h':
if (strlen(optarg) != 0)
- opts->pghost = pg_strdup(optarg);
+ opts->cparams.pghost = pg_strdup(optarg);
break;
case 'j': /* number of restore jobs */
@@ -206,7 +206,7 @@ main(int argc, char **argv)
case 'p':
if (strlen(optarg) != 0)
- opts->pgport = pg_strdup(optarg);
+ opts->cparams.pgport = pg_strdup(optarg);
break;
case 'R':
/* no-op, still accepted for backwards compatibility */
@@ -240,7 +240,7 @@ main(int argc, char **argv)
break;
case 'U':
- opts->username = pg_strdup(optarg);
+ opts->cparams.username = pg_strdup(optarg);
break;
case 'v': /* verbose */
@@ -248,11 +248,11 @@ main(int argc, char **argv)
break;
case 'w':
- opts->promptPassword = TRI_NO;
+ opts->cparams.promptPassword = TRI_NO;
break;
case 'W':
- opts->promptPassword = TRI_YES;
+ opts->cparams.promptPassword = TRI_YES;
break;
case 'x': /* skip ACL dump */
@@ -302,7 +302,7 @@ main(int argc, char **argv)
}
/* Should get at most one of -d and -f, else user is confused */
- if (opts->dbname)
+ if (opts->cparams.dbname)
{
if (opts->filename)
{
--
2.23.0

244
CVE-2020-25695.patch Normal file
View File

@ -0,0 +1,244 @@
From 0c3185e963d9f9dd0608214f7d732b84aa0888fe Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Mon, 9 Nov 2020 07:32:09 -0800
Subject: [PATCH] In security-restricted operations, block enqueue of at-commit
user code.
Specifically, this blocks DECLARE ... WITH HOLD and firing of deferred
triggers within index expressions and materialized view queries. An
attacker having permission to create non-temp objects in at least one
schema could execute arbitrary SQL functions under the identity of the
bootstrap superuser. One can work around the vulnerability by disabling
autovacuum and not manually running ANALYZE, CLUSTER, REINDEX, CREATE
INDEX, VACUUM FULL, or REFRESH MATERIALIZED VIEW. (Don't restore from
pg_dump, since it runs some of those commands.) Plain VACUUM (without
FULL) is safe, and all commands are fine when a trusted user owns the
target object. Performance may degrade quickly under this workaround,
however. Back-patch to 9.5 (all supported versions).
Reviewed by Robert Haas. Reported by Etienne Stalmans.
Security: CVE-2020-25695
---
contrib/postgres_fdw/connection.c | 4 +++
src/backend/access/transam/xact.c | 13 ++++----
src/backend/commands/portalcmds.c | 5 +++
src/backend/commands/trigger.c | 12 +++++++
src/test/regress/expected/privileges.out | 42 ++++++++++++++++++++++++
src/test/regress/sql/privileges.sql | 34 +++++++++++++++++++
6 files changed, 104 insertions(+), 6 deletions(-)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index be4ec07cf9..09ffb21d48 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -645,6 +645,10 @@ pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
/*
* pgfdw_xact_callback --- cleanup at main-transaction end.
+ *
+ * This runs just late enough that it must not enter user-defined code
+ * locally. (Entering such code on the remote side is fine. Its remote
+ * COMMIT TRANSACTION may run deferred triggers.)
*/
static void
pgfdw_xact_callback(XactEvent event, void *arg)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 9004e38e6d..e2ca8a5d2e 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1961,9 +1961,10 @@ CommitTransaction(void)
/*
* Do pre-commit processing that involves calling user-defined code, such
- * as triggers. Since closing cursors could queue trigger actions,
- * triggers could open cursors, etc, we have to keep looping until there's
- * nothing left to do.
+ * as triggers. SECURITY_RESTRICTED_OPERATION contexts must not queue an
+ * action that would run here, because that would bypass the sandbox.
+ * Since closing cursors could queue trigger actions, triggers could open
+ * cursors, etc, we have to keep looping until there's nothing left to do.
*/
for (;;)
{
@@ -1981,9 +1982,6 @@ CommitTransaction(void)
break;
}
- CallXactCallbacks(is_parallel_worker ? XACT_EVENT_PARALLEL_PRE_COMMIT
- : XACT_EVENT_PRE_COMMIT);
-
/*
* The remaining actions cannot call any user-defined code, so it's safe
* to start shutting down within-transaction services. But note that most
@@ -1991,6 +1989,9 @@ CommitTransaction(void)
* the transaction-abort path.
*/
+ CallXactCallbacks(is_parallel_worker ? XACT_EVENT_PARALLEL_PRE_COMMIT
+ : XACT_EVENT_PRE_COMMIT);
+
/* If we might have parallel workers, clean them up now. */
if (IsInParallelMode())
AtEOXact_Parallel(true);
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 46369cf3db..3d01a782da 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -27,6 +27,7 @@
#include "commands/portalcmds.h"
#include "executor/executor.h"
#include "executor/tstoreReceiver.h"
+#include "miscadmin.h"
#include "rewrite/rewriteHandler.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
@@ -64,6 +65,10 @@ PerformCursorOpen(DeclareCursorStmt *cstmt, ParamListInfo params,
*/
if (!(cstmt->options & CURSOR_OPT_HOLD))
RequireTransactionChain(isTopLevel, "DECLARE CURSOR");
+ else if (InSecurityRestrictedOperation())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("cannot create a cursor WITH HOLD within security-restricted operation")));
/*
* Parse analysis was done already, but we still have to run the rule
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 2886aebef4..896cb20051 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -4142,6 +4142,7 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
bool immediate_only)
{
bool found = false;
+ bool deferred_found = false;
AfterTriggerEvent event;
AfterTriggerEventChunk *chunk;
@@ -4177,6 +4178,7 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
*/
if (defer_it && move_list != NULL)
{
+ deferred_found = true;
/* add it to move_list */
afterTriggerAddEvent(move_list, event, evtshared);
/* mark original copy "done" so we don't do it again */
@@ -4184,6 +4186,16 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
}
}
+ /*
+ * We could allow deferred triggers if, before the end of the
+ * security-restricted operation, we were to verify that a SET CONSTRAINTS
+ * ... IMMEDIATE has fired all such triggers. For now, don't bother.
+ */
+ if (deferred_found && InSecurityRestrictedOperation())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("cannot fire deferred trigger within security-restricted operation")));
+
return found;
}
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 65d950f15b..f7f9252a05 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1138,6 +1138,48 @@ SELECT has_table_privilege('regress_user1', 'atest4', 'SELECT WITH GRANT OPTION'
t
(1 row)
+-- security-restricted operations
+\c -
+CREATE ROLE regress_sro_user;
+SET SESSION AUTHORIZATION regress_sro_user;
+CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
+ 'GRANT regress_priv_group2 TO regress_sro_user';
+CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
+ 'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
+-- REFRESH of this MV will queue a GRANT at end of transaction
+CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
+REFRESH MATERIALIZED VIEW sro_mv;
+ERROR: cannot create a cursor WITH HOLD within security-restricted operation
+CONTEXT: SQL function "mv_action" statement 1
+\c -
+REFRESH MATERIALIZED VIEW sro_mv;
+ERROR: cannot create a cursor WITH HOLD within security-restricted operation
+CONTEXT: SQL function "mv_action" statement 1
+SET SESSION AUTHORIZATION regress_sro_user;
+-- INSERT to this table will queue a GRANT at end of transaction
+CREATE TABLE sro_trojan_table ();
+CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
+ 'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
+CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
+ INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
+-- Now, REFRESH will issue such an INSERT, queueing the GRANT
+CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
+ 'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
+REFRESH MATERIALIZED VIEW sro_mv;
+ERROR: cannot fire deferred trigger within security-restricted operation
+CONTEXT: SQL function "mv_action" statement 1
+\c -
+REFRESH MATERIALIZED VIEW sro_mv;
+ERROR: cannot fire deferred trigger within security-restricted operation
+CONTEXT: SQL function "mv_action" statement 1
+BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
+ERROR: must have admin option on role "regress_priv_group2"
+CONTEXT: SQL function "unwanted_grant" statement 1
+SQL statement "SELECT unwanted_grant()"
+PL/pgSQL function sro_trojan() line 1 at PERFORM
+SQL function "mv_action" statement 1
+DROP OWNED BY regress_sro_user;
+DROP ROLE regress_sro_user;
-- Admin options
SET SESSION AUTHORIZATION regress_user4;
CREATE FUNCTION dogrant_ok() RETURNS void LANGUAGE sql SECURITY DEFINER AS
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 902f64c747..baa521bcaf 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -726,6 +726,40 @@ SELECT has_table_privilege('regress_user3', 'atest4', 'SELECT'); -- false
SELECT has_table_privilege('regress_user1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true
+-- security-restricted operations
+\c -
+CREATE ROLE regress_sro_user;
+
+SET SESSION AUTHORIZATION regress_sro_user;
+CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
+ 'GRANT regress_priv_group2 TO regress_sro_user';
+CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
+ 'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
+-- REFRESH of this MV will queue a GRANT at end of transaction
+CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
+REFRESH MATERIALIZED VIEW sro_mv;
+\c -
+REFRESH MATERIALIZED VIEW sro_mv;
+
+SET SESSION AUTHORIZATION regress_sro_user;
+-- INSERT to this table will queue a GRANT at end of transaction
+CREATE TABLE sro_trojan_table ();
+CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
+ 'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
+CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
+ INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
+-- Now, REFRESH will issue such an INSERT, queueing the GRANT
+CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
+ 'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
+REFRESH MATERIALIZED VIEW sro_mv;
+\c -
+REFRESH MATERIALIZED VIEW sro_mv;
+BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
+
+DROP OWNED BY regress_sro_user;
+DROP ROLE regress_sro_user;
+
+
-- Admin options
SET SESSION AUTHORIZATION regress_user4;
--
2.23.0

124
CVE-2020-25696.patch Normal file
View File

@ -0,0 +1,124 @@
From 098fb00799ffb026ff12c64bd21635f963cfc609 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Mon, 9 Nov 2020 07:32:09 -0800
Subject: [PATCH] Ignore attempts to \gset into specially treated variables.
If an interactive psql session used \gset when querying a compromised
server, the attacker could execute arbitrary code as the operating
system account running psql. Using a prefix not found among specially
treated variables, e.g. every lowercase string, precluded the attack.
Fix by issuing a warning and setting no variable for the column in
question. Users wanting the old behavior can use a prefix and then a
meta-command like "\set HISTSIZE :prefix_HISTSIZE". Back-patch to 9.5
(all supported versions).
Reviewed by Robert Haas. Reported by Nick Cleaton.
Security: CVE-2020-25696
---
src/bin/psql/common.c | 7 +++++++
src/bin/psql/variables.c | 26 ++++++++++++++++++++++++++
src/bin/psql/variables.h | 1 +
src/test/regress/expected/psql.out | 4 ++++
src/test/regress/sql/psql.sql | 3 +++
5 files changed, 41 insertions(+)
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a41932ff27..f3d966d7cf 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -878,6 +878,13 @@ StoreQueryTuple(const PGresult *result)
/* concatenate prefix and column name */
varname = psprintf("%s%s", pset.gset_prefix, colname);
+ if (VariableHasHook(pset.vars, varname))
+ {
+ pg_log_warning("attempt to \\gset into specially treated variable \"%s\" ignored",
+ varname);
+ continue;
+ }
+
if (!PQgetisnull(result, 0, i))
value = PQgetvalue(result, 0, i);
else
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 120b25c696..0d28ba9c92 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -360,6 +360,32 @@ SetVariableHooks(VariableSpace space, const char *name,
(void) (*ahook) (current->value);
}
+/*
+ * Return true iff the named variable has substitute and/or assign hook
+ * functions.
+ */
+bool
+VariableHasHook(VariableSpace space, const char *name)
+{
+ struct _variable *current;
+
+ Assert(space);
+ Assert(name);
+
+ for (current = space->next; current; current = current->next)
+ {
+ int cmp = strcmp(current->name, name);
+
+ if (cmp == 0)
+ return (current->substitute_hook != NULL ||
+ current->assign_hook != NULL);
+ if (cmp > 0)
+ break; /* it's not there */
+ }
+
+ return false;
+}
+
/*
* Convenience function to set a variable's value to "on".
*/
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index 02d85b1bc2..8dc5c20ee8 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -90,6 +90,7 @@ bool DeleteVariable(VariableSpace space, const char *name);
void SetVariableHooks(VariableSpace space, const char *name,
VariableSubstituteHook shook,
VariableAssignHook ahook);
+bool VariableHasHook(VariableSpace space, const char *name);
void PsqlVarEnumError(const char *name, const char *value, const char *suggestions);
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 0c94144575..1ae81912c7 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -84,6 +84,10 @@ select 10 as test01, 20 as test02, 'Hello' as test03 \gset pref01_
select 10 as "bad name"
\gset
invalid variable name: "bad name"
+select 97 as "EOF", 'ok' as _foo \gset IGNORE
+attempt to \gset into specially treated variable "IGNOREEOF" ignored
+\echo :IGNORE_foo :IGNOREEOF
+ok 0
-- multiple backslash commands in one line
select 1 as x, 2 as y \gset pref01_ \\ \echo :pref01_x
1
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 4a676c3119..7f8ab2e5c2 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -48,6 +48,9 @@ select 10 as test01, 20 as test02, 'Hello' as test03 \gset pref01_
select 10 as "bad name"
\gset
+select 97 as "EOF", 'ok' as _foo \gset IGNORE
+\echo :IGNORE_foo :IGNOREEOF
+
-- multiple backslash commands in one line
select 1 as x, 2 as y \gset pref01_ \\ \echo :pref01_x
select 3 as x, 4 as y \gset pref01_ \echo :pref01_x \echo :pref01_y
--
2.23.0

70
Fix-error-handling-of-vacuumdb-when-running-out-of-f.patch Normal file
View File

@ -0,0 +1,70 @@
From eec462367ee2b41e02c6e29135c857ad6f2da66a Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 26 Aug 2019 11:14:33 +0900
Subject: [PATCH] Fix error handling of vacuumdb when running out of fds
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When trying to use a high number of jobs, vacuumdb has only checked for
a maximum number of jobs used, causing confusing failures when running
out of file descriptors when the jobs open connections to Postgres.
This commit changes the error handling so as we do not check anymore for
a maximum number of allowed jobs when parsing the option value with
FD_SETSIZE, but check instead if a file descriptor is within the
supported range when opening the connections for the jobs so as this is
detected at the earliest time possible.
Also, improve the error message to give a hint about the number of jobs
recommended, using a wording given by the reviewers of the patch.
Reported-by: Andres Freund
Author: Michael Paquier
Reviewed-by: Andres Freund, Álvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/20190818001858.ho3ev4z57fqhs7a5@alap3.anarazel.de
Backpatch-through: 9.5
---
src/bin/scripts/vacuumdb.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index f888bf73bc..4ac765244a 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -200,12 +200,6 @@ main(int argc, char *argv[])
progname);
exit(1);
}
- if (concurrentCons > FD_SETSIZE - 1)
- {
- fprintf(stderr, _("%s: too many parallel jobs requested (maximum: %d)\n"),
- progname, FD_SETSIZE - 1);
- exit(1);
- }
break;
case 2:
maintenance_db = pg_strdup(optarg);
@@ -443,6 +437,20 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
{
conn = connectDatabase(dbname, host, port, username, prompt_password,
progname, echo, false, true);
+
+ /*
+ * Fail and exit immediately if trying to use a socket in an
+ * unsupported range. POSIX requires open(2) to use the lowest
+ * unused file descriptor and the hint given relies on that.
+ */
+ if (PQsocket(conn) >= FD_SETSIZE)
+ {
+ fprintf(stderr,
+ _("%s: too many jobs for this platform -- try %d"),
+ progname, i);
+ exit(1);
+ }
+
init_slot(slots + i, conn);
}
}
--
2.23.0

50
Remove-some-code-related-to-7.3-and-older-servers-fr.patch Normal file
View File

@ -0,0 +1,50 @@
From 57ba539775f1dd0b0460f1dfe673da00eeef3a2f Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 7 May 2019 14:20:01 +0900
Subject: [PATCH] Remove some code related to 7.3 and older servers from tools
of src/bin/
This code was broken as of 582edc3, and is most likely not used anymore.
Note that pg_dump supports servers down to 8.0, and psql has code to
support servers down to 7.4.
Author: Julien Rouhaud
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAOBaU_Y5y=zo3+2gf+2NJC1pvMYPcbRXoQaPXx=U7+C8Qh4CzQ@mail.gmail.com
---
src/bin/scripts/common.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index 8073ee0d0e..5088c4c48e 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -145,9 +145,8 @@ connectDatabase(const char *dbname, const char *pghost,
exit(1);
}
- if (PQserverVersion(conn) >= 70300)
- PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL,
- progname, echo));
+ PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL,
+ progname, echo));
return conn;
}
@@ -311,13 +310,6 @@ appendQualifiedRelation(PQExpBuffer buf, const char *spec,
PGresult *res;
int ntups;
- /* Before 7.3, the concept of qualifying a name did not exist. */
- if (PQserverVersion(conn) < 70300)
- {
- appendPQExpBufferStr(&sql, spec);
- return;
- }
-
split_table_columns_spec(spec, PQclientEncoding(conn), &table, &columns);
/*
--
2.23.0

76
createdb-Fix-quoting-of-encoding-lc-ctype-and-lc-col.patch Normal file
View File

@ -0,0 +1,76 @@
From a1413123f80f470da1ec422592f228aebe4a8866 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 27 Feb 2020 11:21:14 +0900
Subject: [PATCH 1/2] createdb: Fix quoting of --encoding, --lc-ctype and
--lc-collate
The original coding failed to properly quote those arguments, leading to
failures when using quotes in the values used. As the quoting can be
encoding-sensitive, the connection to the backend needs to be taken
before applying the correct quoting.
Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20200214041004.GB1998@paquier.xyz
Backpatch-through: 9.5
---
src/bin/scripts/createdb.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/bin/scripts/createdb.c b/src/bin/scripts/createdb.c
index 8116b084ff..45d26ecb8c 100644
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -177,6 +177,13 @@ main(int argc, char *argv[])
dbname = get_user_name_or_exit(progname);
}
+ /* No point in trying to use postgres db when creating postgres db. */
+ if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
+ maintenance_db = "template1";
+
+ conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
+ prompt_password, progname, echo);
+
initPQExpBuffer(&sql);
appendPQExpBuffer(&sql, "CREATE DATABASE %s",
@@ -187,23 +194,25 @@ main(int argc, char *argv[])
if (tablespace)
appendPQExpBuffer(&sql, " TABLESPACE %s", fmtId(tablespace));
if (encoding)
- appendPQExpBuffer(&sql, " ENCODING '%s'", encoding);
+ {
+ appendPQExpBufferStr(&sql, " ENCODING ");
+ appendStringLiteralConn(&sql, encoding, conn);
+ }
if (template)
appendPQExpBuffer(&sql, " TEMPLATE %s", fmtId(template));
if (lc_collate)
- appendPQExpBuffer(&sql, " LC_COLLATE '%s'", lc_collate);
+ {
+ appendPQExpBufferStr(&sql, " LC_COLLATE ");
+ appendStringLiteralConn(&sql, lc_collate, conn);
+ }
if (lc_ctype)
- appendPQExpBuffer(&sql, " LC_CTYPE '%s'", lc_ctype);
+ {
+ appendPQExpBufferStr(&sql, " LC_CTYPE ");
+ appendStringLiteralConn(&sql, lc_ctype, conn);
+ }
appendPQExpBufferChar(&sql, ';');
- /* No point in trying to use postgres db when creating postgres db. */
- if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
- maintenance_db = "template1";
-
- conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
- prompt_password, progname, echo);
-
if (echo)
printf("%s\n", sql.data);
result = PQexec(conn, sql.data);
--
2.23.0

13
postgresql.spec
View File

@ -4,7 +4,7 @@
Name: postgresql
Version: 10.5
Release: 17
Release: 18
Summary: PostgreSQL client programs
License: PostgreSQL
URL: http://www.postgresql.org/
@ -34,6 +34,14 @@ Patch8: 0008-CVE-2020-1720.patch
Patch9: 0009-CVE-2020-14349-1.patch
Patch10: 0010-CVE-2020-14349-2.patch
Patch11: 0011-CVE-2020-14350.patch
Patch12: Fix-error-handling-of-vacuumdb-when-running-out-of-f.patch
Patch13: Remove-some-code-related-to-7.3-and-older-servers-fr.patch
Patch14: createdb-Fix-quoting-of-encoding-lc-ctype-and-lc-col.patch
Patch15: CVE-2020-25694-1.patch
Patch16: CVE-2020-25694-2.patch
Patch17: CVE-2020-25694-3.patch
Patch18: CVE-2020-25695.patch
Patch19: CVE-2020-25696.patch
BuildRequires: gcc perl(ExtUtils::MakeMaker) glibc-devel bison flex gawk perl(ExtUtils::Embed)
BuildRequires: perl-devel perl-generators readline-devel zlib-devel systemd systemd-devel
@ -435,6 +443,9 @@ find_lang_bins pltcl.lst pltcl
%attr(-,postgres,postgres) %{_libdir}/pgsql/test
%changelog
* Tue Dec 8 2020 wangxiao <wangxiao65@huawei.com> - 10.5-18
- Fix CVE-2020-25694 CVE-2020-25695 CVE-2020-25696
* Web Sep 9 2020 yanglongkang<yanglongkang@huawei.com> - 10.5-17
- Fix CVE-2020-14349 CVE-2020-14350