Repair failure to check that a table is still compatible with a previously

made query plan.  Use of ALTER COLUMN TYPE creates a hazard for cached
query plans: they could contain Vars that claim a column has a different
type than it now has.  Fix this by checking during plan startup that Vars
at relation scan level match the current relation tuple descriptor.  Since
at that point we already have at least AccessShareLock, we can be sure the
column type will not change underneath us later in the query.  However,
since a backend's locks do not conflict against itself, there is still a
hole for an attacker to exploit: he could try to execute ALTER COLUMN TYPE
while a query is in progress in the current backend.  Seal that hole by
rejecting ALTER TABLE whenever the target relation is already open in
the current backend.

This is a significant security hole: not only can one trivially crash the
backend, but with appropriate misuse of pass-by-reference datatypes it is
possible to read out arbitrary locations in the server process's memory,
which could allow retrieving database content the user should not be able
to see.  Our thanks to Jeff Trout for the initial report.

Security: CVE-2007-0556

1 files changed, 50 insertions, 7 deletions

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0aaf47a19dc..7e347b27f0e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.206.2.1 2007/01/25 04:17:56 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.206.2.2 2007/02/02 00:07:27 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1950,22 +1950,47 @@ update_ri_trigger_args(Oid relid,
 void
 AlterTable(AlterTableStmt *stmt)
 {
-	ATController(relation_openrv(stmt->relation, AccessExclusiveLock),
-				 stmt->cmds,
-				 interpretInhOption(stmt->relation->inhOpt));
+	Relation rel = relation_openrv(stmt->relation, AccessExclusiveLock);
+	int			expected_refcnt;
+
+	/*
+	 * Disallow ALTER TABLE when the current backend has any open reference
+	 * to it besides the one we just got (such as an open cursor or active
+	 * plan); our AccessExclusiveLock doesn't protect us against stomping on
+	 * our own foot, only other people's feet!
+	 *
+	 * Note: the only case known to cause serious trouble is ALTER COLUMN TYPE,
+	 * and some changes are obviously pretty benign, so this could possibly
+	 * be relaxed to only error out for certain types of alterations.  But
+	 * the use-case for allowing any of these things is not obvious, so we
+	 * won't work hard at it for now.
+	 */
+	expected_refcnt = rel->rd_isnailed ? 2 : 1;
+	if (rel->rd_refcnt != expected_refcnt)
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_IN_USE),
+				 errmsg("relation \"%s\" is being used by active queries in this session",
+						RelationGetRelationName(rel))));
+
+	ATController(rel, stmt->cmds, interpretInhOption(stmt->relation->inhOpt));
 }
 
 /*
  * AlterTableInternal
  *
  * ALTER TABLE with target specified by OID
+ *
+ * We do not reject if the relation is already open, because it's quite
+ * likely that one or more layers of caller have it open.  That means it
+ * is unsafe to use this entry point for alterations that could break
+ * existing query plans.
  */
 void
 AlterTableInternal(Oid relid, List *cmds, bool recurse)
 {
-	ATController(relation_open(relid, AccessExclusiveLock),
-				 cmds,
-				 recurse);
+	Relation rel = relation_open(relid, AccessExclusiveLock);
+
+	ATController(rel, cmds, recurse);
 }
 
 static void
@@ -2915,6 +2940,12 @@ ATSimpleRecursion(List **wqueue, Relation rel,
 			if (childrelid == relid)
 				continue;
 			childrel = relation_open(childrelid, AccessExclusiveLock);
+			/* check for child relation in use in this session */
+			if (childrel->rd_refcnt != 1)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_IN_USE),
+						 errmsg("relation \"%s\" is being used by active queries in this session",
+								RelationGetRelationName(childrel))));
 			ATPrepCmd(wqueue, childrel, cmd, false, true);
 			relation_close(childrel, NoLock);
 		}
@@ -2946,6 +2977,12 @@ ATOneLevelRecursion(List **wqueue, Relation rel,
 		Relation	childrel;
 
 		childrel = relation_open(childrelid, AccessExclusiveLock);
+		/* check for child relation in use in this session */
+		if (childrel->rd_refcnt != 1)
+			ereport(ERROR,
+					(errcode(ERRCODE_OBJECT_IN_USE),
+					 errmsg("relation \"%s\" is being used by active queries in this session",
+							RelationGetRelationName(childrel))));
 		ATPrepCmd(wqueue, childrel, cmd, true, true);
 		relation_close(childrel, NoLock);
 	}
@@ -3745,6 +3782,12 @@ ATExecDropColumn(Relation rel, const char *colName,
 			Form_pg_attribute childatt;
 
 			childrel = heap_open(childrelid, AccessExclusiveLock);
+			/* check for child relation in use in this session */
+			if (childrel->rd_refcnt != 1)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_IN_USE),
+						 errmsg("relation \"%s\" is being used by active queries in this session",
+								RelationGetRelationName(childrel))));
 
 			tuple = SearchSysCacheCopyAttName(childrelid, colName);
 			if (!HeapTupleIsValid(tuple))		/* shouldn't happen */