Re: Facility for detecting insecure object naming - Mailing list pgsql-hackers

> On Aug 15, 2018, at 9:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Mark Dilger <hornschnorter@gmail.com> writes:
>> Go ahead and define some new lexical scope mechanism.  I'm probably
>> going to move into the 21st century with you and use it.  (I mostly
>> use "my", not "local", when I write perl code.)  But let's treat that as
>> a new feature independent of fixing the security problems with dynamically
>> scoped variables and privilege escalation attacks.
> 
> To my mind, the really fundamental problem here is that our default/
> traditional search_path includes publicly writable schemas.  I don't
> especially care whether you establish your search_path setting
> dynamically, or lexically, or through magic hocus pocus: if it includes a
> schema that a hostile user can create objects in, you'll get pwned sooner
> not later.  So it doesn't seem to me that Robert's proposal is really
> doing much to move the goalposts.  He's presuming that the main problem
> is that user A and user B have different ideas about which schemas are
> trusted ... but I think that that is, at most, a minority use-case.
> I suspect that a system-wide definition of which schemas are trusted
> would serve many installations just fine.
> 
> The point of the function trust proposal was to provide some degree of
> security even in an environment where there are untrusted schemas in your
> search path.  However, the other approach we could take is to deprecate
> doing that: tell people to stop putting executable objects in "public",
> or maybe even deprecate "public" altogether, so that PG search paths are
> treated more like Unix PATH settings.  You don't put /tmp in your PATH.

This is the part I don't understand.  Of course, I don't put /tmp or ./
in my path, but a well configured system doesn't get hacked simply because
a non-root user puts those in her path:

+ CREATE TABLE security_test_table (str TEXT);
+ CREATE FUNCTION add_prefix(prefix TEXT, str TEXT) RETURNS TEXT AS $$
+   SELECT $1 || ': ' || $2;
+ $$ LANGUAGE sql IMMUTABLE STRICT;
+ CREATE FUNCTION security_test_table_trigger_proc () RETURNS TRIGGER AS $$
+ BEGIN
+   NEW.str := add_prefix('admin', NEW.str);
+   RETURN NEW;
+ END;
+ $$ LANGUAGE plpgsql;
+ CREATE TRIGGER security_test_table_trigger
+   BEFORE INSERT OR UPDATE
+   ON security_test_table
+   FOR EACH ROW
+   EXECUTE PROCEDURE security_test_table_trigger_proc();
+ CREATE ROLE alice
+   NOSUPERUSER 
+   NOCREATEDB
+   NOCREATEROLE
+   NOINHERIT
+   NOREPLICATION
+   NOBYPASSRLS
+   CONNECTION LIMIT 1;
+ CREATE SCHEMA alice;
+ GRANT ALL ON SCHEMA alice TO alice;
+ GRANT INSERT, SELECT ON security_test_table TO alice;
+ SET ROLE alice;
+ CREATE FUNCTION alice.add_prefix(prefix TEXT, str TEXT) RETURNS TEXT AS $$
+   SELECT 'alice: ' || $2;
+ $$ LANGUAGE sql IMMUTABLE STRICT;
+ SET search_path TO alice, public;
+ INSERT INTO security_test_table (str) VALUES ('monkey');
+ RESET ROLE;
+ SELECT * FROM security_test_table;
+       str      
+ ---------------
+  alice: monkey
+ (1 row)
+ 

Unless you are going to prevent Alice from having her own schema in
which she can create her own functions, this needs fixing.  I don't
see point about not putting 'alice' in the path, since Alice can do
so.

What am I missing here?

mark