Ignore SECURITY DEFINER and SET attributes for a PL's call handler.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 31 May 2012 03:27:57 +0000 (23:27 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 31 May 2012 03:27:57 +0000 (23:27 -0400)
It's not very sensible to set such attributes on a handler function;
but if one were to do so, fmgr.c went into infinite recursion because
it would call fmgr_security_definer instead of the handler function proper.
There is no way for fmgr_security_definer to know that it ought to call the
handler and not the original function referenced by the FmgrInfo's fn_oid,
so it tries to do the latter, causing the whole process to start over
again.

Ordinarily such misconfiguration of a procedural language's handler could
be written off as superuser error.  However, because we allow non-superuser
database owners to create procedural languages and the handler for such a
language becomes owned by the database owner, it is possible for a database
owner to crash the backend, which ideally shouldn't be possible without
superuser privileges.  In 9.2 and up we will adjust things so that the
handler functions are always owned by superusers, but in existing branches
this is a minor security fix.

Problem noted by Noah Misch (after several of us had failed to detect
it :-().  This is CVE-2012-2655.

src/backend/utils/fmgr/fmgr.c

index 46d7edaec71a4b3d54e5ee51017e6a0cc3072a10..788f1801a80761379476a958d75f705e89b08950 100644 (file)
@@ -158,7 +158,7 @@ fmgr_lookupByName(const char *name)
 void
 fmgr_info(Oid functionId, FmgrInfo *finfo)
 {
-   fmgr_info_cxt(functionId, finfo, CurrentMemoryContext);
+   fmgr_info_cxt_security(functionId, finfo, CurrentMemoryContext, false);
 }
 
 /*
@@ -173,7 +173,7 @@ fmgr_info_cxt(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt)
 
 /*
  * This one does the actual work.  ignore_security is ordinarily false
- * but is set to true by fmgr_security_definer to avoid recursion.
+ * but is set to true when we need to avoid recursion.
  */
 static void
 fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt,
@@ -223,7 +223,8 @@ fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt,
    /*
     * If it has prosecdef set, non-null proconfig, or if a plugin wants to
     * hook function entry/exit, use fmgr_security_definer call handler ---
-    * unless we are being called again by fmgr_security_definer.
+    * unless we are being called again by fmgr_security_definer or
+    * fmgr_info_other_lang.
     *
     * When using fmgr_security_definer, function stats tracking is always
     * disabled at the outer level, and instead we set the flag properly in
@@ -405,7 +406,13 @@ fmgr_info_other_lang(Oid functionId, FmgrInfo *finfo, HeapTuple procedureTuple)
        elog(ERROR, "cache lookup failed for language %u", language);
    languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
 
-   fmgr_info(languageStruct->lanplcallfoid, &plfinfo);
+   /*
+    * Look up the language's call handler function, ignoring any attributes
+    * that would normally cause insertion of fmgr_security_definer.  We
+    * need to get back a bare pointer to the actual C-language function.
+    */
+   fmgr_info_cxt_security(languageStruct->lanplcallfoid, &plfinfo,
+                          CurrentMemoryContext, true);
    finfo->fn_addr = plfinfo.fn_addr;
 
    /*