Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 862861e

Browse filesBrowse files
tglsfdctglsfdc
committed
Fix a couple of misbehaviors rooted in the fact that the default creation
namespace isn't necessarily first in the search path (there could be implicit schemas ahead of it). Examples are test=# set search_path TO s1; test=# create view pg_timezone_names as select * from pg_timezone_names(); ERROR: "pg_timezone_names" is already a view test=# create table pg_class (f1 int primary key); ERROR: permission denied: "pg_class" is a system catalog You'd expect these commands to create the requested objects in s1, since names beginning with pg_ aren't supposed to be reserved anymore. What is happening is that we create the requested base table and then execute additional commands (here, CREATE RULE or CREATE INDEX), and that code is passed the same RangeVar that was in the original command. Since that RangeVar has schemaname = NULL, the secondary commands think they should do a path search, and that means they find system catalogs that are implicitly in front of s1 in the search path. This is perilously close to being a security hole: if the secondary command failed to apply a permission check then it'd be possible for unprivileged users to make schema modifications to system catalogs. But as far as I can find, there is no code path in which a check doesn't occur. Which makes it just a weird corner-case bug for people who are silly enough to want to name their tables the same as a system catalog. The relevant code has changed quite a bit since 8.2, which means this patch wouldn't work as-is in the back branches. Since it's a corner case no one has reported from the field, I'm not going to bother trying to back-patch.
1 parent 6c96188 commit 862861e
Copy full SHA for 862861e

File tree

Expand file treeCollapse file tree

5 files changed

+66
-27
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+66
-27
lines changed

‎src/backend/catalog/namespace.c

Copy file name to clipboardExpand all lines: src/backend/catalog/namespace.c
+21-2Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* Portions Copyright (c) 1994, Regents of the University of California
1414
*
1515
* IDENTIFICATION
16-
* $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.98 2007/08/21 01:11:13 tgl Exp $
16+
* $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.99 2007/08/27 03:36:08 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -228,7 +228,26 @@ RangeVarGetRelid(const RangeVar *relation, bool failOK)
228228
relation->relname)));
229229
}
230230

231-
if (relation->schemaname)
231+
/*
232+
* If istemp is set, this is a reference to a temp relation. The parser
233+
* never generates such a RangeVar in simple DML, but it can happen in
234+
* contexts such as "CREATE TEMP TABLE foo (f1 int PRIMARY KEY)". Such a
235+
* command will generate an added CREATE INDEX operation, which must be
236+
* careful to find the temp table, even when pg_temp is not first in the
237+
* search path.
238+
*/
239+
if (relation->istemp)
240+
{
241+
if (relation->schemaname)
242+
ereport(ERROR,
243+
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
244+
errmsg("temporary tables cannot specify a schema name")));
245+
if (OidIsValid(myTempNamespace))
246+
relId = get_relname_relid(relation->relname, myTempNamespace);
247+
else /* this probably can't happen? */
248+
relId = InvalidOid;
249+
}
250+
else if (relation->schemaname)
232251
{
233252
/* use exact schema given */
234253
namespaceId = LookupExplicitNamespace(relation->schemaname);

‎src/backend/commands/view.c

Copy file name to clipboardExpand all lines: src/backend/commands/view.c
+7-5Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.101 2007/06/23 22:12:50 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.102 2007/08/27 03:36:08 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -260,14 +260,14 @@ checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc)
260260
}
261261

262262
static void
263-
DefineViewRules(const RangeVar *view, Query *viewParse, bool replace)
263+
DefineViewRules(Oid viewOid, Query *viewParse, bool replace)
264264
{
265265
/*
266266
* Set up the ON SELECT rule. Since the query has already been through
267267
* parse analysis, we use DefineQueryRewrite() directly.
268268
*/
269269
DefineQueryRewrite(pstrdup(ViewSelectRuleName),
270-
(RangeVar *) copyObject((RangeVar *) view),
270+
viewOid,
271271
NULL,
272272
CMD_SELECT,
273273
true,
@@ -404,7 +404,9 @@ DefineView(ViewStmt *stmt, const char *queryString)
404404

405405
/*
406406
* If the user didn't explicitly ask for a temporary view, check whether
407-
* we need one implicitly.
407+
* we need one implicitly. We allow TEMP to be inserted automatically
408+
* as long as the CREATE command is consistent with that --- no explicit
409+
* schema name.
408410
*/
409411
view = stmt->view;
410412
if (!view->istemp && isViewOnTempTable(viewParse))
@@ -441,7 +443,7 @@ DefineView(ViewStmt *stmt, const char *queryString)
441443
/*
442444
* Now create the rules associated with the view.
443445
*/
444-
DefineViewRules(view, viewParse, stmt->replace);
446+
DefineViewRules(viewOid, viewParse, stmt->replace);
445447
}
446448

447449
/*

‎src/backend/parser/parse_utilcmd.c

Copy file name to clipboardExpand all lines: src/backend/parser/parse_utilcmd.c
+15-1Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
2020
* Portions Copyright (c) 1994, Regents of the University of California
2121
*
22-
* $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.2 2007/07/17 05:02:02 neilc Exp $
22+
* $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.3 2007/08/27 03:36:08 tgl Exp $
2323
*
2424
*-------------------------------------------------------------------------
2525
*/
@@ -143,6 +143,20 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
143143
*/
144144
stmt = (CreateStmt *) copyObject(stmt);
145145

146+
/*
147+
* If the target relation name isn't schema-qualified, make it so. This
148+
* prevents some corner cases in which added-on rewritten commands might
149+
* think they should apply to other relations that have the same name
150+
* and are earlier in the search path. "istemp" is equivalent to a
151+
* specification of pg_temp, so no need for anything extra in that case.
152+
*/
153+
if (stmt->relation->schemaname == NULL && !stmt->relation->istemp)
154+
{
155+
Oid namespaceid = RangeVarGetCreationNamespace(stmt->relation);
156+
157+
stmt->relation->schemaname = get_namespace_name(namespaceid);
158+
}
159+
146160
/* Set up pstate */
147161
pstate = make_parsestate(NULL);
148162
pstate->p_sourcetext = queryString;

‎src/backend/rewrite/rewriteDefine.c

Copy file name to clipboardExpand all lines: src/backend/rewrite/rewriteDefine.c
+21-17Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.121 2007/06/23 22:12:51 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.122 2007/08/27 03:36:08 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -17,6 +17,7 @@
1717
#include "access/heapam.h"
1818
#include "catalog/dependency.h"
1919
#include "catalog/indexing.h"
20+
#include "catalog/namespace.h"
2021
#include "catalog/pg_rewrite.h"
2122
#include "miscadmin.h"
2223
#include "optimizer/clauses.h"
@@ -189,13 +190,17 @@ DefineRule(RuleStmt *stmt, const char *queryString)
189190
{
190191
List *actions;
191192
Node *whereClause;
193+
Oid relId;
192194

193195
/* Parse analysis ... */
194196
transformRuleStmt(stmt, queryString, &actions, &whereClause);
195197

196-
/* ... and execution */
198+
/* ... find the relation ... */
199+
relId = RangeVarGetRelid(stmt->relation, false);
200+
201+
/* ... and execute */
197202
DefineQueryRewrite(stmt->rulename,
198-
stmt->relation,
203+
relId,
199204
whereClause,
200205
stmt->event,
201206
stmt->instead,
@@ -213,15 +218,14 @@ DefineRule(RuleStmt *stmt, const char *queryString)
213218
*/
214219
void
215220
DefineQueryRewrite(char *rulename,
216-
RangeVar *event_obj,
221+
Oid event_relid,
217222
Node *event_qual,
218223
CmdType event_type,
219224
bool is_instead,
220225
bool replace,
221226
List *action)
222227
{
223228
Relation event_relation;
224-
Oid ev_relid;
225229
Oid ruleId;
226230
int event_attno;
227231
ListCell *l;
@@ -235,13 +239,12 @@ DefineQueryRewrite(char *rulename,
235239
* grab ShareLock to lock out insert/update/delete actions. But for now,
236240
* let's just grab AccessExclusiveLock all the time.
237241
*/
238-
event_relation = heap_openrv(event_obj, AccessExclusiveLock);
239-
ev_relid = RelationGetRelid(event_relation);
242+
event_relation = heap_open(event_relid, AccessExclusiveLock);
240243

241244
/*
242245
* Check user has permission to apply rules to this relation.
243246
*/
244-
if (!pg_class_ownercheck(ev_relid, GetUserId()))
247+
if (!pg_class_ownercheck(event_relid, GetUserId()))
245248
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
246249
RelationGetRelationName(event_relation));
247250

@@ -352,12 +355,13 @@ DefineQueryRewrite(char *rulename,
352355
* truncated.
353356
*/
354357
if (strncmp(rulename, "_RET", 4) != 0 ||
355-
strncmp(rulename + 4, event_obj->relname,
358+
strncmp(rulename + 4, RelationGetRelationName(event_relation),
356359
NAMEDATALEN - 4 - 4) != 0)
357360
ereport(ERROR,
358361
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
359362
errmsg("view rule for \"%s\" must be named \"%s\"",
360-
event_obj->relname, ViewSelectRuleName)));
363+
RelationGetRelationName(event_relation),
364+
ViewSelectRuleName)));
361365
rulename = pstrdup(ViewSelectRuleName);
362366
}
363367

@@ -377,27 +381,27 @@ DefineQueryRewrite(char *rulename,
377381
ereport(ERROR,
378382
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
379383
errmsg("could not convert table \"%s\" to a view because it is not empty",
380-
event_obj->relname)));
384+
RelationGetRelationName(event_relation))));
381385
heap_endscan(scanDesc);
382386

383387
if (event_relation->rd_rel->reltriggers != 0)
384388
ereport(ERROR,
385389
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
386390
errmsg("could not convert table \"%s\" to a view because it has triggers",
387-
event_obj->relname),
391+
RelationGetRelationName(event_relation)),
388392
errhint("In particular, the table cannot be involved in any foreign key relationships.")));
389393

390394
if (event_relation->rd_rel->relhasindex)
391395
ereport(ERROR,
392396
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
393397
errmsg("could not convert table \"%s\" to a view because it has indexes",
394-
event_obj->relname)));
398+
RelationGetRelationName(event_relation))));
395399

396400
if (event_relation->rd_rel->relhassubclass)
397401
ereport(ERROR,
398402
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
399403
errmsg("could not convert table \"%s\" to a view because it has child tables",
400-
event_obj->relname)));
404+
RelationGetRelationName(event_relation))));
401405

402406
RelisBecomingView = true;
403407
}
@@ -449,7 +453,7 @@ DefineQueryRewrite(char *rulename,
449453
{
450454
ruleId = InsertRule(rulename,
451455
event_type,
452-
ev_relid,
456+
event_relid,
453457
event_attno,
454458
is_instead,
455459
event_qual,
@@ -465,7 +469,7 @@ DefineQueryRewrite(char *rulename,
465469
* backends (including me!) to update relcache entries with the new
466470
* rule.
467471
*/
468-
SetRelationRuleStatus(ev_relid, true, RelisBecomingView);
472+
SetRelationRuleStatus(event_relid, true, RelisBecomingView);
469473
}
470474

471475
/*
@@ -701,7 +705,7 @@ EnableDisableRule(Relation rel, const char *rulename,
701705
/*
702706
* Rename an existing rewrite rule.
703707
*
704-
* This is unused code at the moment.
708+
* This is unused code at the moment. Note that it lacks a permissions check.
705709
*/
706710
#ifdef NOT_USED
707711
void

‎src/include/rewrite/rewriteDefine.h

Copy file name to clipboardExpand all lines: src/include/rewrite/rewriteDefine.h
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/rewrite/rewriteDefine.h,v 1.25 2007/03/19 23:38:32 wieck Exp $
10+
* $PostgreSQL: pgsql/src/include/rewrite/rewriteDefine.h,v 1.26 2007/08/27 03:36:08 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -24,7 +24,7 @@
2424
extern void DefineRule(RuleStmt *stmt, const char *queryString);
2525

2626
extern void DefineQueryRewrite(char *rulename,
27-
RangeVar *event_obj,
27+
Oid event_relid,
2828
Node *event_qual,
2929
CmdType event_type,
3030
bool is_instead,

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.