Re: Loose ends after CVE-2020-14350 (extension installation hazards)

On Fri, Aug 14, 2020 at 02:50:32PM -0400, Tom Lane wrote:
> However, in itself this can only fix references that are resolved during
> execution of the extension script. I don't see a good way to use the
> idea to make earthdistance's SQL functions fully secure. It won't do
> to write, say,
>
> CREATE FUNCTION ll_to_earth(float8, float8)
> ...
> AS 'SELECT @extschema(cube)@.cube(...)';
>
> because this will not survive somebody doing "ALTER EXTENSION cube SET
> SCHEMA schema3". I don't have a proposal for what to do about that.

Another challenge is verifying that the body qualified everything. Via a
simple matter of programming, CREATE EXTENSION could verify that CREATE-time
code observes schema qualification rules. That would not extend to function
bodies.

> Admittedly, we already disclaim security if you run queries with a
> search_path that contains any untrusted schemas ... but it would be
> nice if extensions could be written that (in themselves) are safe
> regardless.

Yes. Even when safety is not a concern, it's a quality problem for the
functions to error out when search_path lacks some schema. As you know, we
get recurring reports about that, e.g.
https://www.postgresql.org/message-id/flat/16534-69f25077c45f34a5%40postgresql.org

> Peter E's proposal for parsing SQL function bodies at
> creation time could perhaps fix this for SQL functions, but we still
> have the issue for other PLs.

Yes. The SQL-specific feature could do enough to let a future version of
earthdistance be trusted.

> 2. [...] lookup_agg_function() allows
> inexact argument type matches for an aggregate's support functions,
> so it could be possible to capture a reference if the intended support
> function doesn't exactly match the aggregate's declared input and
> transition data types.

Should CREATE AGGREGATE support "FINALFUNC = foo(sometype)" input to constrain
the lookup? (It does accept the syntax, but "sometype" is unused and need not
even denote an extant type.)

> 3. As Christoph Berg noted, the fixes in some extension update scripts
> mean that plpgsql has to be installed while those scripts run. How
> much do we care, and if we do, what should we do about it?

I propose not caring at all. Since we have dump/reload of "REVOKE USAGE ON
LANGUAGE plpgsql FROM PUBLIC", extensions requiring plpgsql are fine. (It
could be a problem in a "superuser = false" extension, but core isn't doing
those.) Even saddling plpgsql with a pin dependency would be fine.

> 4. I noticed while testing that hstore--1.0--1.1.sql is completely
> useless nowadays, so it might as well get dropped. It fails with a
> syntax error in every still-supported server version, since "=>" is
> no longer a legal operator name. There's no way to load hstore 1.0
> into a modern server because of that, either.

The chance of this getting reported from the field has been dropping for
several years. It's negligible now.

Thanks,
nm