Skip to content

Commit

Permalink
get rid of {Push/Pop}OverrideSearchPath
Browse files Browse the repository at this point in the history
PushOverrideSearchPath suffers from a vulnerability issue reported
by CVE-2023-2454. Postgres fix this by replacing it with
set_config_option, see [1]. It is recommended that out-of-tree
code should also update such code though the "override" mechanism
remains for compatibility.

The postgres master branch(i.e. PG 17) recently removed
PushOverrideSearchPath() and PopOverrideSearchPath(), see [2].
So this should also ease the process when citus decide to support
Postgres v17.

This patch also contains some trivial typo fix.

[1]: postgres commit: 681d9e4621aac0a9c71364b6f54f00f6d8c4337f
[2]: postgres commit: 7c5c4e1c0396b0617a6f9b659dd7375fb0bfb9dc

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
  • Loading branch information
zhjwpku committed Sep 5, 2023
1 parent 1d540b6 commit d62f124
Show file tree
Hide file tree
Showing 18 changed files with 80 additions and 98 deletions.
8 changes: 3 additions & 5 deletions src/backend/distributed/commands/alter_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "distributed/multi_executor.h"
#include "distributed/multi_logical_planner.h"
#include "distributed/multi_partitioning_utils.h"
#include "distributed/namespace_utils.h"
#include "distributed/reference_table_utils.h"
#include "distributed/relation_access_tracking.h"
#include "distributed/replication_origin_session_utils.h"
Expand Down Expand Up @@ -1764,10 +1765,7 @@ CreateMaterializedViewDDLCommand(Oid matViewOid)
* Set search_path to NIL so that all objects outside of pg_catalog will be
* schema-prefixed.
*/
OverrideSearchPath *overridePath = GetOverrideSearchPath(CurrentMemoryContext);
overridePath->schemas = NIL;
overridePath->addCatalog = true;
PushOverrideSearchPath(overridePath);
int saveNestLevel = PushEmptySearchPath();

/*
* Push the transaction snapshot to be able to get vief definition with pg_get_viewdef
Expand All @@ -1779,7 +1777,7 @@ CreateMaterializedViewDDLCommand(Oid matViewOid)
char *viewDefinition = TextDatumGetCString(viewDefinitionDatum);

PopActiveSnapshot();
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

appendStringInfo(query, "AS %s", viewDefinition);

Expand Down
8 changes: 4 additions & 4 deletions src/backend/distributed/commands/extension.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ static List * GetAllViews(void);
static bool ShouldPropagateExtensionCommand(Node *parseTree);
static bool IsAlterExtensionSetSchemaCitus(Node *parseTree);
static Node * RecreateExtensionStmt(Oid extensionOid);
static List * GenerateGrantCommandsOnExtesionDependentFDWs(Oid extensionId);
static List * GenerateGrantCommandsOnExtensionDependentFDWs(Oid extensionId);


/*
Expand Down Expand Up @@ -985,7 +985,7 @@ CreateExtensionDDLCommand(const ObjectAddress *extensionAddress)

/* any privilege granted on FDWs that belong to the extension should be included */
List *FDWGrants =
GenerateGrantCommandsOnExtesionDependentFDWs(extensionAddress->objectId);
GenerateGrantCommandsOnExtensionDependentFDWs(extensionAddress->objectId);

ddlCommands = list_concat(ddlCommands, FDWGrants);

Expand Down Expand Up @@ -1048,11 +1048,11 @@ RecreateExtensionStmt(Oid extensionOid)


/*
* GenerateGrantCommandsOnExtesionDependentFDWs returns a list of commands that GRANTs
* GenerateGrantCommandsOnExtensionDependentFDWs returns a list of commands that GRANTs
* the privileges on FDWs that are depending on the given extension.
*/
static List *
GenerateGrantCommandsOnExtesionDependentFDWs(Oid extensionId)
GenerateGrantCommandsOnExtensionDependentFDWs(Oid extensionId)
{
List *commands = NIL;
List *FDWOids = GetDependentFDWsToExtension(extensionId);
Expand Down
4 changes: 2 additions & 2 deletions src/backend/distributed/commands/foreign_constraint.c
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ GetForeignConstraintCommandsInternal(Oid relationId, int flags)

List *foreignKeyCommands = NIL;

PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();

Oid foreignKeyOid = InvalidOid;
foreach_oid(foreignKeyOid, foreignKeyOids)
Expand All @@ -906,7 +906,7 @@ GetForeignConstraintCommandsInternal(Oid relationId, int flags)
}

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

return foreignKeyCommands;
}
Expand Down
5 changes: 2 additions & 3 deletions src/backend/distributed/commands/function.c
Original file line number Diff line number Diff line change
Expand Up @@ -909,15 +909,14 @@ GetFunctionDDLCommand(const RegProcedure funcOid, bool useCreateOrReplace)
else
{
Datum sqlTextDatum = (Datum) 0;

PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();

sqlTextDatum = DirectFunctionCall1(pg_get_functiondef,
ObjectIdGetDatum(funcOid));
createFunctionSQL = TextDatumGetCString(sqlTextDatum);

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);
}

return createFunctionSQL;
Expand Down
4 changes: 2 additions & 2 deletions src/backend/distributed/commands/statistics.c
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ GetExplicitStatisticsCommandList(Oid relationId)
RelationClose(relation);

/* generate fully-qualified names */
PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();

Oid statisticsId = InvalidOid;
foreach_oid(statisticsId, statisticsIdList)
Expand Down Expand Up @@ -579,7 +579,7 @@ GetExplicitStatisticsCommandList(Oid relationId)
}

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

return explicitStatisticsCommandList;
}
Expand Down
4 changes: 2 additions & 2 deletions src/backend/distributed/commands/trigger.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ GetExplicitTriggerCommandList(Oid relationId)
{
List *createTriggerCommandList = NIL;

PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();

List *triggerIdList = GetExplicitTriggerIdList(relationId);

Expand Down Expand Up @@ -116,7 +116,7 @@ GetExplicitTriggerCommandList(Oid relationId)
}

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

return createTriggerCommandList;
}
Expand Down
7 changes: 2 additions & 5 deletions src/backend/distributed/commands/view.c
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,7 @@ AppendViewDefinitionToCreateViewCommand(StringInfo buf, Oid viewOid)
* Set search_path to NIL so that all objects outside of pg_catalog will be
* schema-prefixed.
*/
OverrideSearchPath *overridePath = GetOverrideSearchPath(CurrentMemoryContext);
overridePath->schemas = NIL;
overridePath->addCatalog = true;
PushOverrideSearchPath(overridePath);
int saveNestLevel = PushEmptySearchPath();

/*
* Push the transaction snapshot to be able to get vief definition with pg_get_viewdef
Expand All @@ -494,7 +491,7 @@ AppendViewDefinitionToCreateViewCommand(StringInfo buf, Oid viewOid)
char *viewDefinition = TextDatumGetCString(viewDefinitionDatum);

PopActiveSnapshot();
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

appendStringInfo(buf, "AS %s ", viewDefinition);
}
Expand Down
4 changes: 2 additions & 2 deletions src/backend/distributed/deparser/citus_ruleutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ deparse_shard_index_statement(IndexStmt *origStmt, Oid distrelid, int64 shardid,
* Switch to empty search_path to deparse_index_columns to produce fully-
* qualified names in expressions.
*/
PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();

/* index column or expression list begins here */
appendStringInfoChar(buffer, '(');
Expand Down Expand Up @@ -855,7 +855,7 @@ deparse_shard_index_statement(IndexStmt *origStmt, Oid distrelid, int64 shardid,
}

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);
}


Expand Down
12 changes: 6 additions & 6 deletions src/backend/distributed/deparser/deparse_domain_stmts.c
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,9 @@ AppendAlterDomainStmtSetDefault(StringInfo buf, AlterDomainStmt *stmt)
expr = TransformDefaultExpr(expr, stmt->typeName, baseTypeName);

/* deparse while the searchpath is cleared to force qualification of identifiers */
PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();
char *exprSql = deparse_expression(expr, NIL, true, true);
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

appendStringInfo(buf, "SET DEFAULT %s", exprSql);
}
Expand Down Expand Up @@ -443,9 +443,9 @@ AppendConstraint(StringInfo buf, Constraint *constraint, List *domainName,
elog(ERROR, "missing expression for domain constraint");
}

PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();
char *exprSql = deparse_expression(expr, NIL, true, true);
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

appendStringInfo(buf, " CHECK (%s)", exprSql);
return;
Expand All @@ -469,9 +469,9 @@ AppendConstraint(StringInfo buf, Constraint *constraint, List *domainName,
elog(ERROR, "missing expression for domain default");
}

PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();
char *exprSql = deparse_expression(expr, NIL, true, true);
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

appendStringInfo(buf, " DEFAULT %s", exprSql);
return;
Expand Down
4 changes: 2 additions & 2 deletions src/backend/distributed/deparser/deparse_publication_stmts.c
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,11 @@ AppendWhereClauseExpression(StringInfo buf, RangeVar *tableName,

List *relationContext = deparse_context_for(tableName->relname, relation->rd_id);

PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();
char *whereClauseString = deparse_expression(whereClause,
relationContext,
true, true);
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

appendStringInfoString(buf, whereClauseString);

Expand Down
4 changes: 2 additions & 2 deletions src/backend/distributed/deparser/deparse_table_stmts.c
Original file line number Diff line number Diff line change
Expand Up @@ -562,9 +562,9 @@ DeparseRawExprForColumnDefault(Oid relationId, Oid columnTypeId, int32 columnTyp

List *deparseContext = deparse_context_for(get_rel_name(relationId), relationId);

PushOverrideEmptySearchPath(CurrentMemoryContext);
int saveNestLevel = PushEmptySearchPath();
char *defaultExprStr = deparse_expression(defaultExpr, deparseContext, false, false);
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

RelationClose(relation);

Expand Down
22 changes: 7 additions & 15 deletions src/backend/distributed/deparser/ruleutils_14.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "common/keywords.h"
#include "distributed/citus_nodefuncs.h"
#include "distributed/citus_ruleutils.h"
#include "distributed/namespace_utils.h"
#include "executor/spi.h"
#include "foreign/foreign.h"
#include "funcapi.h"
Expand Down Expand Up @@ -610,18 +611,14 @@ pg_get_rule_expr(Node *expression)
{
bool showImplicitCasts = true;
deparse_context context;
OverrideSearchPath *overridePath = NULL;
StringInfo buffer = makeStringInfo();

/*
* Set search_path to NIL so that all objects outside of pg_catalog will be
* schema-prefixed. pg_catalog will be added automatically when we call
* PushOverrideSearchPath(), since we set addCatalog to true;
* PushEmptySearchPath().
*/
overridePath = GetOverrideSearchPath(CurrentMemoryContext);
overridePath->schemas = NIL;
overridePath->addCatalog = true;
PushOverrideSearchPath(overridePath);
int saveNestLevel = PushEmptySearchPath();

context.buf = buffer;
context.namespaces = NIL;
Expand All @@ -638,7 +635,7 @@ pg_get_rule_expr(Node *expression)
get_rule_expr(expression, &context, showImplicitCasts);

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

return buffer->data;
}
Expand Down Expand Up @@ -1955,8 +1952,6 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
deparse_context context;
deparse_namespace dpns;

OverrideSearchPath *overridePath = NULL;

/* Guard against excessively long or deeply-nested queries */
CHECK_FOR_INTERRUPTS();
check_stack_depth();
Expand All @@ -1975,12 +1970,9 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
/*
* Set search_path to NIL so that all objects outside of pg_catalog will be
* schema-prefixed. pg_catalog will be added automatically when we call
* PushOverrideSearchPath(), since we set addCatalog to true;
* PushEmptySearchPath().
*/
overridePath = GetOverrideSearchPath(CurrentMemoryContext);
overridePath->schemas = NIL;
overridePath->addCatalog = true;
PushOverrideSearchPath(overridePath);
int saveNestLevel = PushEmptySearchPath();

context.buf = buf;
context.namespaces = lcons(&dpns, list_copy(parentnamespace));
Expand Down Expand Up @@ -2031,7 +2023,7 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
}

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);
}

/* ----------
Expand Down
22 changes: 7 additions & 15 deletions src/backend/distributed/deparser/ruleutils_15.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#include "distributed/citus_nodefuncs.h"
#include "distributed/citus_ruleutils.h"
#include "distributed/multi_router_planner.h"
#include "distributed/namespace_utils.h"
#include "executor/spi.h"
#include "foreign/foreign.h"
#include "funcapi.h"
Expand Down Expand Up @@ -624,18 +625,14 @@ pg_get_rule_expr(Node *expression)
{
bool showImplicitCasts = true;
deparse_context context;
OverrideSearchPath *overridePath = NULL;
StringInfo buffer = makeStringInfo();

/*
* Set search_path to NIL so that all objects outside of pg_catalog will be
* schema-prefixed. pg_catalog will be added automatically when we call
* PushOverrideSearchPath(), since we set addCatalog to true;
* PushEmptySearchPath(), since we set addCatalog to true;
*/
overridePath = GetOverrideSearchPath(CurrentMemoryContext);
overridePath->schemas = NIL;
overridePath->addCatalog = true;
PushOverrideSearchPath(overridePath);
int saveNestLevel = PushEmptySearchPath();

context.buf = buffer;
context.namespaces = NIL;
Expand All @@ -652,7 +649,7 @@ pg_get_rule_expr(Node *expression)
get_rule_expr(expression, &context, showImplicitCasts);

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);

return buffer->data;
}
Expand Down Expand Up @@ -2038,8 +2035,6 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
deparse_context context;
deparse_namespace dpns;

OverrideSearchPath *overridePath = NULL;

/* Guard against excessively long or deeply-nested queries */
CHECK_FOR_INTERRUPTS();
check_stack_depth();
Expand All @@ -2058,12 +2053,9 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
/*
* Set search_path to NIL so that all objects outside of pg_catalog will be
* schema-prefixed. pg_catalog will be added automatically when we call
* PushOverrideSearchPath(), since we set addCatalog to true;
* PushEmptySearchPath().
*/
overridePath = GetOverrideSearchPath(CurrentMemoryContext);
overridePath->schemas = NIL;
overridePath->addCatalog = true;
PushOverrideSearchPath(overridePath);
int saveNestLevel = PushEmptySearchPath();

context.buf = buf;
context.namespaces = lcons(&dpns, list_copy(parentnamespace));
Expand Down Expand Up @@ -2118,7 +2110,7 @@ get_query_def_extended(Query *query, StringInfo buf, List *parentnamespace,
}

/* revert back to original search_path */
PopOverrideSearchPath();
PopEmptySearchPath(saveNestLevel);
}

/* ----------
Expand Down

0 comments on commit d62f124

Please sign in to comment.