diff options
| -rw-r--r-- | doc/src/sgml/ref/create_function.sgml | 50 | ||||
| -rw-r--r-- | doc/src/sgml/release.sgml | 55 | ||||
| -rw-r--r-- | doc/src/sgml/runtime.sgml | 16 | ||||
| -rw-r--r-- | src/backend/catalog/namespace.c | 165 | ||||
| -rw-r--r-- | src/test/regress/expected/temp.out | 58 | ||||
| -rw-r--r-- | src/test/regress/sql/temp.sql | 33 |
6 files changed, 348 insertions, 29 deletions
diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml index 26c066cd52f..42553f52203 100644 --- a/doc/src/sgml/ref/create_function.sgml +++ b/doc/src/sgml/ref/create_function.sgml @@ -1,5 +1,5 @@ <!-- -$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_function.sgml,v 1.53.2.1 2003/11/12 20:05:15 petere Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_function.sgml,v 1.53.2.2 2007/04/20 02:38:44 tgl Exp $ --> <refentry id="SQL-CREATEFUNCTION"> @@ -382,6 +382,54 @@ CREATE FUNCTION add(integer, integer) RETURNS integer </para> </refsect1> + <refsect1 id="sql-createfunction-security"> + <title>Writing <literal>SECURITY DEFINER</literal> Functions Safely</title> + + <para> + Because a <literal>SECURITY DEFINER</literal> function is executed + with the privileges of the user that created it, care is needed to + ensure that the function cannot be misused. For security, + <xref linkend="guc-search-path"> should be set to exclude any schemas + writable by untrusted users. This prevents + malicious users from creating objects that mask objects used by the + function. Particularly important is in this regard is the + temporary-table schema, which is searched first by default, and + is normally writable by anyone. A secure arrangement can be had + by forcing the temporary schema to be searched last. To do this, + write <literal>pg_temp</> as the last entry in <varname>search_path</>. + This function illustrates safe usage: + </para> + +<programlisting> +CREATE FUNCTION check_password(TEXT, TEXT) +RETURNS BOOLEAN AS ' +DECLARE passed BOOLEAN; + old_path TEXT; +BEGIN + -- Save old search_path; notice we must qualify current_setting + -- to ensure we invoke the right function + old_path := pg_catalog.current_setting(''search_path''); + + -- Set a secure search_path: trusted schemas, then ''pg_temp''. + -- We set is_local = true so that the old value will be restored + -- in event of an error before we reach the function end. + PERFORM pg_catalog.set_config(''search_path'', ''admin, pg_temp'', true); + + -- Do whatever secure work we came for. + SELECT (pwd = $2) INTO passed + FROM pwds + WHERE username = $1; + + -- Restore caller''s search_path + PERFORM pg_catalog.set_config(''search_path'', old_path, true); + + RETURN passed; +END; +' LANGUAGE plpgsql SECURITY DEFINER; +</programlisting> + + </refsect1> + <refsect1 id="sql-createfunction-compat"> <title>Compatibility</title> diff --git a/doc/src/sgml/release.sgml b/doc/src/sgml/release.sgml index b1aac7d1377..8f375baf399 100644 --- a/doc/src/sgml/release.sgml +++ b/doc/src/sgml/release.sgml @@ -1,5 +1,5 @@ <!-- -$Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.235.2.52 2007/04/19 13:01:59 momjian Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.235.2.53 2007/04/20 02:38:44 tgl Exp $ --> <appendix id="release"> @@ -14,7 +14,8 @@ $Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.235.2.52 2007/04/19 13:01: </note> <para> - This release contains a variety of fixes from 7.4.16. + This release contains fixes from 7.4.16, + including a security fix. </para> <sect2> @@ -35,13 +36,37 @@ $Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.235.2.52 2007/04/19 13:01: <listitem> <para> - <filename>/contrib/tsearch2</> fixes (Teodor) + Support explicit placement of the temporary-table schema within + <varname>search_path</>, and disable searching it for functions + and operators (Tom) + </para> + <para> + This is needed to allow a security-definer function to set a + truly secure value of <varname>search_path</>. Without it, + an unprivileged SQL user can use temporary objects to execute code + with the privileges of the security-definer function (CVE-2007-2138). + See <xref linkend="sql-createfunction" + endterm="sql-createfunction-title"> for more information. + </para> + </listitem> + + <listitem> + <para> + <filename>/contrib/tsearch2</> crash fixes (Teodor) + </para> + </listitem> + + <listitem> + <para> + Fix potential-data-corruption bug in how <command>VACUUM FULL</> handles + <command>UPDATE</> chains (Tom, Pavan Deolasee) </para> </listitem> <listitem> <para> - Fix bug in how <command>VACUUM FULL</> handles <command>UPDATE</> chains (Tom, Pavan Deolasee) + Fix PANIC during enlargement of a hash index (bug introduced in 7.4.15) + (Tom) </para> </listitem> @@ -3174,7 +3199,8 @@ DROP SCHEMA information_schema CASCADE; </note> <para> - This release contains a variety of fixes from 7.3.18. + This release contains fixes from 7.3.18, + including a security fix. </para> <sect2> @@ -3195,7 +3221,24 @@ DROP SCHEMA information_schema CASCADE; <listitem> <para> - Fix bug in how <command>VACUUM FULL</> handles <command>UPDATE</> chains (Tom, Pavan Deolasee) + Support explicit placement of the temporary-table schema within + <varname>search_path</>, and disable searching it for functions + and operators (Tom) + </para> + <para> + This is needed to allow a security-definer function to set a + truly secure value of <varname>search_path</>. Without it, + an unprivileged SQL user can use temporary objects to execute code + with the privileges of the security-definer function (CVE-2007-2138). + See <xref linkend="sql-createfunction" + endterm="sql-createfunction-title"> for more information. + </para> + </listitem> + + <listitem> + <para> + Fix potential-data-corruption bug in how <command>VACUUM FULL</> handles + <command>UPDATE</> chains (Tom, Pavan Deolasee) </para> </listitem> diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index 91b59d0f86a..58aa9650bc7 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -1,5 +1,5 @@ <!-- -$Header: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v 1.215.2.9 2006/05/21 20:11:58 tgl Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v 1.215.2.10 2007/04/20 02:38:46 tgl Exp $ --> <Chapter Id="runtime"> @@ -1980,9 +1980,17 @@ SET ENABLE_SEQSCAN TO OFF; mentioned in the path then it will be searched in the specified order. If <literal>pg_catalog</> is not in the path then it will be searched <emphasis>before</> searching any of the path items. - It should also be noted that the temporary-table schema, - <literal>pg_temp_<replaceable>nnn</></>, is implicitly searched before any of - these. + </para> + + <para> + Likewise, the current session's temporary-table schema, + <literal>pg_temp_<replaceable>nnn</></>, is always searched if it + exists. It can be explicitly listed in the path by using the + alias <literal>pg_temp</>. If it is not listed in the path then + it is searched first (before even <literal>pg_catalog</>). However, + the temporary schema is only searched for relation (table, view, + sequence, etc) and data type names. It will never be searched for + function or operator names. </para> <para> diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 8955de675fc..2f4fc400d7a 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -13,7 +13,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/catalog/namespace.c,v 1.58 2003/09/25 06:57:57 petere Exp $ + * $Header: /cvsroot/pgsql/src/backend/catalog/namespace.c,v 1.58.2.1 2007/04/20 02:38:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -64,14 +64,32 @@ * SQL99. Also, this provides a way to search the system namespace first * without thereby making it the default creation target namespace.) * + * For security reasons, searches using the search path will ignore the temp + * namespace when searching for any object type other than relations and + * types. (We must allow types since temp tables have rowtypes.) + * * The default creation target namespace is normally equal to the first * element of the explicit list, but is the "special" namespace when one * has been set. If the explicit list is empty and there is no special * namespace, there is no default target. * - * In bootstrap mode, the search path is set equal to 'pg_catalog', so that + * The textual specification of search_path can include "$user" to refer to + * the namespace named the same as the current user, if any. (This is just + * ignored if there is no such namespace.) Also, it can include "pg_temp" + * to refer to the current backend's temp namespace. This is usually also + * ignorable if the temp namespace hasn't been set up, but there's a special + * case: if "pg_temp" appears first then it should be the default creation + * target. We kluge this case a little bit so that the temp namespace isn't + * set up until the first attempt to create something in it. (The reason for + * klugery is that we can't create the temp namespace outside a transaction, + * but initial GUC processing of search_path happens outside a transaction.) + * tempCreationPending is TRUE if "pg_temp" appears first in the string but + * is not reflected in defaultCreationNamespace because the namespace isn't + * set up yet. + * + * In bootstrap mode, the search path is set equal to "pg_catalog", so that * the system namespace is the only one searched or inserted into. - * The initdb script is also careful to set search_path to 'pg_catalog' for + * The initdb script is also careful to set search_path to "pg_catalog" for * its post-bootstrap standalone backend runs. Otherwise the default search * path is determined by GUC. The factory default path contains the PUBLIC * namespace (if it exists), preceded by the user's personal namespace @@ -99,7 +117,10 @@ static Oid defaultCreationNamespace = InvalidOid; /* first explicit member of list; usually same as defaultCreationNamespace */ static Oid firstExplicitNamespace = InvalidOid; -/* The above four values are valid only if namespaceSearchPathValid */ +/* if TRUE, defaultCreationNamespace is wrong, it should be temp namespace */ +static bool tempCreationPending = false; + +/* The above five values are valid only if namespaceSearchPathValid */ static bool namespaceSearchPathValid = true; /* @@ -235,6 +256,14 @@ RangeVarGetCreationNamespace(const RangeVar *newRelation) if (newRelation->schemaname) { + /* check for pg_temp alias */ + if (strcmp(newRelation->schemaname, "pg_temp") == 0) + { + /* Initialize temp namespace if first time through */ + if (!OidIsValid(myTempNamespace)) + InitTempTableNamespace(); + return myTempNamespace; + } /* use exact schema given */ namespaceId = GetSysCacheOid(NAMESPACENAME, CStringGetDatum(newRelation->schemaname), @@ -250,6 +279,12 @@ RangeVarGetCreationNamespace(const RangeVar *newRelation) { /* use the default creation namespace */ recomputeNamespacePath(); + if (tempCreationPending) + { + /* Need to initialize temp namespace */ + InitTempTableNamespace(); + return myTempNamespace; + } namespaceId = defaultCreationNamespace; if (!OidIsValid(namespaceId)) ereport(ERROR, @@ -490,12 +525,16 @@ FuncnameGetCandidates(List *names, int nargs) } else { - /* Consider only procs that are in the search path */ + /* + * Consider only procs that are in the search path and are not + * in the temp namespace. + */ List *nsp; foreach(nsp, namespaceSearchPath) { - if (procform->pronamespace == lfirsto(nsp)) + if (procform->pronamespace == lfirsto(nsp) && + procform->pronamespace != myTempNamespace) break; pathpos++; } @@ -705,12 +744,16 @@ OpernameGetCandidates(List *names, char oprkind) } else { - /* Consider only opers that are in the search path */ + /* + * Consider only opers that are in the search path and are not + * in the temp namespace. + */ List *nsp; foreach(nsp, namespaceSearchPath) { - if (operform->oprnamespace == lfirsto(nsp)) + if (operform->oprnamespace == lfirsto(nsp) && + operform->oprnamespace != myTempNamespace) break; pathpos++; } @@ -886,7 +929,8 @@ OpclassGetCandidates(Oid amid) /* Consider only opclasses that are in the search path */ foreach(nsp, namespaceSearchPath) { - if (opcform->opcnamespace == lfirsto(nsp)) + if (opcform->opcnamespace == lfirsto(nsp) && + opcform->opcnamespace != myTempNamespace) break; pathpos++; } @@ -984,6 +1028,9 @@ OpclassnameGetOpcid(Oid amid, const char *opcname) { Oid namespaceId = lfirsto(lptr); + if (namespaceId == myTempNamespace) + continue; /* do not look in temp namespace */ + opcid = GetSysCacheOid(CLAAMNAMENSP, ObjectIdGetDatum(amid), PointerGetDatum(opcname), @@ -1066,6 +1113,9 @@ ConversionGetConid(const char *conname) { Oid namespaceId = lfirsto(lptr); + if (namespaceId == myTempNamespace) + continue; /* do not look in temp namespace */ + conid = GetSysCacheOid(CONNAMENSP, PointerGetDatum(conname), ObjectIdGetDatum(namespaceId), @@ -1191,6 +1241,19 @@ LookupExplicitNamespace(const char *nspname) Oid namespaceId; AclResult aclresult; + /* check for pg_temp alias */ + if (strcmp(nspname, "pg_temp") == 0) + { + if (OidIsValid(myTempNamespace)) + return myTempNamespace; + /* + * Since this is used only for looking up existing objects, there + * is no point in trying to initialize the temp namespace here; + * and doing so might create problems for some callers. + * Just fall through and give the "does not exist" error. + */ + } + namespaceId = GetSysCacheOid(NAMESPACENAME, CStringGetDatum(nspname), 0, 0, 0); @@ -1213,21 +1276,28 @@ LookupExplicitNamespace(const char *nspname) * format), determine what namespace the object should be created in. * Also extract and return the object name (last component of list). * - * This is *not* used for tables. Hence, the TEMP table namespace is - * never selected as the creation target. + * Note: calling this may result in a CommandCounterIncrement operation, + * if we have to create or clean out the temp namespace. */ Oid QualifiedNameGetCreationNamespace(List *names, char **objname_p) { char *schemaname; - char *objname; Oid namespaceId; /* deconstruct the name list */ - DeconstructQualifiedName(names, &schemaname, &objname); + DeconstructQualifiedName(names, &schemaname, objname_p); if (schemaname) { + /* check for pg_temp alias */ + if (strcmp(schemaname, "pg_temp") == 0) + { + /* Initialize temp namespace if first time through */ + if (!OidIsValid(myTempNamespace)) + InitTempTableNamespace(); + return myTempNamespace; + } /* use exact schema given */ namespaceId = GetSysCacheOid(NAMESPACENAME, CStringGetDatum(schemaname), @@ -1242,6 +1312,12 @@ QualifiedNameGetCreationNamespace(List *names, char **objname_p) { /* use the default creation namespace */ recomputeNamespacePath(); + if (tempCreationPending) + { + /* Need to initialize temp namespace */ + InitTempTableNamespace(); + return myTempNamespace; + } namespaceId = defaultCreationNamespace; if (!OidIsValid(namespaceId)) ereport(ERROR, @@ -1251,7 +1327,6 @@ QualifiedNameGetCreationNamespace(List *names, char **objname_p) /* Note: callers will check for CREATE rights when appropriate */ - *objname_p = objname; return namespaceId; } @@ -1431,6 +1506,10 @@ FindConversionByName(List *name) foreach(lptr, namespaceSearchPath) { namespaceId = lfirsto(lptr); + + if (namespaceId == myTempNamespace) + continue; /* do not look in temp namespace */ + conoid = FindConversion(conversion_name, namespaceId); if (OidIsValid(conoid)) return conoid; @@ -1456,6 +1535,9 @@ FindDefaultConversionProc(int4 for_encoding, int4 to_encoding) { Oid namespaceId = lfirsto(lptr); + if (namespaceId == myTempNamespace) + continue; /* do not look in temp namespace */ + proc = FindDefaultConversion(namespaceId, for_encoding, to_encoding); if (OidIsValid(proc)) return proc; @@ -1477,6 +1559,7 @@ recomputeNamespacePath(void) List *oidlist; List *newpath; List *l; + bool temp_missing; Oid firstNS; MemoryContext oldcxt; @@ -1504,6 +1587,7 @@ recomputeNamespacePath(void) * has already been accepted.) Don't make duplicate entries, either. */ oidlist = NIL; + temp_missing = false; foreach(l, namelist) { char *curname = (char *) lfirst(l); @@ -1533,6 +1617,21 @@ recomputeNamespacePath(void) oidlist = lappendo(oidlist, namespaceId); } } + else if (strcmp(curname, "pg_temp") == 0) + { + /* pg_temp --- substitute temp namespace, if any */ + if (OidIsValid(myTempNamespace)) + { + if (!oidMember(myTempNamespace, oidlist)) + oidlist = lappendo(oidlist, myTempNamespace); + } + else + { + /* If it ought to be the creation namespace, set flag */ + if (oidlist == NIL) + temp_missing = true; + } + } else { /* normal namespace reference */ @@ -1548,7 +1647,9 @@ recomputeNamespacePath(void) } /* - * Remember the first member of the explicit list. + * Remember the first member of the explicit list. (Note: this is + * nominally wrong if temp_missing, but we need it anyway to distinguish + * explicit from implicit mention of pg_catalog.) */ if (oidlist == NIL) firstNS = InvalidOid; @@ -1588,9 +1689,16 @@ recomputeNamespacePath(void) */ firstExplicitNamespace = firstNS; if (OidIsValid(mySpecialNamespace)) + { defaultCreationNamespace = mySpecialNamespace; + /* don't have to create temp in this state */ + tempCreationPending = false; + } else + { defaultCreationNamespace = firstNS; + tempCreationPending = temp_missing; + } /* Mark the path valid. */ namespaceSearchPathValid = true; @@ -1612,6 +1720,8 @@ InitTempTableNamespace(void) char namespaceName[NAMEDATALEN]; Oid namespaceId; + Assert(!OidIsValid(myTempNamespace)); + /* * First, do permission check to see if we are authorized to make temp * tables. We use a nonstandard error message here since @@ -1782,9 +1892,9 @@ assign_search_path(const char *newval, bool doit, bool interactive) { /* * Verify that all the names are either valid namespace names or - * "$user". We do not require $user to correspond to a valid - * namespace. We do not check for USAGE rights, either; should - * we? + * "$user" or "pg_temp". We do not require $user to correspond to a + * valid namespace, and pg_temp might not exist yet. We do not check + * for USAGE rights, either; should we? */ foreach(l, namelist) { @@ -1792,6 +1902,8 @@ assign_search_path(const char *newval, bool doit, bool interactive) if (strcmp(curname, "$user") == 0) continue; + if (strcmp(curname, "pg_temp") == 0) + continue; if (!SearchSysCacheExists(NAMESPACENAME, CStringGetDatum(curname), 0, 0, 0)) @@ -1837,6 +1949,7 @@ InitializeSearchPath(void) MemoryContextSwitchTo(oldcxt); defaultCreationNamespace = PG_CATALOG_NAMESPACE; firstExplicitNamespace = PG_CATALOG_NAMESPACE; + tempCreationPending = false; namespaceSearchPathValid = true; namespaceUser = GetUserId(); } @@ -1872,6 +1985,9 @@ NamespaceCallback(Datum arg, Oid relid) * includeImplicit is true. * * NB: caller must treat the list as read-only! + * + * Note: calling this may result in a CommandCounterIncrement operation, + * if we have to create or clean out the temp namespace. */ List * fetch_search_path(bool includeImplicit) @@ -1880,6 +1996,19 @@ fetch_search_path(bool includeImplicit) recomputeNamespacePath(); + /* + * If the temp namespace should be first, force it to exist. This is + * so that callers can trust the result to reflect the actual default + * creation namespace. It's a bit bogus to do this here, since + * current_schema() is supposedly a stable function without side-effects, + * but the alternatives seem worse. + */ + if (tempCreationPending) + { + InitTempTableNamespace(); + recomputeNamespacePath(); + } + result = namespaceSearchPath; if (!includeImplicit) { diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out index 650f5f704ed..13f1f28d64b 100644 --- a/src/test/regress/expected/temp.out +++ b/src/test/regress/expected/temp.out @@ -82,3 +82,61 @@ ERROR: relation "temptest" does not exist -- ON COMMIT is only allowed for TEMP CREATE TABLE temptest(col int) ON COMMIT DELETE ROWS; ERROR: ON COMMIT can only be used on temporary tables +-- Test manipulation of temp schema's placement in search path +create table public.whereami (f1 text); +insert into public.whereami values ('public'); +create temp table whereami (f1 text); +insert into whereami values ('temp'); +create function public.whoami() returns text + as 'select ''public''::text' language sql; +create function pg_temp.whoami() returns text + as 'select ''temp''::text' language sql; +-- default should have pg_temp implicitly first, but only for tables +select * from whereami; + f1 +------ + temp +(1 row) + +select whoami(); + whoami +-------- + public +(1 row) + +-- can list temp first explicitly, but it still doesn't affect functions +set search_path = pg_temp, public; +select * from whereami; + f1 +------ + temp +(1 row) + +select whoami(); + whoami +-------- + public +(1 row) + +-- or put it last for security +set search_path = public, pg_temp; +select * from whereami; + f1 +-------- + public +(1 row) + +select whoami(); + whoami +-------- + public +(1 row) + +-- you can invoke a temp function explicitly, though +select pg_temp.whoami(); + whoami +-------- + temp +(1 row) + +drop table public.whereami; diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql index 397d00bfd94..c7e0e035e40 100644 --- a/src/test/regress/sql/temp.sql +++ b/src/test/regress/sql/temp.sql @@ -83,3 +83,36 @@ SELECT * FROM temptest; -- ON COMMIT is only allowed for TEMP CREATE TABLE temptest(col int) ON COMMIT DELETE ROWS; + +-- Test manipulation of temp schema's placement in search path + +create table public.whereami (f1 text); +insert into public.whereami values ('public'); + +create temp table whereami (f1 text); +insert into whereami values ('temp'); + +create function public.whoami() returns text + as 'select ''public''::text' language sql; + +create function pg_temp.whoami() returns text + as 'select ''temp''::text' language sql; + +-- default should have pg_temp implicitly first, but only for tables +select * from whereami; +select whoami(); + +-- can list temp first explicitly, but it still doesn't affect functions +set search_path = pg_temp, public; +select * from whereami; +select whoami(); + +-- or put it last for security +set search_path = public, pg_temp; +select * from whereami; +select whoami(); + +-- you can invoke a temp function explicitly, though +select pg_temp.whoami(); + +drop table public.whereami; |