OSDN Git Service

Fix a couple of misbehaviors rooted in the fact that the default creation
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 27 Aug 2007 03:36:08 +0000 (03:36 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 27 Aug 2007 03:36:08 +0000 (03:36 +0000)
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.

src/backend/catalog/namespace.c
src/backend/commands/view.c
src/backend/parser/parse_utilcmd.c
src/backend/rewrite/rewriteDefine.c
src/include/rewrite/rewriteDefine.h

index 84220bd..46e0312 100644 (file)
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.98 2007/08/21 01:11:13 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.99 2007/08/27 03:36:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -228,7 +228,26 @@ RangeVarGetRelid(const RangeVar *relation, bool failOK)
                                                        relation->relname)));
        }
 
-       if (relation->schemaname)
+       /*
+        * If istemp is set, this is a reference to a temp relation.  The parser
+        * never generates such a RangeVar in simple DML, but it can happen in
+        * contexts such as "CREATE TEMP TABLE foo (f1 int PRIMARY KEY)".  Such a
+        * command will generate an added CREATE INDEX operation, which must be
+        * careful to find the temp table, even when pg_temp is not first in the
+        * search path.
+        */
+       if (relation->istemp)
+       {
+               if (relation->schemaname)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+                                        errmsg("temporary tables cannot specify a schema name")));
+               if (OidIsValid(myTempNamespace))
+                       relId = get_relname_relid(relation->relname, myTempNamespace);
+               else                                    /* this probably can't happen? */
+                       relId = InvalidOid;
+       }
+       else if (relation->schemaname)
        {
                /* use exact schema given */
                namespaceId = LookupExplicitNamespace(relation->schemaname);
index f8dac12..a6ef94d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.101 2007/06/23 22:12:50 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.102 2007/08/27 03:36:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -260,14 +260,14 @@ checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc)
 }
 
 static void
-DefineViewRules(const RangeVar *view, Query *viewParse, bool replace)
+DefineViewRules(Oid viewOid, Query *viewParse, bool replace)
 {
        /*
         * Set up the ON SELECT rule.  Since the query has already been through
         * parse analysis, we use DefineQueryRewrite() directly.
         */
        DefineQueryRewrite(pstrdup(ViewSelectRuleName),
-                                          (RangeVar *) copyObject((RangeVar *) view),
+                                          viewOid,
                                           NULL,
                                           CMD_SELECT,
                                           true,
@@ -404,7 +404,9 @@ DefineView(ViewStmt *stmt, const char *queryString)
 
        /*
         * If the user didn't explicitly ask for a temporary view, check whether
-        * we need one implicitly.
+        * we need one implicitly.  We allow TEMP to be inserted automatically
+        * as long as the CREATE command is consistent with that --- no explicit
+        * schema name.
         */
        view = stmt->view;
        if (!view->istemp && isViewOnTempTable(viewParse))
@@ -441,7 +443,7 @@ DefineView(ViewStmt *stmt, const char *queryString)
        /*
         * Now create the rules associated with the view.
         */
-       DefineViewRules(view, viewParse, stmt->replace);
+       DefineViewRules(viewOid, viewParse, stmt->replace);
 }
 
 /*
index f17ad48..54e7dfd 100644 (file)
@@ -19,7 +19,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *     $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.2 2007/07/17 05:02:02 neilc Exp $
+ *     $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.3 2007/08/27 03:36:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -143,6 +143,20 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
         */
        stmt = (CreateStmt *) copyObject(stmt);
 
+       /*
+        * If the target relation name isn't schema-qualified, make it so.  This
+        * prevents some corner cases in which added-on rewritten commands might
+        * think they should apply to other relations that have the same name
+        * and are earlier in the search path.  "istemp" is equivalent to a
+        * specification of pg_temp, so no need for anything extra in that case.
+        */
+       if (stmt->relation->schemaname == NULL && !stmt->relation->istemp)
+       {
+               Oid             namespaceid = RangeVarGetCreationNamespace(stmt->relation);
+
+               stmt->relation->schemaname = get_namespace_name(namespaceid);
+       }
+
        /* Set up pstate */
        pstate = make_parsestate(NULL);
        pstate->p_sourcetext = queryString;
index 540f340..34c1306 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.121 2007/06/23 22:12:51 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.122 2007/08/27 03:36:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,6 +17,7 @@
 #include "access/heapam.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
+#include "catalog/namespace.h"
 #include "catalog/pg_rewrite.h"
 #include "miscadmin.h"
 #include "optimizer/clauses.h"
@@ -189,13 +190,17 @@ DefineRule(RuleStmt *stmt, const char *queryString)
 {
        List       *actions;
        Node       *whereClause;
+       Oid                     relId;
 
        /* Parse analysis ... */
        transformRuleStmt(stmt, queryString, &actions, &whereClause);
 
-       /* ... and execution */
+       /* ... find the relation ... */
+       relId = RangeVarGetRelid(stmt->relation, false);
+
+       /* ... and execute */
        DefineQueryRewrite(stmt->rulename,
-                                          stmt->relation,
+                                          relId,
                                           whereClause,
                                           stmt->event,
                                           stmt->instead,
@@ -213,7 +218,7 @@ DefineRule(RuleStmt *stmt, const char *queryString)
  */
 void
 DefineQueryRewrite(char *rulename,
-                                  RangeVar *event_obj,
+                                  Oid event_relid,
                                   Node *event_qual,
                                   CmdType event_type,
                                   bool is_instead,
@@ -221,7 +226,6 @@ DefineQueryRewrite(char *rulename,
                                   List *action)
 {
        Relation        event_relation;
-       Oid                     ev_relid;
        Oid                     ruleId;
        int                     event_attno;
        ListCell   *l;
@@ -235,13 +239,12 @@ DefineQueryRewrite(char *rulename,
         * grab ShareLock to lock out insert/update/delete actions.  But for now,
         * let's just grab AccessExclusiveLock all the time.
         */
-       event_relation = heap_openrv(event_obj, AccessExclusiveLock);
-       ev_relid = RelationGetRelid(event_relation);
+       event_relation = heap_open(event_relid, AccessExclusiveLock);
 
        /*
         * Check user has permission to apply rules to this relation.
         */
-       if (!pg_class_ownercheck(ev_relid, GetUserId()))
+       if (!pg_class_ownercheck(event_relid, GetUserId()))
                aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
                                           RelationGetRelationName(event_relation));
 
@@ -352,12 +355,13 @@ DefineQueryRewrite(char *rulename,
                         * truncated.
                         */
                        if (strncmp(rulename, "_RET", 4) != 0 ||
-                               strncmp(rulename + 4, event_obj->relname,
+                               strncmp(rulename + 4, RelationGetRelationName(event_relation),
                                                NAMEDATALEN - 4 - 4) != 0)
                                ereport(ERROR,
                                                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                                                 errmsg("view rule for \"%s\" must be named \"%s\"",
-                                                               event_obj->relname, ViewSelectRuleName)));
+                                                               RelationGetRelationName(event_relation),
+                                                               ViewSelectRuleName)));
                        rulename = pstrdup(ViewSelectRuleName);
                }
 
@@ -377,27 +381,27 @@ DefineQueryRewrite(char *rulename,
                                ereport(ERROR,
                                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                                 errmsg("could not convert table \"%s\" to a view because it is not empty",
-                                                               event_obj->relname)));
+                                                               RelationGetRelationName(event_relation))));
                        heap_endscan(scanDesc);
 
                        if (event_relation->rd_rel->reltriggers != 0)
                                ereport(ERROR,
                                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                                 errmsg("could not convert table \"%s\" to a view because it has triggers",
-                                                               event_obj->relname),
+                                                               RelationGetRelationName(event_relation)),
                                                 errhint("In particular, the table cannot be involved in any foreign key relationships.")));
 
                        if (event_relation->rd_rel->relhasindex)
                                ereport(ERROR,
                                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                                 errmsg("could not convert table \"%s\" to a view because it has indexes",
-                                                               event_obj->relname)));
+                                                               RelationGetRelationName(event_relation))));
 
                        if (event_relation->rd_rel->relhassubclass)
                                ereport(ERROR,
                                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                                 errmsg("could not convert table \"%s\" to a view because it has child tables",
-                                                               event_obj->relname)));
+                                                               RelationGetRelationName(event_relation))));
 
                        RelisBecomingView = true;
                }
@@ -449,7 +453,7 @@ DefineQueryRewrite(char *rulename,
        {
                ruleId = InsertRule(rulename,
                                                        event_type,
-                                                       ev_relid,
+                                                       event_relid,
                                                        event_attno,
                                                        is_instead,
                                                        event_qual,
@@ -465,7 +469,7 @@ DefineQueryRewrite(char *rulename,
                 * backends (including me!) to update relcache entries with the new
                 * rule.
                 */
-               SetRelationRuleStatus(ev_relid, true, RelisBecomingView);
+               SetRelationRuleStatus(event_relid, true, RelisBecomingView);
        }
 
        /*
@@ -701,7 +705,7 @@ EnableDisableRule(Relation rel, const char *rulename,
 /*
  * Rename an existing rewrite rule.
  *
- * This is unused code at the moment.
+ * This is unused code at the moment.  Note that it lacks a permissions check.
  */
 #ifdef NOT_USED
 void
index 5b21cf8..f446934 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/rewrite/rewriteDefine.h,v 1.25 2007/03/19 23:38:32 wieck Exp $
+ * $PostgreSQL: pgsql/src/include/rewrite/rewriteDefine.h,v 1.26 2007/08/27 03:36:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -24,7 +24,7 @@
 extern void DefineRule(RuleStmt *stmt, const char *queryString);
 
 extern void DefineQueryRewrite(char *rulename,
-                                  RangeVar *event_obj,
+                                  Oid event_relid,
                                   Node *event_qual,
                                   CmdType event_type,
                                   bool is_instead,