Postgres Pro
Downloads
Home   >   mailing lists

Thread: Bug - DoS - Handler function lookups consider non-handler functions

Bug - DoS - Handler function lookups consider non-handler functions

From
"David G. Johnston"
Date:
Today:

create extension tsm_system_rows ;
create schema s1;
create function s1.system_rows(internal) returns void language c as 'tsm_system_rows.so', 'tsm_system_rows_handler';
set search_path to s1,public,pg_catalog;
select count(*) from pg_class tablesample system_rows(0);
ERROR:  function system_rows must return type tsm_handler
LINE 1: select count(*) from pg_class tablesample system_rows(0);

The above is a denial-of-service due to our decision to lookup handler functions by name regardless of return type and consider it an error if a function with the wrong return type shows up (in particular, even though one with the correct signature exists and otherwise would have been found).

The attached POC fixes this by allowing callers to also specify the OID of the handler type as part of their function lookup criteria.  Tablesample is fixed to use this new call though possibly others exist.  I'm not particularly fond of what I ended up with naming conventions but figure it's good enough for now.

Patch applied and re-running the above:

select count(*) from pg_class tablesample system_rows(0);
 count
-------
     0
(1 row)

I noticed this when reviewing the extensible copy formats patch which used tablesample as a reference.

David J.

Attachment

Re: Bug - DoS - Handler function lookups consider non-handler functions

From
Tom Lane
Date:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> create extension tsm_system_rows ;
> create schema s1;
> create function s1.system_rows(internal) returns void language c as
> 'tsm_system_rows.so', 'tsm_system_rows_handler';
> set search_path to s1,public,pg_catalog;
> select count(*) from pg_class tablesample system_rows(0);
> ERROR:  function system_rows must return type tsm_handler
> LINE 1: select count(*) from pg_class tablesample system_rows(0);

> The above is a denial-of-service due to our decision to lookup handler
> functions by name regardless of return type and consider it an error if a
> function with the wrong return type shows up (in particular, even though
> one with the correct signature exists and otherwise would have been found).

I do not think this is a bug: it's superuser error.  Yeah, it'd be
great if we could prevent people from creating incorrect support
functions, but that'd require solving the halting problem.

> The attached POC fixes this by allowing callers to also specify the OID of
> the handler type as part of their function lookup criteria.

I'm not on board with that at all.  The law of unintended consequences
comes into play the moment you start messing with function lookup
rules.  And I'm not at all convinced that "ignore it" is an
improvement over "throw an error".

And please, let's stop with the scare tactics of calling this sort
of thing a denial-of-service.

            regards, tom lane