Prevent PL/Tcl from loading the "unknown" module from pltcl_modules unless

tglsfdc · tglsfdc · commit 4aadfba465c1 · 2010-05-13T18:29:54.000Z
that is a regular table or view owned by a superuser. This prevents a trojan horse attack whereby any unprivileged SQL user could create such a table and insert code into it that would then get executed in other users' sessions whenever they call pltcl functions. Worse yet, because the code was automatically loaded into both the "normal" and "safe" interpreters at first use, the attacker could execute unrestricted Tcl code in the "normal" interpreter without there being any pltclu functions anywhere, or indeed anyone else using pltcl at all: installing pltcl is sufficient to open the hole. Change the initialization logic so that the "unknown" code is only loaded into an interpreter when the interpreter is first really used. (That doesn't add any additional security in this particular context, but it seems a prudent change, and anyway the former behavior violated the principle of least astonishment.) Security: CVE-2010-1170
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
@@ -1,5 +1,5 @@
 <!--
-$Header: /cvsroot/pgsql/doc/src/sgml/pltcl.sgml,v 2.26.2.2 2004/01/24 23:06:41 tgl Exp $
+$Header: /cvsroot/pgsql/doc/src/sgml/pltcl.sgml,v 2.26.2.3 2010/05/13 18:29:54 tgl Exp $
 -->
 
  <chapter id="pltcl">
@@ -646,11 +646,13 @@ CREATE TRIGGER trig_mytab_modcount BEFORE INSERT OR UPDATE ON mytab
 	It recognizes a special table, <literal>pltcl_modules</>, which
 	is presumed to contain modules of Tcl code.  If this table
 	exists, the module <literal>unknown</> is fetched from the table
-	and loaded into the Tcl interpreter immediately after creating
-	the interpreter.
+	and loaded into the Tcl interpreter immediately before the first
+	execution of a PL/Tcl function in a database session.  (This
+	happens separately for PL/Tcl and PL/TclU, if both are used,
+	because separate interpreters are used for the two languages.)
        </para>
        <para>
-        While the <literal>unknown</> module could actually contain any
+	While the <literal>unknown</> module could actually contain any
 	initialization script you need, it normally defines a Tcl
 	<function>unknown</> procedure that is invoked whenever Tcl does
 	not recognize an invoked procedure name.  <application>PL/Tcl</>'s standard version
@@ -663,18 +665,22 @@ CREATE TRIGGER trig_mytab_modcount BEFORE INSERT OR UPDATE ON mytab
 	is reasonably quick.
        </para>
        <para>
-        The <productname>PostgreSQL</productname> distribution includes
+	The <productname>PostgreSQL</productname> distribution includes
 	support scripts to maintain these tables:
 	<command>pltcl_loadmod</>, <command>pltcl_listmod</>,
 	<command>pltcl_delmod</>, as well as source for the standard
-	<literal>unknown</> module in <filename>share/unknown.pltcl</>.  This module
+	<literal>unknown</> module in <filename>share/unknown.pltcl</>.	 This module
 	must be loaded
 	into each database initially to support the autoloading mechanism.
        </para>
        <para>
-        The tables <literal>pltcl_modules</> and <literal>pltcl_modfuncs</>
+	The tables <literal>pltcl_modules</> and <literal>pltcl_modfuncs</>
 	must be readable by all, but it is wise to make them owned and
-	writable only by the database administrator.
+	writable only by the database administrator.  As a security
+	precaution, PL/Tcl will ignore <literal>pltcl_modules</> (and thus,
+	not attempt to load the <literal>unknown</> module) unless it is
+	owned by a superuser.  But update privileges on this table can be
+	granted to other users, if you trust them sufficiently.
        </para>
    </sect1>
 
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
@@ -31,7 +31,7 @@
  *	  ENHANCEMENTS, OR MODIFICATIONS.
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/pl/tcl/pltcl.c,v 1.79.2.2 2010/01/25 01:58:57 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/pl/tcl/pltcl.c,v 1.79.2.3 2010/05/13 18:29:54 tgl Exp $
  *
  **********************************************************************/
 
@@ -49,16 +49,20 @@
 #endif
 
 #include "access/heapam.h"
+#include "catalog/namespace.h"
 #include "catalog/pg_language.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
 #include "commands/trigger.h"
 #include "executor/spi.h"
 #include "fmgr.h"
+#include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "parser/parse_type.h"
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
+#include "utils/inval.h"
+#include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
 #if defined(UNICODE_CONVERSION) && TCL_MAJOR_VERSION == 8 \
@@ -129,7 +133,8 @@ typedef struct pltcl_query_desc
  * Global data
  **********************************************************************/
 static bool pltcl_pm_init_done = false;
-static bool pltcl_be_init_done = false;
+static bool pltcl_be_norm_init_done = false;
+static bool pltcl_be_safe_init_done = false;
 static int	pltcl_call_level = 0;
 static int	pltcl_restart_in_progress = 0;
 static Tcl_Interp *pltcl_hold_interp = NULL;
@@ -143,9 +148,9 @@ static FunctionCallInfo pltcl_current_fcinfo = NULL;
 /**********************************************************************
  * Forward declarations
  **********************************************************************/
-static void pltcl_init_all(void);
-static void pltcl_init_interp(Tcl_Interp *interp);
 
+static void pltcl_init_interp(Tcl_Interp *interp);
+static Tcl_Interp *pltcl_fetch_interp(bool pltrusted);
 static void pltcl_init_load_unknown(Tcl_Interp *interp);
 
 Datum		pltcl_call_handler(PG_FUNCTION_ARGS);
@@ -249,38 +254,12 @@ pltcl_init(void)
 	pltcl_pm_init_done = true;
 }
 
-/**********************************************************************
- * pltcl_init_all()		- Initialize all
- **********************************************************************/
-static void
-pltcl_init_all(void)
-{
-	/************************************************************
-	 * Execute postmaster-startup safe initialization
-	 ************************************************************/
-	if (!pltcl_pm_init_done)
-		pltcl_init();
-
-	/************************************************************
-	 * Any other initialization that must be done each time a new
-	 * backend starts:
-	 * - Try to load the unknown procedure from pltcl_modules
-	 ************************************************************/
-	if (!pltcl_be_init_done)
-	{
-		if (SPI_connect() != SPI_OK_CONNECT)
-			elog(ERROR, "SPI_connect failed");
-		pltcl_init_load_unknown(pltcl_norm_interp);
-		pltcl_init_load_unknown(pltcl_safe_interp);
-		if (SPI_finish() != SPI_OK_FINISH)
-			elog(ERROR, "SPI_finish failed");
-		pltcl_be_init_done = true;
-	}
-}
-
-
 /**********************************************************************
  * pltcl_init_interp() - initialize a Tcl interpreter
+ *
+ * The work done here must be safe to do in the postmaster process,
+ * in case the pltcl library is preloaded in the postmaster.  Note
+ * that this is applied separately to the "normal" and "safe" interpreters.
  **********************************************************************/
 static void
 pltcl_init_interp(Tcl_Interp *interp)
@@ -307,6 +286,42 @@ pltcl_init_interp(Tcl_Interp *interp)
 					  pltcl_SPI_lastoid, NULL, NULL);
 }
 
+/**********************************************************************
+ * pltcl_fetch_interp() - fetch the Tcl interpreter to use for a function
+ *
+ * This also takes care of any on-first-use initialization required.
+ * The initialization work done here can't be done in the postmaster, and
+ * hence is not safe to do at library load time, because it may invoke
+ * arbitrary user-defined code.
+ * Note: we assume caller has already connected to SPI.
+ **********************************************************************/
+static Tcl_Interp *
+pltcl_fetch_interp(bool pltrusted)
+{
+	Tcl_Interp *interp;
+
+	/* On first use, we try to load the unknown procedure from pltcl_modules */
+	if (pltrusted)
+	{
+		interp = pltcl_safe_interp;
+		if (!pltcl_be_safe_init_done)
+		{
+			pltcl_init_load_unknown(interp);
+			pltcl_be_safe_init_done = true;
+		}
+	}
+	else
+	{
+		interp = pltcl_norm_interp;
+		if (!pltcl_be_norm_init_done)
+		{
+			pltcl_init_load_unknown(interp);
+			pltcl_be_norm_init_done = true;
+		}
+	}
+
+	return interp;
+}
 
 /**********************************************************************
  * pltcl_init_load_unknown()	- Load the unknown procedure from
@@ -315,6 +330,12 @@ pltcl_init_interp(Tcl_Interp *interp)
 static void
 pltcl_init_load_unknown(Tcl_Interp *interp)
 {
+	Oid			relOid;
+	Relation	pmrel;
+	char	   *pmrelname,
+			   *nspname;
+	char	   *buf;
+	int			buflen;
 	int			spi_rc;
 	int			tcl_rc;
 	Tcl_DString unknown_src;
@@ -324,45 +345,87 @@ pltcl_init_load_unknown(Tcl_Interp *interp)
 
 	/************************************************************
 	 * Check if table pltcl_modules exists
+	 *
+	 * We allow the table to be found anywhere in the search_path.
+	 * This is for backwards compatibility.  To ensure that the table
+	 * is trustworthy, we require it to be owned by a superuser.
+	 *
+	 * this next bit of code is the same as try_relation_openrv(),
+	 * which only exists in 8.4 and up.
 	 ************************************************************/
-	spi_rc = SPI_exec("select 1 from pg_catalog.pg_class "
-					  "where relname = 'pltcl_modules'", 1);
-	SPI_freetuptable(SPI_tuptable);
-	if (spi_rc != SPI_OK_SELECT)
-		elog(ERROR, "select from pg_class failed");
-	if (SPI_processed == 0)
+
+	/* Check for shared-cache-inval messages */
+	AcceptInvalidationMessages();
+
+	/* Look up the appropriate relation using namespace search */
+	relOid = RangeVarGetRelid(makeRangeVar(NULL, "pltcl_modules"), true);
+
+	/* Drop out on not-found */
+	if (!OidIsValid(relOid))
 		return;
 
+	/* Let relation_open do the rest */
+	pmrel = relation_open(relOid, AccessShareLock);
+
+	if (pmrel == NULL)
+		return;
+	/* must be table or view, else ignore */
+	if (!(pmrel->rd_rel->relkind == RELKIND_RELATION ||
+		  pmrel->rd_rel->relkind == RELKIND_VIEW))
+	{
+		relation_close(pmrel, AccessShareLock);
+		return;
+	}
+	/* must be owned by superuser, else ignore */
+	if (!superuser_arg(pmrel->rd_rel->relowner))
+	{
+		relation_close(pmrel, AccessShareLock);
+		return;
+	}
+	/* get fully qualified table name for use in select command */
+	nspname = get_namespace_name(RelationGetNamespace(pmrel));
+	if (!nspname)
+		elog(ERROR, "cache lookup failed for namespace %u",
+			 RelationGetNamespace(pmrel));
+	pmrelname = quote_qualified_identifier(nspname,
+										   RelationGetRelationName(pmrel));
+
 	/************************************************************
-	 * Read all the row's from it where modname = 'unknown' in
-	 * the order of modseq
+	 * Read all the rows from it where modname = 'unknown',
+	 * in the order of modseq
 	 ************************************************************/
-	Tcl_DStringInit(&unknown_src);
+	buflen = strlen(pmrelname) + 100;
+	buf = (char *) palloc(buflen);
+	snprintf(buf, buflen,
+			 "select modsrc from %s where modname = 'unknown' order by modseq",
+			 pmrelname);
 
-	spi_rc = SPI_exec("select modseq, modsrc from pltcl_modules "
-					  "where modname = 'unknown' "
-					  "order by modseq", 0);
+	spi_rc = SPI_exec(buf, 0);
 	if (spi_rc != SPI_OK_SELECT)
 		elog(ERROR, "select from pltcl_modules failed");
 
+	pfree(buf);
+
 	/************************************************************
 	 * If there's nothing, module unknown doesn't exist
 	 ************************************************************/
 	if (SPI_processed == 0)
 	{
-		Tcl_DStringFree(&unknown_src);
 		SPI_freetuptable(SPI_tuptable);
 		elog(WARNING, "module \"unknown\" not found in pltcl_modules");
+		relation_close(pmrel, AccessShareLock);
 		return;
 	}
 
 	/************************************************************
-	 * There is a module named unknown. Resemble the
+	 * There is a module named unknown. Reassemble the
 	 * source from the modsrc attributes and evaluate
 	 * it in the Tcl interpreter
 	 ************************************************************/
 	fno = SPI_fnumber(SPI_tuptable->tupdesc, "modsrc");
 
+	Tcl_DStringInit(&unknown_src);
+
 	for (i = 0; i < SPI_processed; i++)
 	{
 		part = SPI_getvalue(SPI_tuptable->vals[i],
@@ -376,8 +439,19 @@ pltcl_init_load_unknown(Tcl_Interp *interp)
 		}
 	}
 	tcl_rc = Tcl_GlobalEval(interp, Tcl_DStringValue(&unknown_src));
+
 	Tcl_DStringFree(&unknown_src);
 	SPI_freetuptable(SPI_tuptable);
+
+	if (tcl_rc != TCL_OK)
+	{
+		UTF_BEGIN;
+		elog(ERROR, "could not load module \"unknown\": %s",
+			 UTF_U2E(Tcl_GetStringResult(interp)));
+		UTF_END;
+	}
+
+	relation_close(pmrel, AccessShareLock);
 }
 
 
@@ -398,9 +472,10 @@ pltcl_call_handler(PG_FUNCTION_ARGS)
 	FunctionCallInfo save_fcinfo;
 
 	/************************************************************
-	 * Initialize interpreters
+	 * Initialize interpreters if not done previously
 	 ************************************************************/
-	pltcl_init_all();
+	if (!pltcl_pm_init_done)
+		pltcl_init();
 
 	/************************************************************
 	 * Connect to SPI manager
@@ -467,10 +542,7 @@ pltcl_func_handler(PG_FUNCTION_ARGS)
 	/* Find or compile the function */
 	prodesc = compile_pltcl_function(fcinfo->flinfo->fn_oid, InvalidOid);
 
-	if (prodesc->lanpltrusted)
-		interp = pltcl_safe_interp;
-	else
-		interp = pltcl_norm_interp;
+	interp = pltcl_fetch_interp(prodesc->lanpltrusted);
 
 	/************************************************************
 	 * Create the tcl command to call the internal
@@ -654,10 +726,7 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS)
 	prodesc = compile_pltcl_function(fcinfo->flinfo->fn_oid,
 									 RelationGetRelid(trigdata->tg_relation));
 
-	if (prodesc->lanpltrusted)
-		interp = pltcl_safe_interp;
-	else
-		interp = pltcl_norm_interp;
+	interp = pltcl_fetch_interp(prodesc->lanpltrusted);
 
 	tupdesc = trigdata->tg_relation->rd_att;
 
@@ -1078,10 +1147,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid)
 		prodesc->lanpltrusted = langStruct->lanpltrusted;
 		ReleaseSysCache(langTup);
 
-		if (prodesc->lanpltrusted)
-			interp = pltcl_safe_interp;
-		else
-			interp = pltcl_norm_interp;
+		interp = pltcl_fetch_interp(prodesc->lanpltrusted);
 
 		/************************************************************
 		 * Get the required information for input conversion of the
