Re: CREATE FUNCTION ... SEARCH { DEFAULT | SYSTEM | SESSION } - Mailing list pgsql-hackers

Hi,

On 2023-08-11 19:35:22 -0700, Jeff Davis wrote:
> Controlling search_path is critical for the correctness and security of
> functions. Right now, the author of a function without a SET clause has
> little ability to control the function's behavior, because even basic
> operators like "+" involve search_path. This is a big problem for, e.g.
> functions used in expression indexes which are called by any user with
> write privileges on the table.

> Motivation:
>
> I'd like to (eventually) get to safe-by-default behavior. In other
> words, the simplest function declaration should be safe for the most
> common use cases.

I'm not sure that anything based, directly or indirectly, on search_path
really is a realistic way to get there.


> To get there, we need some way to explicitly specify the less common
> cases. Right now there's no way for the function author to indicate
> that a function intends to use the session's search path. We also need
> an easier way to specify that the user wants a safe search_path ("SET
> search_path = pg_catalog, pg_temp" is arcane).

No disagreement with that. Even if I don't yet agree that your proposal is a
convincing path to "easy security for PLs" - just making the search path stuff
less arcane is good.


> And when we know more about the user's actual intent, then it will be
> easier to either form a transition plan to push users into the safer
> behavior, or at least warn strongly when the user is doing something
> dangerous (i.e. using a function that depends on the session's search
> path as part of an expression index).

I think that'd be pretty painful from a UX perspective. Having to write
e.g. operators as operator(schema, op) just sucks as an experience. And with
extensions plenty of operators will live outside of pg_catalog, so there is
plenty things that will need qualifying.  And because of things like type
coercion search, which prefers "bettering fitting" coercions over search path
order, you can't just put "less important" things later in search path.


I wonder if we ought to work more on "fossilizing" the result of search path
resolutions at the time functions are created, rather than requiring the user
to do so explicitly.  Most of the problem here comes down to the fact that if
a user creates a function like 'a + b' we'll not resolve the operator, the
potential type coercions etc, when the function is created - we do so when the
function is executed.

We can't just store the oids at the time, because that'd end up very fragile -
tables/functions/... might be dropped and recreated etc and thus change their
oid. But we could change the core PLs to rewrite all the queries (*) so that
they schema qualify absolutely everything, including operators and implicit
type casts.

That way objects referenced by functions can still be replaced, but search
path can't be used to "inject" objects in different schemas. Obviously it
could lead to errors on some schema changes - e.g. changing a column type
might mean that a relevant cast lives in a different place than with the old
type - but I think that'll be quite rare. Perhaps we could offer a ALTER
FUNCTION ... REFRESH REFERENCES; or such?

One obvious downside of such an approach is that it requires some work with
each PL. I'm not sure that's avoidable - and I suspect that most "security
sensitive" functions are written in just a few languages.


(*) Obviously the one thing that doesn't work for is use of EXECUTE in plpgsql
and similar constructs elsewhere. I'm not sure there's much that can be done
to make that safe, but it's worth thinking about more.

Greetings,

Andres Freund