Documentation fix for CREATE FUNCTION

Started by Albe Laurenzover 9 years ago4 messages
#1Albe Laurenz
laurenz.albe@wien.gv.at
1 attachment(s)

I just noticed that the documentation for CREATE FUNCTION still mentions
that the temporary namespace is searched for functions even though that
has been removed with commit aa27977.

Attached is a patch to fix that.

Yours,
Laurenz Albe

Attachments:

0001-Fix-mention-of-pg_temp-in-CREATE-FUNCTION-documentat.patchapplication/octet-stream; name=0001-Fix-mention-of-pg_temp-in-CREATE-FUNCTION-documentat.patchDownload
From 7d087c68d9dea2c5ab44900205cdacdfe279158c Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@wien.gv.at>
Date: Wed, 13 Jul 2016 09:35:31 +0200
Subject: [PATCH] Fix mention of pg_temp in CREATE FUNCTION documentation

Commit aa27977 removed searching of the temporary-table schema
for functions and operators, but the documentation still stated
that pg_temp is searched.
---
 doc/src/sgml/ref/create_function.sgml |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml
index 097e2bd..7ee4acb 100644
--- a/doc/src/sgml/ref/create_function.sgml
+++ b/doc/src/sgml/ref/create_function.sgml
@@ -751,11 +751,9 @@ SELECT * FROM dup(42);
     <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 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 obtained
-    by forcing the temporary schema to be searched last.  To do this,
-    write <literal>pg_temp</><indexterm><primary>pg_temp</><secondary>securing functions</></> as the last entry in <varname>search_path</>.
+    function.
+    Note that for security reasons, the temporary-table schema
+    <literal>pg_temp</> is never searched for functions.
     This function illustrates safe usage:
    </para>
 
@@ -772,8 +770,8 @@ BEGIN
 END;
 $$  LANGUAGE plpgsql
     SECURITY DEFINER
-    -- Set a secure search_path: trusted schema(s), then 'pg_temp'.
-    SET search_path = admin, pg_temp;
+    -- Set a secure search_path with only trusted schema(s).
+    SET search_path = admin;
 </programlisting>
 
    <para>
-- 
1.7.1

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Albe Laurenz (#1)
Re: Documentation fix for CREATE FUNCTION

Albe Laurenz <laurenz.albe@wien.gv.at> writes:

I just noticed that the documentation for CREATE FUNCTION still mentions
that the temporary namespace is searched for functions even though that
has been removed with commit aa27977.

The example you propose to correct was introduced by that same commit,
which should make you think twice about whether it really was invalidated
by that commit.

I believe the reason for forcing pg_temp to the back of the path is to
prevent unqualified table names from being captured by pg_temp entries.
This risk exists despite the rule against searching pg_temp for functions
or operators. A maliciously named temp table could at least prevent
a security definer function from doing what it was supposed to, and
could probably hijack control entirely via triggers or rules.

Possibly the documentation should be more explicit about why this is
being done, but the example code is good as-is.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Albe Laurenz
laurenz.albe@wien.gv.at
In reply to: Tom Lane (#2)
1 attachment(s)
Re: Documentation fix for CREATE FUNCTION

Tom Lane wrote:

Albe Laurenz <laurenz.albe@wien.gv.at> writes:

I just noticed that the documentation for CREATE FUNCTION still mentions
that the temporary namespace is searched for functions even though that
has been removed with commit aa27977.

The example you propose to correct was introduced by that same commit,
which should make you think twice about whether it really was invalidated
by that commit.

Yes, I wondered about that.

I believe the reason for forcing pg_temp to the back of the path is to
prevent unqualified table names from being captured by pg_temp entries.
This risk exists despite the rule against searching pg_temp for functions
or operators. A maliciously named temp table could at least prevent
a security definer function from doing what it was supposed to, and
could probably hijack control entirely via triggers or rules.

Possibly the documentation should be more explicit about why this is
being done, but the example code is good as-is.

Maybe something like the attached would keep people like me from
misunderstanding this.

Yours,
Laurenz Albe

Attachments:

0001-Improve-example-in-CREATE-FUNCTION-documentation.patchapplication/octet-stream; name=0001-Improve-example-in-CREATE-FUNCTION-documentation.patchDownload
From f8a432dbe5adacb8027aeef3ef536b3b2b040d70 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@wien.gv.at>
Date: Fri, 15 Jul 2016 13:45:21 +0200
Subject: [PATCH] Improve example in CREATE FUNCTION documentation

Explain in more detail why it is a good idea to have pg_temp
as the last search_path element in SECURITY DEFINER functions.
---
 doc/src/sgml/ref/create_function.sgml |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml
index 097e2bd..d8d5324 100644
--- a/doc/src/sgml/ref/create_function.sgml
+++ b/doc/src/sgml/ref/create_function.sgml
@@ -753,7 +753,10 @@ SELECT * FROM dup(42);
     malicious users from creating objects that mask objects used by the
     function.  Particularly important 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 obtained
+    is normally writable by anyone (even though functions and operators
+    in this schema are never used unless explicitly qualified, it should be
+    made sure that no temporary table will be accessed by accident).
+    A secure arrangement can be obtained
     by forcing the temporary schema to be searched last.  To do this,
     write <literal>pg_temp</><indexterm><primary>pg_temp</><secondary>securing functions</></> as the last entry in <varname>search_path</>.
     This function illustrates safe usage:
-- 
1.7.1

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Albe Laurenz (#3)
Re: Documentation fix for CREATE FUNCTION

Albe Laurenz <laurenz.albe@wien.gv.at> writes:

Tom Lane wrote:

I believe the reason for forcing pg_temp to the back of the path is to
prevent unqualified table names from being captured by pg_temp entries.
This risk exists despite the rule against searching pg_temp for functions
or operators. A maliciously named temp table could at least prevent
a security definer function from doing what it was supposed to, and
could probably hijack control entirely via triggers or rules.

Possibly the documentation should be more explicit about why this is
being done, but the example code is good as-is.

Maybe something like the attached would keep people like me from
misunderstanding this.

I rewrote this a bit and pushed it. Thanks for the suggestion!

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ce150e7e0fc1a127fee7933d71f4204a79ecce04

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers