Remove typmod checking from the recent security-related patches. It turns

tglsfdc · tglsfdc · commit a8c3f161fb21 · 2007-02-06T17:35:20.000Z
out that ExecEvalVar and friends don't necessarily have access to a tuple
descriptor with correct typmod: it definitely can contain -1, and possibly
might contain other values that are different from the Var's value.
Arguably this should be cleaned up someday, but it's not a simple change,
and in any case typmod discrepancies don't pose a security hazard.
Per reports from numerous people :-(

I'm not entirely sure whether the failure can occur in 8.0 --- the simple
test cases reported so far don't trigger it there.  But back-patch the
change all the way anyway.
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.212 2007/02/03 14:06:53 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.213 2007/02/06 17:35:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -488,8 +488,11 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext,
 		 * Note: we allow a reference to a dropped attribute.  slot_getattr
 		 * will force a NULL result in such cases.
 		 *
-		 * Note: we check typmod, but allow the case that the Var has
-		 * unspecified typmod while the column has a specific typmod.
+		 * Note: ideally we'd check typmod as well as typid, but that seems
+		 * impractical at the moment: in many cases the tupdesc will have
+		 * been generated by ExecTypeFromTL(), and that can't guarantee to
+		 * generate an accurate typmod in all cases, because some expression
+		 * node types don't carry typmod.
 		 */
 		if (attnum > 0)
 		{
@@ -505,9 +508,7 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext,
 			/* can't check type if dropped, since atttypid is probably 0 */
 			if (!attr->attisdropped)
 			{
-				if (variable->vartype != attr->atttypid ||
-					(variable->vartypmod != attr->atttypmod &&
-					 variable->vartypmod != -1))
+				if (variable->vartype != attr->atttypid)
 					ereport(ERROR,
 							(errmsg("attribute %d has wrong type", attnum),
 							 errdetail("Table has type %s, but query expects %s.",
@@ -3362,9 +3363,8 @@ ExecEvalFieldSelect(FieldSelectState *fstate,
 	}
 
 	/* Check for type mismatch --- possible after ALTER COLUMN TYPE? */
-	if (fselect->resulttype != attr->atttypid ||
-		(fselect->resulttypmod != attr->atttypmod &&
-		 fselect->resulttypmod != -1))
+	/* As in ExecEvalVar, we should but can't check typmod */
+	if (fselect->resulttype != attr->atttypid)
 		ereport(ERROR,
 				(errmsg("attribute %d has wrong type", fieldnum),
 				 errdetail("Table has type %s, but query expects %s.",
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.143 2007/02/02 00:07:03 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.144 2007/02/06 17:35:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -632,10 +632,7 @@ ExecBuildProjectionInfo(List *targetList,
 			break;
 		}
 		attr = inputDesc->attrs[variable->varattno - 1];
-		if (attr->attisdropped ||
-			variable->vartype != attr->atttypid ||
-			(variable->vartypmod != attr->atttypmod &&
-			 variable->vartypmod != -1))
+		if (attr->attisdropped || variable->vartype != attr->atttypid)
 		{
 			isVarList = false;
 			break;

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`	`*`
`9`	`9`	`*`
`10`	`10`	`* IDENTIFICATION`
`11`		`- * $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.143 2007/02/02 00:07:03 tgl Exp $`
	`11`	`+ * $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.144 2007/02/06 17:35:20 tgl Exp $`
`12`	`12`	`*`
`13`	`13`	`*-------------------------------------------------------------------------`
`14`	`14`	`*/`
`@@ -632,10 +632,7 @@ ExecBuildProjectionInfo(List *targetList,`
`632`	`632`	`break;`
`633`	`633`	`}`
`634`	`634`	`attr = inputDesc->attrs[variable->varattno - 1];`
`635`		`- if (attr->attisdropped \|\|`
`636`		`- variable->vartype != attr->atttypid \|\|`
`637`		`- (variable->vartypmod != attr->atttypmod &&`
`638`		`- variable->vartypmod != -1))`
	`635`	`+ if (attr->attisdropped \|\| variable->vartype != attr->atttypid)`
`639`	`636`	`{`
`640`	`637`	`isVarList = false;`
`641`	`638`	`break;`