Repair insufficiently careful type checking for SQL-language functions:

we should check that the function code returns the claimed result datatype
every time we parse the function for execution.  Formerly, for simple
scalar result types we assumed the creation-time check was sufficient, but
this fails if the function selects from a table that's been redefined since
then, and even more obviously fails if check_function_bodies had been OFF.

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-0555

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index b915a02c310ac4b619fccce47ce87e98b97037d5..4e19d9fc8d26459ddb57ba35b74a36fb8624b7dc 100644 (file)
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/catalog/pg_proc.c,v 1.94 2002/09/18 21:35:20 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/catalog/pg_proc.c,v 1.94.2.1 2007/02/02 00:04:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -33,7 +33,6 @@
 #include "utils/syscache.h"
 
 
-static void checkretval(Oid rettype, char fn_typtype, List *queryTreeList);
 Datum          fmgr_internal_validator(PG_FUNCTION_ARGS);
 Datum          fmgr_c_validator(PG_FUNCTION_ARGS);
 Datum          fmgr_sql_validator(PG_FUNCTION_ARGS);
@@ -294,15 +293,15 @@ ProcedureCreate(const char *procedureName,
 }
 
 /*
- * checkretval() -- check return value of a list of sql parse trees.
+ * check_sql_fn_retval() -- check return value of a list of sql parse trees.
  *
  * The return value of a sql function is the value returned by
  * the final query in the function.  We do some ad-hoc define-time
  * type checking here to be sure that the user is returning the
  * type he claims.
  */
-static void
-checkretval(Oid rettype, char fn_typtype, List *queryTreeList)
+void
+check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList)
 {
        Query      *parse;
        int                     cmd;
@@ -592,7 +591,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
        prosrc = DatumGetCString(DirectFunctionCall1(textout, tmp));
 
        querytree_list = pg_parse_and_rewrite(prosrc, proc->proargtypes, proc->pronargs);
-       checkretval(proc->prorettype, functyptype, querytree_list);
+       check_sql_fn_retval(proc->prorettype, functyptype, querytree_list);
 
        ReleaseSysCache(tuple);
 

diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 9ee7ae1fca1fa0c05e84714bd0e33ee302998748..9e7bc76b1eede5f0948bcdc45f6b028032e760ae 100644 (file)
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/executor/functions.c,v 1.57.2.1 2003/06/12 17:29:37 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/executor/functions.c,v 1.57.2.2 2007/02/02 00:04:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -24,6 +24,7 @@
 #include "tcop/tcopprot.h"
 #include "tcop/utility.h"
 #include "utils/builtins.h"
+#include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
 
@@ -53,7 +54,7 @@ typedef struct local_es
 
 typedef struct
 {
-       int                     typlen;                 /* length of the return type */
+       int16           typlen;                 /* length of the return type */
        bool            typbyval;               /* true if return type is pass by value */
        bool            returnsTuple;   /* true if return type is a tuple */
        bool            shutdown_reg;   /* true if registered shutdown callback */
@@ -71,7 +72,8 @@ typedef SQLFunctionCache *SQLFunctionCachePtr;
 
 /* non-export function prototypes */
 static execution_state *init_execution_state(char *src,
-                                        Oid *argOidVect, int nargs);
+                                        Oid *argOidVect, int nargs,
+                                        Oid rettype);
 static void init_sql_fcache(FmgrInfo *finfo);
 static void postquel_start(execution_state *es);
 static TupleTableSlot *postquel_getnext(execution_state *es);
@@ -84,7 +86,8 @@ static void ShutdownSQLFunction(Datum arg);
 
 
 static execution_state *
-init_execution_state(char *src, Oid *argOidVect, int nargs)
+init_execution_state(char *src, Oid *argOidVect, int nargs,
+                                        Oid rettype)
 {
        execution_state *firstes;
        execution_state *preves;
@@ -93,6 +96,14 @@ init_execution_state(char *src, Oid *argOidVect, int nargs)
 
        queryTree_list = pg_parse_and_rewrite(src, argOidVect, nargs);
 
+       /*
+        * Check that the function returns the type it claims to.  Although
+        * this was already done when the function was defined,
+        * we have to recheck because database objects used in the function's
+        * queries might have changed type.
+        */
+       check_sql_fn_retval(rettype, get_typtype(rettype), queryTree_list);
+
        firstes = NULL;
        preves = NULL;
 
@@ -150,6 +161,7 @@ static void
 init_sql_fcache(FmgrInfo *finfo)
 {
        Oid                     foid = finfo->fn_oid;
+       Oid                     rettype;
        HeapTuple       procedureTuple;
        HeapTuple       typeTuple;
        Form_pg_proc procedureStruct;
@@ -176,12 +188,14 @@ init_sql_fcache(FmgrInfo *finfo)
        /*
         * get the return type from the procedure tuple
         */
+       rettype = procedureStruct->prorettype;
+
        typeTuple = SearchSysCache(TYPEOID,
-                                                  ObjectIdGetDatum(procedureStruct->prorettype),
+                                                          ObjectIdGetDatum(rettype),
                                                           0, 0, 0);
        if (!HeapTupleIsValid(typeTuple))
                elog(ERROR, "init_sql_fcache: Cache lookup failed for type %u",
-                        procedureStruct->prorettype);
+                        rettype);
 
        typeStruct = (Form_pg_type) GETSTRUCT(typeTuple);
 
@@ -194,7 +208,7 @@ init_sql_fcache(FmgrInfo *finfo)
        fcache->typlen = typeStruct->typlen;
 
        if (typeStruct->typtype != 'c' &&
-               procedureStruct->prorettype != RECORDOID)
+               rettype != RECORDOID)
        {
                /* The return type is not a composite type, so just use byval */
                fcache->typbyval = typeStruct->typbyval;
@@ -243,7 +257,7 @@ init_sql_fcache(FmgrInfo *finfo)
                         foid);
        src = DatumGetCString(DirectFunctionCall1(textout, tmp));
 
-       fcache->func_state = init_execution_state(src, argOidVect, nargs);
+       fcache->func_state = init_execution_state(src, argOidVect, nargs, rettype);
 
        pfree(src);
 

diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index eb35e86893f37cd1be31465f3acf04eb25ccb3a7..943a9927ea347c115ca8ce1eec71588782bd3334 100644 (file)
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: pg_proc.h,v 1.275.2.3 2006/11/28 19:19:25 tgl Exp $
+ * $Id: pg_proc.h,v 1.275.2.4 2007/02/02 00:04:16 tgl Exp $
  *
  * NOTES
  *       The script catalog/genbki.sh reads this file and generates .bki
@@ -3150,4 +3150,7 @@ extern Oid ProcedureCreate(const char *procedureName,
                                int parameterCount,
                                const Oid *parameterTypes);
 
+extern void check_sql_fn_retval(Oid rettype, char fn_typtype,
+                                       List *queryTreeList);
+
 #endif   /* PG_PROC_H */