diff options
| -rw-r--r-- | doc/src/sgml/ref/create_function.sgml | 50 | ||||
| -rw-r--r-- | doc/src/sgml/release.sgml | 24 | ||||
| -rw-r--r-- | doc/src/sgml/runtime.sgml | 16 | ||||
| -rw-r--r-- | src/backend/catalog/namespace.c | 162 | ||||
| -rw-r--r-- | src/test/regress/expected/temp.out | 58 | ||||
| -rw-r--r-- | src/test/regress/sql/temp.sql | 32 |
6 files changed, 316 insertions, 26 deletions
diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml index 6c737f26141..cc4a138ae82 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.43 2002/09/21 18:32:54 petere Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_function.sgml,v 1.43.2.1 2007/04/20 02:38:57 tgl Exp $ --> <refentry id="SQL-CREATEFUNCTION"> @@ -448,6 +448,54 @@ Point * complex_to_point (Complex *z) </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 10177e0ca93..e252be51a58 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.163.2.41 2007/04/19 13:01:44 momjian Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.163.2.42 2007/04/20 02:38:57 tgl Exp $ --> <appendix id="release"> @@ -14,7 +14,8 @@ $Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.163.2.41 2007/04/19 13:01: </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> @@ -35,7 +36,24 @@ $Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.163.2.41 2007/04/19 13:01: <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 718e90d8f85..a4beea16c5f 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.147.2.8 2006/05/21 20:12:20 tgl Exp $ +$Header: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v 1.147.2.9 2007/04/20 02:38:58 tgl Exp $ --> <Chapter Id="runtime"> @@ -1769,9 +1769,17 @@ dynamic_library_path = '/usr/local/lib/postgresql:/home/my_project/lib:$libdir' 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 5450b6d110f..c2a450a6c92 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.38.2.2 2003/03/06 22:55:03 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/catalog/namespace.c,v 1.38.2.3 2007/04/20 02:38:58 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -63,14 +63,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 @@ -98,7 +116,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; /* @@ -223,6 +244,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), @@ -236,6 +265,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)) elog(ERROR, "No namespace has been selected to create in"); @@ -473,12 +508,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 == (Oid) lfirsti(nsp)) + if (procform->pronamespace == (Oid) lfirsti(nsp) && + procform->pronamespace != myTempNamespace) break; pathpos++; } @@ -688,12 +727,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 == (Oid) lfirsti(nsp)) + if (operform->oprnamespace == (Oid) lfirsti(nsp) && + operform->oprnamespace != myTempNamespace) break; pathpos++; } @@ -869,7 +912,8 @@ OpclassGetCandidates(Oid amid) /* Consider only opclasses that are in the search path */ foreach(nsp, namespaceSearchPath) { - if (opcform->opcnamespace == (Oid) lfirsti(nsp)) + if (opcform->opcnamespace == (Oid) lfirsti(nsp) && + opcform->opcnamespace != myTempNamespace) break; pathpos++; } @@ -967,6 +1011,9 @@ OpclassnameGetOpcid(Oid amid, const char *opcname) { Oid namespaceId = (Oid) lfirsti(lptr); + if (namespaceId == myTempNamespace) + continue; /* do not look in temp namespace */ + opcid = GetSysCacheOid(CLAAMNAMENSP, ObjectIdGetDatum(amid), PointerGetDatum(opcname), @@ -1089,6 +1136,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); @@ -1108,21 +1168,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), @@ -1136,6 +1203,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)) elog(ERROR, "No namespace has been selected to create in"); @@ -1143,7 +1216,6 @@ QualifiedNameGetCreationNamespace(List *names, char **objname_p) /* Note: callers will check for CREATE rights when appropriate */ - *objname_p = objname; return namespaceId; } @@ -1320,6 +1392,10 @@ FindConversionByName(List *name) foreach(lptr, namespaceSearchPath) { namespaceId = (Oid) lfirsti(lptr); + + if (namespaceId == myTempNamespace) + continue; /* do not look in temp namespace */ + conoid = FindConversion(conversion_name, namespaceId); if (OidIsValid(conoid)) return conoid; @@ -1345,6 +1421,9 @@ FindDefaultConversionProc(int4 for_encoding, int4 to_encoding) { Oid namespaceId = (Oid) lfirsti(lptr); + if (namespaceId == myTempNamespace) + continue; /* do not look in temp namespace */ + proc = FindDefaultConversion(namespaceId, for_encoding, to_encoding); if (OidIsValid(proc)) return proc; @@ -1366,6 +1445,7 @@ recomputeNamespacePath(void) List *oidlist; List *newpath; List *l; + bool temp_missing; Oid firstNS; MemoryContext oldcxt; @@ -1393,6 +1473,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); @@ -1422,6 +1503,21 @@ recomputeNamespacePath(void) oidlist = lappendi(oidlist, namespaceId); } } + else if (strcmp(curname, "pg_temp") == 0) + { + /* pg_temp --- substitute temp namespace, if any */ + if (OidIsValid(myTempNamespace)) + { + if (!intMember(myTempNamespace, oidlist)) + oidlist = lappendi(oidlist, myTempNamespace); + } + else + { + /* If it ought to be the creation namespace, set flag */ + if (oidlist == NIL) + temp_missing = true; + } + } else { /* normal namespace reference */ @@ -1437,7 +1533,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; @@ -1477,9 +1575,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; @@ -1501,6 +1606,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 @@ -1669,9 +1776,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) { @@ -1679,6 +1786,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)) @@ -1722,6 +1831,7 @@ InitializeSearchPath(void) MemoryContextSwitchTo(oldcxt); defaultCreationNamespace = PG_CATALOG_NAMESPACE; firstExplicitNamespace = PG_CATALOG_NAMESPACE; + tempCreationPending = false; namespaceSearchPathValid = true; namespaceUser = GetUserId(); } @@ -1757,6 +1867,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) @@ -1765,6 +1878,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 1800b9ebe56..8585255961b 100644 --- a/src/test/regress/expected/temp.out +++ b/src/test/regress/expected/temp.out @@ -36,3 +36,61 @@ CREATE TEMP TABLE temptest(col int); SET autocommit TO 'on'; SELECT * FROM temptest; ERROR: Relation "temptest" does not exist +-- 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 6f45b2eaac7..64764dbf9fc 100644 --- a/src/test/regress/sql/temp.sql +++ b/src/test/regress/sql/temp.sql @@ -48,3 +48,35 @@ SET autocommit TO 'on'; SELECT * FROM temptest; +-- 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; |