Facility for detecting insecure object naming

From: Noah Misch <noah(at)leadboat(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Facility for detecting insecure object naming
Date: 2018-08-05 08:04:41
Message-ID: 20180805080441.GH1688868@rfd.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Commit e09144e documented the rules for writing safe qualified names, but
those rules are tedious to apply in practice. Spotting the defects in this
function definition (from an unpublished draft intended for
https://postgr.es/m/20180710014308.GA805781@rfd.leadboat.com) is, I think, too
hard:

CREATE FUNCTION latitude(earth)
RETURNS float8
LANGUAGE SQL
IMMUTABLE STRICT
PARALLEL SAFE
AS 'SELECT CASE
WHEN @cube_schema(at)(dot)cube_ll_coord($1, 3) OPERATOR(pg_catalog./)
@extschema(at)(dot)earth() OPERATOR(pg_catalog.<) -1 THEN -90::pg_catalog.float8
WHEN @cube_schema(at)(dot)cube_ll_coord($1, 3) OPERATOR(pg_catalog./)
@extschema(at)(dot)earth() OPERATOR(pg_catalog.>) 1 THEN 90::pg_catalog.float8
ELSE pg_catalog.degrees(pg_catalog.asin(@cube_schema(at)(dot)cube_ll_coord($1, 3)
OPERATOR(pg_catalog./) @extschema(at)(dot)earth()))
END';

If hackers and non-core extension authors are to write such code, let's make
it easier to check the work. Different classes of code need different checks.
In each class, qualified function and operator names referring to untrusted
schemas need an exact match of function parameters, including any VARIADIC.
Class-specific rules:

a. SQL intended to run under secure search_path. No class-specific rules.
src/bin code is an example of this class, and this is the only secure class
for end-user applications.

b. SQL intended for a particular search_path, possibly untrusted. Unqualified
names need an exact match. Use a qualified name for any object whose
schema appears in search_path later than some untrusted schema. Examples
of this class include extension scripts, pre-CVE-2018-1058 pg_dump, some
functions with "SET search_path", and many casual end-user applications.

c. SQL intended to work the same under every search_path setting. Do not use
any unqualified name. Most pg_catalog and contrib functions, but not those
having a "SET search_path" annotation, are examples of this class.

I believe PostgreSQL can apply each class's rules given a list of trusted
schemas and a flag to enable the checks. Class (b) naturally degenerates to
class (a) if every schema of search_path appears in the whitelist. To check
class-(c) code, issue "SET search_path = not_in_whitelist, pg_temp,
pg_catalog, ..." before the test queries. That's something of a hack, but I
didn't think of a non-hack that I liked better.

Should this feature warn about "SELECT 'earth()'::pg_catalog.regprocedure"
under the conditions that would make it warn about "SELECT earth()"? Should
it offer the option to warn or not warn? Some uses of reg*,
e.g. appendQualifiedRelation(), would consider those to be false positives.

Then there's the question of exact UI naming. Some possibilities:

SET lint_trusted_schemas = pg_catalog, admin
SET lint = reg*, exact_match, qualified_name
SET lint = all
SET lint = ''

SET lint_trusted_schemas = pg_catalog, admin
SET lint_name_security = on

SET name_security_warning_trusted_schemas = pg_catalog, admin
SET name_security_warning = on

SET name_security_warnings_trusted_schemas = pg_catalog, admin
SET warnings = reg*, exact_match, qualified_name

Preferences, other ideas?

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2018-08-05 09:48:48 Re: Pluggable Storage - Andres's take
Previous Message Tom Lane 2018-08-05 03:26:06 Re: pg_upgrade from 9.4 to 10.4