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

update to 10.23

Browse Source 
CVE-2023-2454 and CVE-2023-2455
This commit is contained in:
dillon_chen 2023-11-17 18:04:43 +08:00
parent 1e774b6a90
commit 02e0e51f1d
26 changed files with 375 additions and 5388 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

108
6000-CVE-2019-10164-1.patch
View File

@ -1,108 +0,0 @@
From 90adc16ea13750a6b6f704c6cf65dc0f1bdb845c Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 17 Jun 2019 21:48:34 +0900
Subject: [PATCH] Fix buffer overflow when parsing SCRAM verifiers in backend
Any authenticated user can overflow a stack-based buffer by changing the
user's own password to a purpose-crafted value. This often suffices to
execute arbitrary code as the PostgreSQL operating system account.
This fix is contributed by multiple folks, based on an initial analysis
from Tom Lane. This issue has been introduced by 68e61ee, so it was
possible to make use of it at authentication time. It became more
easily to trigger after ccae190 which has made the SCRAM parsing more
strict when changing a password, in the case where the client passes
down a verifier already hashed using SCRAM. Back-patch to v10 where
SCRAM has been introduced.
Reported-by: Alexander Lakhin
Author: Jonathan Katz, Heikki Linnakangas, Michael Paquier
Security: CVE-2019-10164
Backpatch-through: 10
---
src/backend/libpq/auth-scram.c | 35 ++++++++++++++++++++++++++--------
src/test/regress/expected/password.out | 23 ++++++++++++++++++++++
src/test/regress/sql/password.sql | 18 +++++++++++++++++
3 files changed, 68 insertions(+), 8 deletions(-)
diff -Nurp postgresql-10.5/src/backend/libpq/auth-scram.c postgresql-10.5-bak/src/backend/libpq/auth-scram.c
--- postgresql-10.5/src/backend/libpq/auth-scram.c 2018-08-06 16:05:31.000000000 -0400
+++ postgresql-10.5-bak/src/backend/libpq/auth-scram.c 2019-08-01 10:03:08.505000000 -0400
@@ -474,6 +474,12 @@ scram_verify_plain_password(const char *
/*
* Parse and validate format of given SCRAM verifier.
*
+ * On success, the iteration count, salt, stored key, and server key are
+ * extracted from the verifier, and returned to the caller. For 'stored_key'
+ * and 'server_key', the caller must pass pre-allocated buffers of size
+ * SCRAM_KEY_LEN. Salt is returned as a base64-encoded, null-terminated
+ * string. The buffer for the salt is palloc'd by this function.
+ *
* Returns true if the SCRAM verifier has been parsed, and false otherwise.
*/
static bool
@@ -489,6 +495,8 @@ parse_scram_verifier(const char *verifie
char *serverkey_str;
int decoded_len;
char *decoded_salt_buf;
+ char *decoded_stored_buf;
+ char *decoded_server_buf;
/*
* The verifier is of form:
@@ -521,7 +529,8 @@ parse_scram_verifier(const char *verifie
* although we return the encoded version to the caller.
*/
decoded_salt_buf = palloc(pg_b64_dec_len(strlen(salt_str)));
- decoded_len = pg_b64_decode(salt_str, strlen(salt_str), decoded_salt_buf);
+ decoded_len = pg_b64_decode(salt_str, strlen(salt_str),
+ decoded_salt_buf);
if (decoded_len < 0)
goto invalid_verifier;
*salt = pstrdup(salt_str);
@@ -529,28 +538,38 @@ parse_scram_verifier(const char *verifie
/*
* Decode StoredKey and ServerKey.
*/
- if (pg_b64_dec_len(strlen(storedkey_str) != SCRAM_KEY_LEN))
- goto invalid_verifier;
+ decoded_stored_buf = palloc(pg_b64_dec_len(strlen(storedkey_str)));
decoded_len = pg_b64_decode(storedkey_str, strlen(storedkey_str),
- (char *) stored_key);
+ decoded_stored_buf);
if (decoded_len != SCRAM_KEY_LEN)
goto invalid_verifier;
+ memcpy(stored_key, decoded_stored_buf, SCRAM_KEY_LEN);
- if (pg_b64_dec_len(strlen(serverkey_str) != SCRAM_KEY_LEN))
- goto invalid_verifier;
+ decoded_server_buf = palloc(pg_b64_dec_len(strlen(serverkey_str)));
decoded_len = pg_b64_decode(serverkey_str, strlen(serverkey_str),
- (char *) server_key);
+ decoded_server_buf);
if (decoded_len != SCRAM_KEY_LEN)
goto invalid_verifier;
+ memcpy(server_key, decoded_server_buf, SCRAM_KEY_LEN);
return true;
invalid_verifier:
- pfree(v);
*salt = NULL;
return false;
}
+/*
+ * Generate plausible SCRAM verifier parameters for mock authentication.
+ *
+ * In a normal authentication, these are extracted from the verifier
+ * stored in the server. This function generates values that look
+ * realistic, for when there is no stored verifier.
+ *
+ * Like in parse_scram_verifier(), for 'stored_key' and 'server_key', the
+ * caller must pass pre-allocated buffers of size SCRAM_KEY_LEN, and
+ * the buffer for the salt is palloc'd by this function.
+ */
static void
mock_scram_verifier(const char *username, int *iterations, char **salt,
uint8 *stored_key, uint8 *server_key)

77
6001-CVE-2019-10164-2.patch
View File

@ -1,77 +0,0 @@
From d72a7e4da1001b29a661a4b1a52cb5c4d708bab0 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 17 Jun 2019 22:14:09 +0900
Subject: [PATCH] Fix buffer overflow when processing SCRAM final message in
libpq
When a client connects to a rogue server sending specifically-crafted
messages, this can suffice to execute arbitrary code as the operating
system account used by the client.
While on it, fix one error handling when decoding an incorrect salt
included in the first message received from server.
Author: Michael Paquier
Reviewed-by: Jonathan Katz, Heikki Linnakangas
Security: CVE-2019-10164
Backpatch-through: 10
---
src/interfaces/libpq/fe-auth-scram.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 7fa7f34c80..4cdf9ba93b 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -462,6 +462,12 @@ read_server_first_message(fe_scram_state *state, char *input,
state->saltlen = pg_b64_decode(encoded_salt,
strlen(encoded_salt),
state->salt);
+ if (state->saltlen < 0)
+ {
+ printfPQExpBuffer(errormessage,
+ libpq_gettext("malformed SCRAM message (invalid salt)\n"));
+ return false;
+ }
iterations_str = read_attr_value(&input, 'i', errormessage);
if (iterations_str == NULL)
@@ -492,6 +498,7 @@ read_server_final_message(fe_scram_state *state, char *input,
PQExpBuffer errormessage)
{
char *encoded_server_signature;
+ char *decoded_server_signature;
int server_signature_len;
state->server_final_message = strdup(input);
@@ -525,15 +532,27 @@ read_server_final_message(fe_scram_state *state, char *input,
printfPQExpBuffer(errormessage,
libpq_gettext("malformed SCRAM message (garbage at end of server-final-message)\n"));
+ server_signature_len = pg_b64_dec_len(strlen(encoded_server_signature));
+ decoded_server_signature = malloc(server_signature_len);
+ if (!decoded_server_signature)
+ {
+ printfPQExpBuffer(errormessage,
+ libpq_gettext("out of memory\n"));
+ return false;
+ }
+
server_signature_len = pg_b64_decode(encoded_server_signature,
strlen(encoded_server_signature),
- state->ServerSignature);
+ decoded_server_signature);
if (server_signature_len != SCRAM_KEY_LEN)
{
+ free(decoded_server_signature);
printfPQExpBuffer(errormessage,
libpq_gettext("malformed SCRAM message (invalid server signature)\n"));
return false;
}
+ memcpy(state->ServerSignature, decoded_server_signature, SCRAM_KEY_LEN);
+ free(decoded_server_signature);
return true;
}
--
2.11.0

44
CVE-2018-16850.patch
View File

@ -1,44 +0,0 @@
From 21a134a26100b377191735ad49f996eac03047de Mon Sep 17 00:00:00 2001
From: guiyao <guiyao@huawei.com>
Date: Thu, 17 Dec 2020 09:34:13 -0500
Subject: [PATCH] backport to fix CVE-2018-16850
---
src/backend/utils/adt/ruleutils.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 4c9a49c..0eea491 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -952,22 +952,24 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
value = fastgetattr(ht_trig, Anum_pg_trigger_tgoldtable,
tgrel->rd_att, &isnull);
if (!isnull)
- tgoldtable = NameStr(*((NameData *) DatumGetPointer(value)));
+ tgoldtable = NameStr(*DatumGetName(value));
else
tgoldtable = NULL;
value = fastgetattr(ht_trig, Anum_pg_trigger_tgnewtable,
tgrel->rd_att, &isnull);
if (!isnull)
- tgnewtable = NameStr(*((NameData *) DatumGetPointer(value)));
+ tgnewtable = NameStr(*DatumGetName(value));
else
tgnewtable = NULL;
if (tgoldtable != NULL || tgnewtable != NULL)
{
appendStringInfoString(&buf, "REFERENCING ");
if (tgoldtable != NULL)
- appendStringInfo(&buf, "OLD TABLE AS %s ", tgoldtable);
+ appendStringInfo(&buf, "OLD TABLE AS %s ",
+ quote_identifier(tgoldtable));
if (tgnewtable != NULL)
- appendStringInfo(&buf, "NEW TABLE AS %s ", tgnewtable);
+ appendStringInfo(&buf, "NEW TABLE AS %s ",
+ quote_identifier(tgnewtable));
}
if (TRIGGER_FOR_ROW(trigrec->tgtype))
--
1.8.3.1

75
CVE-2019-10130.patch
View File

@ -1,75 +0,0 @@
From 1aebfbea83c4a3e1a0aba4b0910135dc5a45666c Mon Sep 17 00:00:00 2001
From: Dean Rasheed <dean.a.rasheed@gmail.com>
Date: Mon, 6 May 2019 11:38:43 +0100
Subject: [PATCH] Fix security checks for selectivity estimation functions with
RLS.
In commit e2d4ef8de8, security checks were added to prevent
user-supplied operators from running over data from pg_statistic
unless the user has table or column privileges on the table, or the
operator is leakproof. For a table with RLS, however, checking for
table or column privileges is insufficient, since that does not
guarantee that the user has permission to view all of the column's
data.
Fix this by also checking for securityQuals on the RTE, and insisting
that the operator be leakproof if there are any. Thus the
leakproofness check will only be skipped if there are no securityQuals
and the user has table or column privileges on the table -- i.e., only
if we know that the user has access to all the data in the column.
Back-patch to 9.5 where RLS was added.
Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost.
Security: CVE-2019-10130
---
src/backend/utils/adt/selfuncs.c | 21 +++++++++++++++------
src/test/regress/expected/rowsecurity.out | 21 +++++++++++++++++++++
src/test/regress/sql/rowsecurity.sql | 20 ++++++++++++++++++++
3 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index b41991315520..514612857ad6 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -4597,9 +4597,13 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
* For simplicity, we insist on the whole
* table being selectable, rather than trying
* to identify which column(s) the index
- * depends on.
+ * depends on. Also require all rows to be
+ * selectable --- there must be no
+ * securityQuals from security barrier views
+ * or RLS policies.
*/
vardata->acl_ok =
+ rte->securityQuals == NIL &&
(pg_class_aclcheck(rte->relid, GetUserId(),
ACL_SELECT) == ACLCHECK_OK);
}
@@ -4663,12 +4667,17 @@ examine_simple_variable(PlannerInfo *root, Var *var,
if (HeapTupleIsValid(vardata->statsTuple))
{
- /* check if user has permission to read this column */
+ /*
+ * Check if user has permission to read this column. We require
+ * all rows to be accessible, so there must be no securityQuals
+ * from security barrier views or RLS policies.
+ */
vardata->acl_ok =
- (pg_class_aclcheck(rte->relid, GetUserId(),
- ACL_SELECT) == ACLCHECK_OK) ||
- (pg_attribute_aclcheck(rte->relid, var->varattno, GetUserId(),
- ACL_SELECT) == ACLCHECK_OK);
+ rte->securityQuals == NIL &&
+ ((pg_class_aclcheck(rte->relid, GetUserId(),
+ ACL_SELECT) == ACLCHECK_OK) ||
+ (pg_attribute_aclcheck(rte->relid, var->varattno, GetUserId(),
+ ACL_SELECT) == ACLCHECK_OK));
}
else
{
--
2.11.0

224
CVE-2019-10208.patch
View File

@ -1,224 +0,0 @@
From 2062007cbf7008de383c8f2f4a9ad466ea243acf Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Mon, 5 Aug 2019 07:48:41 -0700
Subject: [PATCH] Require the schema qualification in pg_temp.type_name(arg).
Commit aa27977fe21a7dfa4da4376ad66ae37cb8f0d0b5 introduced this
restriction for pg_temp.function_name(arg); do likewise for types
created in temporary schemas. Programs that this breaks should add
"pg_temp." schema qualification or switch to arg::type_name syntax.
Back-patch to 9.4 (all supported versions).
Reviewed by Tom Lane. Reported by Tom Lane.
Security: CVE-2019-10208
---
src/backend/catalog/namespace.c | 15 ++++++++++++++-
src/backend/parser/parse_func.c | 7 ++++++-
src/backend/parser/parse_type.c | 24 +++++++++++++++++++++---
src/backend/utils/adt/ruleutils.c | 8 ++++++++
src/include/catalog/namespace.h | 1 +
src/include/parser/parse_type.h | 3 +++
src/test/regress/expected/temp.out | 15 +++++++++++++++
src/test/regress/sql/temp.sql | 12 ++++++++++++
8 files changed, 79 insertions(+), 5 deletions(-)
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 5a737c8ab3..6b75ce10da 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -739,13 +739,23 @@ RelationIsVisible(Oid relid)
/*
* TypenameGetTypid
+ * Wrapper for binary compatibility.
+ */
+Oid
+TypenameGetTypid(const char *typname)
+{
+ return TypenameGetTypidExtended(typname, true);
+}
+
+/*
+ * TypenameGetTypidExtended
* Try to resolve an unqualified datatype name.
* Returns OID if type found in search path, else InvalidOid.
*
* This is essentially the same as RelnameGetRelid.
*/
Oid
-TypenameGetTypid(const char *typname)
+TypenameGetTypidExtended(const char *typname, bool temp_ok)
{
Oid typid;
ListCell *l;
@@ -756,6 +766,9 @@ TypenameGetTypid(const char *typname)
{
Oid namespaceId = lfirst_oid(l);
+ if (!temp_ok && namespaceId == myTempNamespace)
+ continue; /* do not look in temp namespace */
+
typid = GetSysCacheOid2(TYPENAMENSP,
PointerGetDatum(typname),
ObjectIdGetDatum(namespaceId));
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 25ae4e5949..571d3d1b62 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -1769,7 +1769,12 @@ FuncNameAsType(List *funcname)
Oid result;
Type typtup;
- typtup = LookupTypeName(NULL, makeTypeNameFromNameList(funcname), NULL, false);
+ /*
+ * temp_ok=false protects the <refsect1 id="sql-createfunction-security">
+ * contract for writing SECURITY DEFINER functions safely.
+ */
+ typtup = LookupTypeNameExtended(NULL, makeTypeNameFromNameList(funcname),
+ NULL, false, false);
if (typtup == NULL)
return InvalidOid;
diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c
index d0b3fbeb57..1171381487 100644
--- a/src/backend/parser/parse_type.c
+++ b/src/backend/parser/parse_type.c
@@ -33,6 +33,18 @@ static int32 typenameTypeMod(ParseState *pstate, const TypeName *typeName,
/*
* LookupTypeName
+ * Wrapper for typical case.
+ */
+Type
+LookupTypeName(ParseState *pstate, const TypeName *typeName,
+ int32 *typmod_p, bool missing_ok)
+{
+ return LookupTypeNameExtended(pstate,
+ typeName, typmod_p, true, missing_ok);
+}
+
+/*
+ * LookupTypeNameExtended
* Given a TypeName object, lookup the pg_type syscache entry of the type.
* Returns NULL if no such type can be found. If the type is found,
* the typmod value represented in the TypeName struct is computed and
@@ -51,11 +63,17 @@ static int32 typenameTypeMod(ParseState *pstate, const TypeName *typeName,
* found but is a shell, and there is typmod decoration, an error will be
* thrown --- this is intentional.
*
+ * If temp_ok is false, ignore types in the temporary namespace. Pass false
+ * when the caller will decide, using goodness of fit criteria, whether the
+ * typeName is actually a type or something else. If typeName always denotes
+ * a type (or denotes nothing), pass true.
+ *
* pstate is only used for error location info, and may be NULL.
*/
Type
-LookupTypeName(ParseState *pstate, const TypeName *typeName,
- int32 *typmod_p, bool missing_ok)
+LookupTypeNameExtended(ParseState *pstate,
+ const TypeName *typeName, int32 *typmod_p,
+ bool temp_ok, bool missing_ok)
{
Oid typoid;
HeapTuple tup;
@@ -172,7 +190,7 @@ LookupTypeName(ParseState *pstate, const TypeName *typeName,
else
{
/* Unqualified type name, so search the search path */
- typoid = TypenameGetTypid(typname);
+ typoid = TypenameGetTypidExtended(typname, temp_ok);
}
/* If an array reference, return the array type instead */
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3d1a701775..01789291bb 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9273,6 +9273,14 @@ get_coercion_expr(Node *arg, deparse_context *context,
if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, ')');
}
+
+ /*
+ * Never emit resulttype(arg) functional notation. A pg_proc entry could
+ * take precedence, and a resulttype in pg_temp would require schema
+ * qualification that format_type_with_typemod() would usually omit. We've
+ * standardized on arg::resulttype, but CAST(arg AS resulttype) notation
+ * would work fine.
+ */
appendStringInfo(buf, "::%s",
format_type_with_typemod(resulttype, resulttypmod));
}
diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index f2ee935623..59c5ea5be2 100644
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -66,6 +66,7 @@ extern Oid RelnameGetRelid(const char *relname);
extern bool RelationIsVisible(Oid relid);
extern Oid TypenameGetTypid(const char *typname);
+extern Oid TypenameGetTypidExtended(const char *typname, bool temp_ok);
extern bool TypeIsVisible(Oid typid);
extern FuncCandidateList FuncnameGetCandidates(List *names,
diff --git a/src/include/parser/parse_type.h b/src/include/parser/parse_type.h
index 7b843d0b9d..ac1bf1009e 100644
--- a/src/include/parser/parse_type.h
+++ b/src/include/parser/parse_type.h
@@ -21,6 +21,9 @@ typedef HeapTuple Type;
extern Type LookupTypeName(ParseState *pstate, const TypeName *typeName,
int32 *typmod_p, bool missing_ok);
+extern Type LookupTypeNameExtended(ParseState *pstate,
+ const TypeName *typeName, int32 *typmod_p,
+ bool temp_ok, bool missing_ok);
extern Oid LookupTypeNameOid(ParseState *pstate, const TypeName *typeName,
bool missing_ok);
extern Type typenameType(ParseState *pstate, const TypeName *typeName,
diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out
index 7046deb0c8..aad562791e 100644
--- a/src/test/regress/expected/temp.out
+++ b/src/test/regress/expected/temp.out
@@ -199,3 +199,18 @@ select pg_temp.whoami();
(1 row)
drop table public.whereami;
+-- types in temp schema
+set search_path = pg_temp, public;
+create domain pg_temp.nonempty as text check (value <> '');
+-- function-syntax invocation of types matches rules for functions
+select nonempty('');
+ERROR: function nonempty(unknown) does not exist
+LINE 1: select nonempty('');
+ ^
+HINT: No function matches the given name and argument types. You might need to add explicit type casts.
+select pg_temp.nonempty('');
+ERROR: value for domain nonempty violates check constraint "nonempty_check"
+-- other syntax matches rules for tables
+select ''::nonempty;
+ERROR: value for domain nonempty violates check constraint "nonempty_check"
+reset search_path;
diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql
index d69a243fe7..c14c11444c 100644
--- a/src/test/regress/sql/temp.sql
+++ b/src/test/regress/sql/temp.sql
@@ -151,3 +151,15 @@ select pg_temp.whoami();
select pg_temp.whoami();
drop table public.whereami;
+
+-- types in temp schema
+set search_path = pg_temp, public;
+create domain pg_temp.nonempty as text check (value <> '');
+-- function-syntax invocation of types matches rules for functions
+select nonempty('');
+select pg_temp.nonempty('');
+-- other syntax matches rules for tables
+select ''::nonempty;
+
+reset search_path;
+
--
2.11.0

96
CVE-2020-14349-1.patch
View File

@ -1,96 +0,0 @@
From 11da97024abbe76b8c81e3f2375b2a62e9717c67 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Mon, 10 Aug 2020 09:22:54 -0700
Subject: [PATCH] Empty search_path in logical replication apply worker and
walsender.
This is like CVE-2018-1058 commit
582edc369cdbd348d68441fc50fa26a84afd0c1a. Today, a malicious user of a
publisher or subscriber database can invoke arbitrary SQL functions
under an identity running replication, often a superuser. This fix may
cause "does not exist" or "no schema has been selected to create in"
errors in a replication process. After upgrading, consider watching
server logs for these errors. Objects accruing schema qualification in
the wake of the earlier commit are unlikely to need further correction.
Back-patch to v10, which introduced logical replication.
Security: CVE-2020-14349
reason: fix CVE-2020-14349
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=11da97024abbe76b8c81e3f2375b2a62e9717c67
Signed-off-by: Noah Misch <noah@leadboat.com>
---
.../libpqwalreceiver/libpqwalreceiver.c | 17 +++++++++++++++++
src/backend/replication/logical/worker.c | 6 ++++++
src/test/subscription/t/001_rep_changes.pl | 4 ++++
3 files changed, 27 insertions(+)
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 37b481c..564e6d3 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -23,6 +23,7 @@
#include "pqexpbuffer.h"
#include "access/xlog.h"
#include "catalog/pg_type.h"
+#include "fe_utils/connect.h"
#include "funcapi.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
@@ -210,6 +211,22 @@ libpqrcv_connect(const char *conninfo, bool logical, const char *appname,
return NULL;
}
+ if (logical)
+ {
+ PGresult *res;
+
+ res = libpqrcv_PQexec(conn->streamConn,
+ ALWAYS_SECURE_SEARCH_PATH_SQL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ PQclear(res);
+ ereport(ERROR,
+ (errmsg("could not clear search path: %s",
+ pchomp(PQerrorMessage(conn->streamConn)))));
+ }
+ PQclear(res);
+ }
+
conn->logical = logical;
return conn;
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index bd60094..07b765a 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1548,6 +1548,12 @@ ApplyWorkerMain(Datum main_arg)
BackgroundWorkerInitializeConnectionByOid(MyLogicalRepWorker->dbid,
MyLogicalRepWorker->userid);
+ /*
+ * Set always-secure search path, so malicious users can't redirect user
+ * code (e.g. pg_index.indexprs).
+ */
+ SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
+
/* Load the subscription into persistent memory context. */
ApplyContext = AllocSetContextCreate(TopMemoryContext,
"ApplyContext",
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index 0136c79..cda275b 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -16,6 +16,10 @@ $node_subscriber->init(allows_streaming => 'logical');
$node_subscriber->start;
# Create some preexisting content on publisher
+$node_publisher->safe_psql(
+ 'postgres',
+ "CREATE FUNCTION public.pg_get_replica_identity_index(int)
+ RETURNS regclass LANGUAGE sql AS 'SELECT 1/0'"); # shall not call
$node_publisher->safe_psql('postgres',
"CREATE TABLE tab_notrep AS SELECT generate_series(1,10) AS a");
$node_publisher->safe_psql('postgres',
--
2.23.0

54
CVE-2020-14349-2.patch
View File

@ -1,54 +0,0 @@
From cec57b1a0fbcd3833086ba686897c5883e0a2afc Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Mon, 10 Aug 2020 09:22:54 -0700
Subject: [PATCH] Document clashes between logical replication and untrusted
users.
Back-patch to v10, which introduced logical replication.
Security: CVE-2020-14349
reason: fix CVE-2020-14349
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=cec57b1a0fbcd3833086ba686897c5883e0a2afc
Signed-off-by: Noah Misch <noah@leadboat.com>
---
doc/src/sgml/logical-replication.sgml | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 41770a4..f5086b2 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -490,11 +490,27 @@
<sect1 id="logical-replication-security">
<title>Security</title>
+ <para>
+ A user able to modify the schema of subscriber-side tables can execute
+ arbitrary code as a superuser. Limit ownership
+ and <literal>TRIGGER</literal> privilege on such tables to roles that
+ superusers trust. Moreover, if untrusted users can create tables, use only
+ publications that list tables explicitly. That is to say, create a
+ subscription <literal>FOR ALL TABLES</literal> only when superusers trust
+ every user permitted to create a non-temp table on the publisher or the
+ subscriber.
+ </para>
+
<para>
The role used for the replication connection must have
- the <literal>REPLICATION</literal> attribute (or be a superuser). Access for the role must be
- configured in <filename>pg_hba.conf</filename> and it must have the
- <literal>LOGIN</literal> attribute.
+ the <literal>REPLICATION</literal> attribute (or be a superuser). If the
+ role lacks <literal>SUPERUSER</literal> and <literal>BYPASSRLS</literal>,
+ publisher row security policies can execute. If the role does not trust
+ all table owners, include <literal>options=-crow_security=off</literal> in
+ the connection string; if a table owner then adds a row security policy,
+ that setting will cause replication to halt rather than execute the policy.
+ Access for the role must be configured in <filename>pg_hba.conf</filename>
+ and it must have the <literal>LOGIN</literal> attribute.
</para>
<para>
--
2.23.0

1199
CVE-2020-14350.patch
View File

File diff suppressed because it is too large Load Diff

42
CVE-2020-1720.patch
View File

@ -1,42 +0,0 @@
From b048f558dd7c26a0c630a2cff29d3d8981eaf6b9 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 10 Feb 2020 11:47:09 -0300
Subject: [PATCH] Fix priv checks for ALTER <object> DEPENDS ON EXTENSION
Marking an object as dependant on an extension did not have any
privilege check whatsoever; this allowed any user to mark objects as
droppable by anyone able to DROP EXTENSION, which could be used to cause
system-wide havoc. Disallow by checking that the calling user owns the
mentioned object.
(No constraints are placed on the extension.)
Security: CVE-2020-1720
Reported-by: Tom Lane
Discussion: 31605.1566429043@sss.pgh.pa.us
---
src/backend/commands/alter.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index fca85ba2c17f..1cb84182b05f 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -430,6 +430,17 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
get_object_address_rv(stmt->objectType, stmt->relation, (List *) stmt->object,
&rel, AccessExclusiveLock, false);
+ /*
+ * Verify that the user is entitled to run the command.
+ *
+ * We don't check any privileges on the extension, because that's not
+ * needed. The object owner is stipulating, by running this command, that
+ * the extension owner can drop the object whenever they feel like it,
+ * which is not considered a problem.
+ */
+ check_object_ownership(GetUserId(),
+ stmt->objectType, address, stmt->object, rel);
+
/*
* If a relation was involved, it would have been opened and locked. We
* don't need the relation here, but we'll retain the lock until commit.

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

@ -1,492 +0,0 @@
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
View File

File diff suppressed because it is too large Load Diff

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

@ -1,782 +0,0 @@
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

241
CVE-2020-25695.patch
View File

@ -1,241 +0,0 @@
From f97ecea1ed7b09c6f1398540a1d72a57eee70c9f 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 885bd075798c..5dcff3d07624 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 37f31a2c31ea..00cec50e8402 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1994,9 +1994,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 (;;)
{
@@ -2014,9 +2015,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
@@ -2024,6 +2022,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 46369cf3dbee..3d01a782da06 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 f83840625348..9c04eee48422 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -4144,6 +4144,7 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
bool immediate_only)
{
bool found = false;
+ bool deferred_found = false;
AfterTriggerEvent event;
AfterTriggerEventChunk *chunk;
@@ -4179,6 +4180,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 */
@@ -4186,6 +4188,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 dacc98450514..26ee16a0c370 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1253,6 +1253,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_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_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 4263315a2d87..f979cccea03f 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -761,6 +761,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_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;

121
CVE-2020-25696.patch
View File

@ -1,121 +0,0 @@
From a498db87be103f93856dd515a574b2d67d3c1237 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 d04a7943d6a9..4e342c180a6c 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -894,6 +894,13 @@ StoreQueryTuple(const PGresult *result)
/* concatenate prefix and column name */
varname = psprintf("%s%s", pset.gset_prefix, colname);
+ if (VariableHasHook(pset.vars, varname))
+ {
+ psql_error("attempt to \\gset into specially treated variable \"%s\" ignored\n",
+ 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 120b25c69661..0d28ba9c92bb 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 02d85b1bc2e4..8dc5c20ee8fc 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 0c94144575af..1ae81912c734 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 4a676c311955..7f8ab2e5c218 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

90
CVE-2021-23214.patch
View File

@ -1,90 +0,0 @@
From 9ae0f1112954989e955b4b29e4580216eccfcee4 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 8 Nov 2021 11:01:43 -0500
Subject: [PATCH] Reject extraneous data after SSL or GSS encryption handshake.
The server collects up to a bufferload of data whenever it reads data
from the client socket. When SSL or GSS encryption is requested
during startup, any additional data received with the initial
request message remained in the buffer, and would be treated as
already-decrypted data once the encryption handshake completed.
Thus, a man-in-the-middle with the ability to inject data into the
TCP connection could stuff some cleartext data into the start of
a supposedly encryption-protected database session.
This could be abused to send faked SQL commands to the server,
although that would only work if the server did not demand any
authentication data. (However, a server relying on SSL certificate
authentication might well not do so.)
To fix, throw a protocol-violation error if the internal buffer
is not empty after the encryption handshake.
Our thanks to Jacob Champion for reporting this problem.
Security: CVE-2021-23214
---
src/backend/libpq/pqcomm.c | 12 ++++++++++++
src/backend/postmaster/postmaster.c | 13 +++++++++++++
src/include/libpq/libpq.h | 1 +
3 files changed, 26 insertions(+)
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 4452ea4228cb..31bedac24912 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -1199,6 +1199,18 @@ pq_getstring(StringInfo s)
}
}
+/* --------------------------------
+ * pq_buffer_has_data - is any buffered data available to read?
+ *
+ * This will *not* attempt to read more data.
+ * --------------------------------
+ */
+bool
+pq_buffer_has_data(void)
+{
+ return (PqRecvPointer < PqRecvLength);
+}
+
/* --------------------------------
* pq_startmsgread - begin reading a message from the client.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 586d6a7d3b96..661b2d037f2a 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2061,6 +2061,19 @@ ProcessStartupPacket(Port *port, bool SSLdone)
if (SSLok == 'S' && secure_open_server(port) == -1)
return STATUS_ERROR;
#endif
+
+ /*
+ * At this point we should have no data already buffered. If we do,
+ * it was received before we performed the SSL handshake, so it wasn't
+ * encrypted and indeed may have been injected by a man-in-the-middle.
+ * We report this case to the client.
+ */
+ if (pq_buffer_has_data())
+ ereport(FATAL,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("received unencrypted data after SSL request"),
+ errdetail("This could be either a client-software bug or evidence of an attempted man-in-the-middle attack.")));
+
/* regular startup packet, cancel, etc packet should follow... */
/* but not another SSL negotiation request */
return ProcessStartupPacket(port, true);
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index fd2dd5853ccf..d3cf746de39f 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -70,6 +70,7 @@ extern int pq_getmessage(StringInfo s, int maxlen);
extern int pq_getbyte(void);
extern int pq_peekbyte(void);
extern int pq_getbyte_if_available(unsigned char *c);
+extern bool pq_buffer_has_data(void);
extern int pq_putbytes(const char *s, size_t len);
/*

82
CVE-2021-23222.patch
View File

@ -1,82 +0,0 @@
From e65d9c8cd15a86207f1da387a9c917c93c14ea11 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 8 Nov 2021 11:14:56 -0500
Subject: [PATCH] libpq: reject extraneous data after SSL or GSS encryption
handshake.
libpq collects up to a bufferload of data whenever it reads data from
the socket. When SSL or GSS encryption is requested during startup,
any additional data received with the server's yes-or-no reply
remained in the buffer, and would be treated as already-decrypted data
once the encryption handshake completed. Thus, a man-in-the-middle
with the ability to inject data into the TCP connection could stuff
some cleartext data into the start of a supposedly encryption-protected
database session.
This could probably be abused to inject faked responses to the
client's first few queries, although other details of libpq's behavior
make that harder than it sounds. A different line of attack is to
exfiltrate the client's password, or other sensitive data that might
be sent early in the session. That has been shown to be possible with
a server vulnerable to CVE-2021-23214.
To fix, throw a protocol-violation error if the internal buffer
is not empty after the encryption handshake.
Our thanks to Jacob Champion for reporting this problem.
Security: CVE-2021-23222
---
doc/src/sgml/protocol.sgml | 14 ++++++++++++++
src/interfaces/libpq/fe-connect.c | 13 +++++++++++++
2 files changed, 27 insertions(+)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 3a269640fcd6..6a2d4a14fce5 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1348,6 +1348,20 @@
and proceed without requesting <acronym>SSL</acronym>.
</para>
+ <para>
+ When <acronym>SSL</acronym> encryption can be performed, the server
+ is expected to send only the single <literal>S</literal> byte and then
+ wait for the frontend to initiate an <acronym>SSL</acronym> handshake.
+ If additional bytes are available to read at this point, it likely
+ means that a man-in-the-middle is attempting to perform a
+ buffer-stuffing attack
+ (<ulink url="https://www.postgresql.org/support/security/CVE-2021-23222/">CVE-2021-23222</ulink>).
+ Frontends should be coded either to read exactly one byte from the
+ socket before turning the socket over to their SSL library, or to
+ treat it as a protocol violation if they find they have read additional
+ bytes.
+ </para>
+
<para>
An initial SSLRequest can also be used in a connection that is being
opened to send a CancelRequest message.
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 18c09472bed4..03b7cd60d391 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2719,6 +2719,19 @@ PQconnectPoll(PGconn *conn)
pollres = pqsecure_open_client(conn);
if (pollres == PGRES_POLLING_OK)
{
+ /*
+ * At this point we should have no data already buffered.
+ * If we do, it was received before we performed the SSL
+ * handshake, so it wasn't encrypted and indeed may have
+ * been injected by a man-in-the-middle.
+ */
+ if (conn->inCursor != conn->inEnd)
+ {
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("received unencrypted data after SSL response\n"));
+ goto error_return;
+ }
+
/* SSL handshake done, ready to send startup packet */
conn->status = CONNECTION_MADE;
return PGRES_POLLING_WRITING;

364
CVE-2021-32028.patch
View File

@ -1,364 +0,0 @@
From 52a4413627319980843bb8f375f28c7f01c45e18 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 10 May 2021 11:02:30 -0400
Subject: [PATCH] Fix mishandling of resjunk columns in ON CONFLICT ... UPDATE
tlists.
It's unusual to have any resjunk columns in an ON CONFLICT ... UPDATE
list, but it can happen when MULTIEXPR_SUBLINK SubPlans are present.
If it happens, the ON CONFLICT UPDATE code path would end up storing
tuples that include the values of the extra resjunk columns. That's
fairly harmless in the short run, but if new columns are added to
the table then the values would become accessible, possibly leading
to malfunctions if they don't match the datatypes of the new columns.
This had escaped notice through a confluence of missing sanity checks,
including
* There's no cross-check that a tuple presented to heap_insert or
heap_update matches the table rowtype. While it's difficult to
check that fully at reasonable cost, we can easily add assertions
that there aren't too many columns.
* The output-column-assignment cases in execExprInterp.c lacked
any sanity checks on the output column numbers, which seems like
an oversight considering there are plenty of assertion checks on
input column numbers. Add assertions there too.
* We failed to apply nodeModifyTable's ExecCheckPlanOutput() to
the ON CONFLICT UPDATE tlist. That wouldn't have caught this
specific error, since that function is chartered to ignore resjunk
columns; but it sure seems like a bad omission now that we've seen
this bug.
In HEAD, the right way to fix this is to make the processing of
ON CONFLICT UPDATE tlists work the same as regular UPDATE tlists
now do, that is don't add "SET x = x" entries, and use
ExecBuildUpdateProjection to evaluate the tlist and combine it with
old values of the not-set columns. This adds a little complication
to ExecBuildUpdateProjection, but allows removal of a comparable
amount of now-dead code from the planner.
In the back branches, the most expedient solution seems to be to
(a) use an output slot for the ON CONFLICT UPDATE projection that
actually matches the target table, and then (b) invent a variant of
ExecBuildProjectionInfo that can be told to not store values resulting
from resjunk columns, so it doesn't try to store into nonexistent
columns of the output slot. (We can't simply ignore the resjunk columns
altogether; they have to be evaluated for MULTIEXPR_SUBLINK to work.)
This works back to v10. In 9.6, projections work much differently and
we can't cheaply give them such an option. The 9.6 version of this
patch works by inserting a JunkFilter when it's necessary to get rid
of resjunk columns.
In addition, v11 and up have the reverse problem when trying to
perform ON CONFLICT UPDATE on a partitioned table. Through a
further oversight, adjust_partition_tlist() discarded resjunk columns
when re-ordering the ON CONFLICT UPDATE tlist to match a partition.
This accidentally prevented the storing-bogus-tuples problem, but
at the cost that MULTIEXPR_SUBLINK cases didn't work, typically
crashing if more than one row has to be updated. Fix by preserving
resjunk columns in that routine. (I failed to resist the temptation
to add more assertions there too, and to do some minor code
beautification.)
Per report from Andres Freund. Back-patch to all supported branches.
Security: CVE-2021-32028
---
src/backend/access/heap/heapam.c | 8 +++++++
src/backend/executor/execExpr.c | 33 +++++++++++++++++++++++++-
src/backend/executor/execExprInterp.c | 11 ++++++++-
src/backend/executor/nodeModifyTable.c | 22 +++++++++++------
src/include/executor/executor.h | 6 +++++
src/test/regress/expected/update.out | 12 ++++++----
src/test/regress/sql/update.sql | 6 ++---
7 files changed, 81 insertions(+), 17 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0c59ff11e5..25fcd4c524 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2414,6 +2414,10 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
Buffer vmbuffer = InvalidBuffer;
bool all_visible_cleared = false;
+ /* Cheap, simplistic check that the tuple matches the rel's rowtype. */
+ Assert(HeapTupleHeaderGetNatts(tup->t_data) <=
+ RelationGetNumberOfAttributes(relation));
+
/*
* Fill in tuple header fields, assign an OID, and toast the tuple if
* necessary.
@@ -3515,6 +3519,10 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
Assert(ItemPointerIsValid(otid));
+ /* Cheap, simplistic check that the tuple matches the rel's rowtype. */
+ Assert(HeapTupleHeaderGetNatts(newtup->t_data) <=
+ RelationGetNumberOfAttributes(relation));
+
/*
* Forbid this during a parallel operation, lest it allocate a combocid.
* Other workers might need that combocid for visibility checks, and we
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 7496189fab..2a2741352a 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -303,6 +303,32 @@ ExecBuildProjectionInfo(List *targetList,
TupleTableSlot *slot,
PlanState *parent,
TupleDesc inputDesc)
+{
+ return ExecBuildProjectionInfoExt(targetList,
+ econtext,
+ slot,
+ true,
+ parent,
+ inputDesc);
+}
+
+/*
+ * ExecBuildProjectionInfoExt
+ *
+ * As above, with one additional option.
+ *
+ * If assignJunkEntries is true (the usual case), resjunk entries in the tlist
+ * are not handled specially: they are evaluated and assigned to the proper
+ * column of the result slot. If assignJunkEntries is false, resjunk entries
+ * are evaluated, but their result is discarded without assignment.
+ */
+ProjectionInfo *
+ExecBuildProjectionInfoExt(List *targetList,
+ ExprContext *econtext,
+ TupleTableSlot *slot,
+ bool assignJunkEntries,
+ PlanState *parent,
+ TupleDesc inputDesc)
{
ProjectionInfo *projInfo = makeNode(ProjectionInfo);
ExprState *state;
@@ -337,7 +363,8 @@ ExecBuildProjectionInfo(List *targetList,
*/
if (tle->expr != NULL &&
IsA(tle->expr, Var) &&
- ((Var *) tle->expr)->varattno > 0)
+ ((Var *) tle->expr)->varattno > 0 &&
+ (assignJunkEntries || !tle->resjunk))
{
/* Non-system Var, but how safe is it? */
variable = (Var *) tle->expr;
@@ -401,6 +428,10 @@ ExecBuildProjectionInfo(List *targetList,
ExecInitExprRec(tle->expr, parent, state,
&state->resvalue, &state->resnull);
+ /* This makes it easy to discard resjunk results when told to. */
+ if (!assignJunkEntries && tle->resjunk)
+ continue;
+
/*
* Column might be referenced multiple times in upper nodes, so
* force value to R/O - but only if it could be an expanded datum.
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index b5f5b735f8..de51561769 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -564,6 +564,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
* care of at compilation time. But see EEOP_INNER_VAR comments.
*/
Assert(attnum >= 0 && attnum < innerslot->tts_nvalid);
+ Assert(resultnum >= 0 && resultnum < resultslot->tts_tupleDescriptor->natts);
resultslot->tts_values[resultnum] = innerslot->tts_values[attnum];
resultslot->tts_isnull[resultnum] = innerslot->tts_isnull[attnum];
@@ -580,6 +581,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
* care of at compilation time. But see EEOP_INNER_VAR comments.
*/
Assert(attnum >= 0 && attnum < outerslot->tts_nvalid);
+ Assert(resultnum >= 0 && resultnum < resultslot->tts_tupleDescriptor->natts);
resultslot->tts_values[resultnum] = outerslot->tts_values[attnum];
resultslot->tts_isnull[resultnum] = outerslot->tts_isnull[attnum];
@@ -596,6 +598,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
* care of at compilation time. But see EEOP_INNER_VAR comments.
*/
Assert(attnum >= 0 && attnum < scanslot->tts_nvalid);
+ Assert(resultnum >= 0 && resultnum < resultslot->tts_tupleDescriptor->natts);
resultslot->tts_values[resultnum] = scanslot->tts_values[attnum];
resultslot->tts_isnull[resultnum] = scanslot->tts_isnull[attnum];
@@ -606,6 +609,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
{
int resultnum = op->d.assign_tmp.resultnum;
+ Assert(resultnum >= 0 && resultnum < resultslot->tts_tupleDescriptor->natts);
resultslot->tts_values[resultnum] = state->resvalue;
resultslot->tts_isnull[resultnum] = state->resnull;
@@ -616,6 +620,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
{
int resultnum = op->d.assign_tmp.resultnum;
+ Assert(resultnum >= 0 && resultnum < resultslot->tts_tupleDescriptor->natts);
resultslot->tts_isnull[resultnum] = state->resnull;
if (!resultslot->tts_isnull[resultnum])
resultslot->tts_values[resultnum] =
@@ -1747,8 +1752,10 @@ ExecJustAssignInnerVar(ExprState *state, ExprContext *econtext, bool *isnull)
*
* Since we use slot_getattr(), we don't need to implement the FETCHSOME
* step explicitly, and we also needn't Assert that the attnum is in range
- * --- slot_getattr() will take care of any problems.
+ * --- slot_getattr() will take care of any problems. Nonetheless, check
+ * that resultnum is in range.
*/
+ Assert(resultnum >= 0 && resultnum < outslot->tts_tupleDescriptor->natts);
outslot->tts_values[resultnum] =
slot_getattr(inslot, attnum, &outslot->tts_isnull[resultnum]);
return 0;
@@ -1765,6 +1772,7 @@ ExecJustAssignOuterVar(ExprState *state, ExprContext *econtext, bool *isnull)
TupleTableSlot *outslot = state->resultslot;
/* See comments in ExecJustAssignInnerVar */
+ Assert(resultnum >= 0 && resultnum < outslot->tts_tupleDescriptor->natts);
outslot->tts_values[resultnum] =
slot_getattr(inslot, attnum, &outslot->tts_isnull[resultnum]);
return 0;
@@ -1781,6 +1789,7 @@ ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull)
TupleTableSlot *outslot = state->resultslot;
/* See comments in ExecJustAssignInnerVar */
+ Assert(resultnum >= 0 && resultnum < outslot->tts_tupleDescriptor->natts);
outslot->tts_values[resultnum] =
slot_getattr(inslot, attnum, &outslot->tts_isnull[resultnum]);
return 0;
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 2f6a1102cd..54bb2cb572 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2135,7 +2135,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (node->onConflictAction == ONCONFLICT_UPDATE)
{
ExprContext *econtext;
- TupleDesc tupDesc;
/* insert may only have one plan, inheritance is not expanded */
Assert(nplans == 1);
@@ -2155,16 +2154,25 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_excludedtlist = node->exclRelTlist;
/* create target slot for UPDATE SET projection */
- tupDesc = ExecTypeFromTL((List *) node->onConflictSet,
- resultRelInfo->ri_RelationDesc->rd_rel->relhasoids);
mtstate->mt_conflproj = ExecInitExtraTupleSlot(mtstate->ps.state);
- ExecSetSlotDescriptor(mtstate->mt_conflproj, tupDesc);
+ ExecSetSlotDescriptor(mtstate->mt_conflproj,
+ resultRelInfo->ri_RelationDesc->rd_att);
+
+ /*
+ * The onConflictSet tlist should already have been adjusted to emit
+ * the table's exact column list. It could also contain resjunk
+ * columns, which should be evaluated but not included in the
+ * projection result.
+ */
+ ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
+ node->onConflictSet);
/* build UPDATE SET projection state */
resultRelInfo->ri_onConflictSetProj =
- ExecBuildProjectionInfo(node->onConflictSet, econtext,
- mtstate->mt_conflproj, &mtstate->ps,
- resultRelInfo->ri_RelationDesc->rd_att);
+ ExecBuildProjectionInfoExt(node->onConflictSet, econtext,
+ mtstate->mt_conflproj, false,
+ &mtstate->ps,
+ resultRelInfo->ri_RelationDesc->rd_att);
/* build DO UPDATE WHERE clause expression */
if (node->onConflictWhere)
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 379e7c77a1..da6ec7cab3 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -263,6 +263,12 @@ extern ProjectionInfo *ExecBuildProjectionInfo(List *targetList,
TupleTableSlot *slot,
PlanState *parent,
TupleDesc inputDesc);
+extern ProjectionInfo *ExecBuildProjectionInfoExt(List *targetList,
+ ExprContext *econtext,
+ TupleTableSlot *slot,
+ bool assignJunkEntries,
+ PlanState *parent,
+ TupleDesc inputDesc);
extern ExprState *ExecPrepareExpr(Expr *node, EState *estate);
extern ExprState *ExecPrepareQual(List *qual, EState *estate);
extern ExprState *ExecPrepareCheck(List *qual, EState *estate);
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index 1778023600..54166727fb 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -199,7 +199,7 @@ SELECT a, b, char_length(c) FROM update_test;
(4 rows)
-- Test ON CONFLICT DO UPDATE
-INSERT INTO upsert_test VALUES(1, 'Boo');
+INSERT INTO upsert_test VALUES(1, 'Boo'), (3, 'Zoo');
-- uncorrelated sub-select:
WITH aaa AS (SELECT 1 AS a, 'Foo' AS b) INSERT INTO upsert_test
VALUES (1, 'Bar') ON CONFLICT(a)
@@ -210,22 +210,24 @@ WITH aaa AS (SELECT 1 AS a, 'Foo' AS b) INSERT INTO upsert_test
(1 row)
-- correlated sub-select:
-INSERT INTO upsert_test VALUES (1, 'Baz') ON CONFLICT(a)
+INSERT INTO upsert_test VALUES (1, 'Baz'), (3, 'Zaz') ON CONFLICT(a)
DO UPDATE SET (b, a) = (SELECT b || ', Correlated', a from upsert_test i WHERE i.a = upsert_test.a)
RETURNING *;
a | b
---+-----------------
1 | Foo, Correlated
-(1 row)
+ 3 | Zoo, Correlated
+(2 rows)
-- correlated sub-select (EXCLUDED.* alias):
-INSERT INTO upsert_test VALUES (1, 'Bat') ON CONFLICT(a)
+INSERT INTO upsert_test VALUES (1, 'Bat'), (3, 'Zot') ON CONFLICT(a)
DO UPDATE SET (b, a) = (SELECT b || ', Excluded', a from upsert_test i WHERE i.a = excluded.a)
RETURNING *;
a | b
---+---------------------------
1 | Foo, Correlated, Excluded
-(1 row)
+ 3 | Zoo, Correlated, Excluded
+(2 rows)
DROP TABLE update_test;
DROP TABLE upsert_test;
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index 4cfb8ac7be..d8ff0bc6ff 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -100,17 +100,17 @@ UPDATE update_test t
SELECT a, b, char_length(c) FROM update_test;
-- Test ON CONFLICT DO UPDATE
-INSERT INTO upsert_test VALUES(1, 'Boo');
+INSERT INTO upsert_test VALUES(1, 'Boo'), (3, 'Zoo');
-- uncorrelated sub-select:
WITH aaa AS (SELECT 1 AS a, 'Foo' AS b) INSERT INTO upsert_test
VALUES (1, 'Bar') ON CONFLICT(a)
DO UPDATE SET (b, a) = (SELECT b, a FROM aaa) RETURNING *;
-- correlated sub-select:
-INSERT INTO upsert_test VALUES (1, 'Baz') ON CONFLICT(a)
+INSERT INTO upsert_test VALUES (1, 'Baz'), (3, 'Zaz') ON CONFLICT(a)
DO UPDATE SET (b, a) = (SELECT b || ', Correlated', a from upsert_test i WHERE i.a = upsert_test.a)
RETURNING *;
-- correlated sub-select (EXCLUDED.* alias):
-INSERT INTO upsert_test VALUES (1, 'Bat') ON CONFLICT(a)
+INSERT INTO upsert_test VALUES (1, 'Bat'), (3, 'Zot') ON CONFLICT(a)
DO UPDATE SET (b, a) = (SELECT b || ', Excluded', a from upsert_test i WHERE i.a = excluded.a)
RETURNING *;
--
2.30.2

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

@ -1,70 +0,0 @@
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
View File

@ -1,50 +0,0 @@
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
View File

@ -1,76 +0,0 @@
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

249
postgresql-10.23-CVE-2023-2454.patch Normal file
View File

@ -0,0 +1,249 @@
From 681d9e4621aac0a9c71364b6f54f00f6d8c4337f Mon Sep 17 00:00:00 2001
From 8d525d7b9545884a3e0d79adcd61543f9ae2ae28 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Mon, 8 May 2023 06:14:07 -0700
Subject: Replace last PushOverrideSearchPath() call with
set_config_option().
The two methods don't cooperate, so set_config_option("search_path",
...) has been ineffective under non-empty overrideStack. This defect
enabled an attacker having database-level CREATE privilege to execute
arbitrary code as the bootstrap superuser. While that particular attack
requires v13+ for the trusted extension attribute, other attacks are
feasible in all supported versions.
Standardize on the combination of NewGUCNestLevel() and
set_config_option("search_path", ...). It is newer than
PushOverrideSearchPath(), more-prevalent, and has no known
disadvantages. The "override" mechanism remains for now, for
compatibility with out-of-tree code. Users should update such code,
which likely suffers from the same sort of vulnerability closed here.
Back-patch to v11 (all supported versions).
Alexander Lakhin. Reported by Alexander Lakhin.
Security: CVE-2023-2454
---
contrib/seg/Makefile | 2 +-
contrib/seg/expected/security.out | 32 ++++++++++++++++++
contrib/seg/sql/security.sql | 32 ++++++++++++++++++
src/backend/catalog/namespace.c | 4 +++
src/backend/commands/schemacmds.c | 37 ++++++++++++++------
src/test/regress/expected/namespace.out | 45 +++++++++++++++++++++++++
src/test/regress/sql/namespace.sql | 24 +++++++++++++
7 files changed, 165 insertions(+), 11 deletions(-)
create mode 100644 contrib/seg/expected/security.out
create mode 100644 contrib/seg/sql/security.sql
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 14e57adee2..73ddb67882 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3515,6 +3515,10 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
/*
* PushOverrideSearchPath - temporarily override the search path
*
+ * Do not use this function; almost any usage introduces a security
+ * vulnerability. It exists for the benefit of legacy code running in
+ * non-security-sensitive environments.
+ *
* We allow nested overrides, hence the push/pop terminology. The GUC
* search_path variable is ignored while an override is active.
*
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 48590247f8..b6a71154a8 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -30,6 +30,7 @@
#include "commands/schemacmds.h"
#include "miscadmin.h"
#include "parser/parse_utilcmd.h"
+#include "parser/scansup.h"
#include "tcop/utility.h"
#include "utils/acl.h"
#include "utils/builtins.h"
@@ -53,14 +54,16 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
{
const char *schemaName = stmt->schemaname;
Oid namespaceId;
- OverrideSearchPath *overridePath;
List *parsetree_list;
ListCell *parsetree_item;
Oid owner_uid;
Oid saved_uid;
int save_sec_context;
+ int save_nestlevel;
+ char *nsp = namespace_search_path;
AclResult aclresult;
ObjectAddress address;
+ StringInfoData pathbuf;
GetUserIdAndSecContext(&saved_uid, &save_sec_context);
@@ -153,14 +156,26 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
CommandCounterIncrement();
/*
- * Temporarily make the new namespace be the front of the search path, as
- * well as the default creation target namespace. This will be undone at
- * the end of this routine, or upon error.
+ * Prepend the new schema to the current search path.
+ *
+ * We use the equivalent of a function SET option to allow the setting to
+ * persist for exactly the duration of the schema creation. guc.c also
+ * takes care of undoing the setting on error.
*/
- overridePath = GetOverrideSearchPath(CurrentMemoryContext);
- overridePath->schemas = lcons_oid(namespaceId, overridePath->schemas);
- /* XXX should we clear overridePath->useTemp? */
- PushOverrideSearchPath(overridePath);
+ save_nestlevel = NewGUCNestLevel();
+
+ initStringInfo(&pathbuf);
+ appendStringInfoString(&pathbuf, quote_identifier(schemaName));
+
+ while (scanner_isspace(*nsp))
+ nsp++;
+
+ if (*nsp != '\0')
+ appendStringInfo(&pathbuf, ", %s", nsp);
+
+ (void) set_config_option("search_path", pathbuf.data,
+ PGC_USERSET, PGC_S_SESSION,
+ GUC_ACTION_SAVE, true, 0, false);
/*
* Report the new schema to possibly interested event triggers. Note we
@@ -215,8 +230,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
CommandCounterIncrement();
}
- /* Reset search path to normal state */
- PopOverrideSearchPath();
+ /*
+ * Restore the GUC variable search_path we set above.
+ */
+ AtEOXact_GUC(true, save_nestlevel);
/* Reset current user and security context */
SetUserIdAndSecContext(saved_uid, save_sec_context);
diff --git a/src/test/regress/expected/namespace.out b/src/test/regress/expected/namespace.out
index 2564d1b080..a62fd8ded0 100644
--- a/src/test/regress/expected/namespace.out
+++ b/src/test/regress/expected/namespace.out
@@ -1,6 +1,14 @@
--
-- Regression tests for schemas (namespaces)
--
+-- set the whitespace-only search_path to test that the
+-- GUC list syntax is preserved during a schema creation
+SELECT pg_catalog.set_config('search_path', ' ', false);
+ set_config
+------------
+
+(1 row)
+
CREATE SCHEMA test_schema_1
CREATE UNIQUE INDEX abc_a_idx ON abc (a)
CREATE VIEW abc_view AS
@@ -9,6 +17,43 @@ CREATE SCHEMA test_schema_1
a serial,
b int UNIQUE
);
+-- verify that the correct search_path restored on abort
+SET search_path to public;
+BEGIN;
+SET search_path to public, test_schema_1;
+CREATE SCHEMA test_schema_2
+ CREATE VIEW abc_view AS SELECT c FROM abc;
+ERROR: column "c" does not exist
+LINE 2: CREATE VIEW abc_view AS SELECT c FROM abc;
+ ^
+COMMIT;
+SHOW search_path;
+ search_path
+-------------
+ public
+(1 row)
+
+-- verify that the correct search_path preserved
+-- after creating the schema and on commit
+BEGIN;
+SET search_path to public, test_schema_1;
+CREATE SCHEMA test_schema_2
+ CREATE VIEW abc_view AS SELECT a FROM abc;
+SHOW search_path;
+ search_path
+-----------------------
+ public, test_schema_1
+(1 row)
+
+COMMIT;
+SHOW search_path;
+ search_path
+-----------------------
+ public, test_schema_1
+(1 row)
+
+DROP SCHEMA test_schema_2 CASCADE;
+NOTICE: drop cascades to view test_schema_2.abc_view
-- verify that the objects were created
SELECT COUNT(*) FROM pg_class WHERE relnamespace =
(SELECT oid FROM pg_namespace WHERE nspname = 'test_schema_1');
diff --git a/src/test/regress/sql/namespace.sql b/src/test/regress/sql/namespace.sql
index 6b12c96193..3474f5ecf4 100644
--- a/src/test/regress/sql/namespace.sql
+++ b/src/test/regress/sql/namespace.sql
@@ -2,6 +2,10 @@
-- Regression tests for schemas (namespaces)
--
+-- set the whitespace-only search_path to test that the
+-- GUC list syntax is preserved during a schema creation
+SELECT pg_catalog.set_config('search_path', ' ', false);
+
CREATE SCHEMA test_schema_1
CREATE UNIQUE INDEX abc_a_idx ON abc (a)
@@ -13,6 +17,26 @@ CREATE SCHEMA test_schema_1
b int UNIQUE
);
+-- verify that the correct search_path restored on abort
+SET search_path to public;
+BEGIN;
+SET search_path to public, test_schema_1;
+CREATE SCHEMA test_schema_2
+ CREATE VIEW abc_view AS SELECT c FROM abc;
+COMMIT;
+SHOW search_path;
+
+-- verify that the correct search_path preserved
+-- after creating the schema and on commit
+BEGIN;
+SET search_path to public, test_schema_1;
+CREATE SCHEMA test_schema_2
+ CREATE VIEW abc_view AS SELECT a FROM abc;
+SHOW search_path;
+COMMIT;
+SHOW search_path;
+DROP SCHEMA test_schema_2 CASCADE;
+
-- verify that the objects were created
SELECT COUNT(*) FROM pg_class WHERE relnamespace =
(SELECT oid FROM pg_namespace WHERE nspname = 'test_schema_1');
diff --git a/contrib/sepgsql/expected/ddl.out b/contrib/sepgsql/expected/ddl.out
index e8da587564..15d2b9c5e7 100644
--- a/contrib/sepgsql/expected/ddl.out
+++ b/contrib/sepgsql/expected/ddl.out
@@ -24,7 +24,6 @@ LOG: SELinux: allowed { create } scontext=unconfined_u:unconfined_r:sepgsql_reg
CREATE USER regress_sepgsql_test_user;
CREATE SCHEMA regtest_schema;
LOG: SELinux: allowed { create } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema"
-LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=system_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="public"
GRANT ALL ON SCHEMA regtest_schema TO regress_sepgsql_test_user;
SET search_path = regtest_schema, public;
CREATE TABLE regtest_table (x serial primary key, y text);
--
2.41.0

114
postgresql-10.23-CVE-2023-2455.patch Normal file
View File

@ -0,0 +1,114 @@
From ca73753b090c33bc69ce299b4d7fff891a77b8ad Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 8 May 2023 10:12:44 -0400
Subject: Handle RLS dependencies in inlined set-returning
functions properly.
If an SRF in the FROM clause references a table having row-level
security policies, and we inline that SRF into the calling query,
we neglected to mark the plan as potentially dependent on which
role is executing it. This could lead to later executions in the
same session returning or hiding rows that should have been hidden
or returned instead.
Our thanks to Wolfgang Walther for reporting this problem.
Stephen Frost and Tom Lane
Security: CVE-2023-2455
---
src/backend/optimizer/util/clauses.c | 7 ++++++
src/test/regress/expected/rowsecurity.out | 27 +++++++++++++++++++++++
src/test/regress/sql/rowsecurity.sql | 20 +++++++++++++++++
3 files changed, 54 insertions(+)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index a9c7bc342e..11269fee3e 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5205,6 +5205,13 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
*/
record_plan_function_dependency(root, func_oid);
+ /*
+ * We must also notice if the inserted query adds a dependency on the
+ * calling role due to RLS quals.
+ */
+ if (querytree->hasRowSecurity)
+ root->glob->dependsOnRole = true;
+
return querytree;
/* Here if func is not inlinable: release temp memory and return NULL */
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 38f53ed486..e278346420 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -4427,6 +4427,33 @@ SELECT * FROM rls_tbl;
DROP TABLE rls_tbl;
RESET SESSION AUTHORIZATION;
+-- CVE-2023-2455: inlining an SRF may introduce an RLS dependency
+create table rls_t (c text);
+insert into rls_t values ('invisible to bob');
+alter table rls_t enable row level security;
+grant select on rls_t to regress_rls_alice, regress_rls_bob;
+create policy p1 on rls_t for select to regress_rls_alice using (true);
+create policy p2 on rls_t for select to regress_rls_bob using (false);
+create function rls_f () returns setof rls_t
+ stable language sql
+ as $$ select * from rls_t $$;
+prepare q as select current_user, * from rls_f();
+set role regress_rls_alice;
+execute q;
+ current_user | c
+-------------------+------------------
+ regress_rls_alice | invisible to bob
+(1 row)
+
+set role regress_rls_bob;
+execute q;
+ current_user | c
+--------------+---
+(0 rows)
+
+RESET ROLE;
+DROP FUNCTION rls_f();
+DROP TABLE rls_t;
--
-- Clean up objects
--
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 0fd0cded7d..3d664538a6 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -2127,6 +2127,26 @@ SELECT * FROM rls_tbl;
DROP TABLE rls_tbl;
RESET SESSION AUTHORIZATION;
+-- CVE-2023-2455: inlining an SRF may introduce an RLS dependency
+create table rls_t (c text);
+insert into rls_t values ('invisible to bob');
+alter table rls_t enable row level security;
+grant select on rls_t to regress_rls_alice, regress_rls_bob;
+create policy p1 on rls_t for select to regress_rls_alice using (true);
+create policy p2 on rls_t for select to regress_rls_bob using (false);
+create function rls_f () returns setof rls_t
+ stable language sql
+ as $$ select * from rls_t $$;
+prepare q as select current_user, * from rls_f();
+set role regress_rls_alice;
+execute q;
+set role regress_rls_bob;
+execute q;
+
+RESET ROLE;
+DROP FUNCTION rls_f();
+DROP TABLE rls_t;
+
--
-- Clean up objects
--
--
2.41.0

BIN
postgresql-10.5.tar.bz2 → postgresql-10.23.tar.bz2
View File

Binary file not shown.

1
postgresql-10.23.tar.bz2.sha256 Normal file
View File

@ -0,0 +1 @@
94a4b2528372458e5662c18d406629266667c437198160a18cdfd2c4a4d6eee9 postgresql-10.23.tar.bz2

1
postgresql-10.5.tar.bz2.sha256
View File

@ -1 +0,0 @@
6c8e616c91a45142b85c0aeb1f29ebba4a361309e86469e0fb4617b6a73c4011 postgresql-10.5.tar.bz2

35
postgresql.spec
View File

@ -3,8 +3,8 @@
%global macrosdir %(d=%{_rpmconfigdir}/macros.d; [ -d $d ] || d=%{_sysconfdir}/rpm; echo $d)
Name: postgresql
Version: 10.5
Release: 23
Version: 10.23
Release: 1
Summary: PostgreSQL client programs
License: PostgreSQL
URL: http://www.postgresql.org/
@ -25,28 +25,11 @@ Source13: postgresql_pkg_tests.sh
Patch0000: 0000-postgresql-var-run-socket.patch
Patch0001: 0000-rpm-pgsql.patch
Patch6000: 6000-CVE-2019-10164-1.patch
Patch6001: 6001-CVE-2019-10164-2.patch
Patch6002: CVE-2019-10208.patch
Patch6003: CVE-2018-16850.patch
Patch6004: CVE-2019-10130.patch
Patch6005: CVE-2020-1720.patch
Patch6006: CVE-2020-14349-1.patch
Patch6007: CVE-2020-14349-2.patch
Patch6008: CVE-2020-14350.patch
Patch6009: createdb-Fix-quoting-of-encoding-lc-ctype-and-lc-col.patch
Patch6010: Remove-some-code-related-to-7.3-and-older-servers-fr.patch
Patch6011: Fix-error-handling-of-vacuumdb-when-running-out-of-f.patch
Patch6012: CVE-2020-25694-1.patch
Patch6013: CVE-2020-25694-2.patch
Patch6014: CVE-2020-25694-3.patch
Patch6015: CVE-2020-25695.patch
Patch6016: CVE-2020-25696.patch
Patch6017: CVE-2021-20229.patch
Patch6018: CVE-2021-32028.patch
Patch6019: CVE-2021-23214.patch
Patch6020: CVE-2021-23222.patch
#Patch0002: postgresql-10.15-contrib-dblink-expected-out.patch
#rename Patch6017 number
Patch0003: CVE-2021-20229.patch
Patch0004: postgresql-10.23-CVE-2023-2454.patch
Patch0005: postgresql-10.23-CVE-2023-2455.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
@ -437,6 +420,10 @@ find_lang_bins pltcl.lst pltcl
%attr(-,postgres,postgres) %{_libdir}/pgsql/test
%changelog
* Fri Nov 17 2023 dillon chen <dillon.chen@gmail.com> - 10.23-1
- CVE-2023-2454 and CVE-2023-2455
- update to 10.23
* Fri Mar 11 2022 wangkai <wangkai385@huawei.com> - 10.5-23
- Fix CVE-2021-23214 CVE-2021-23222