Misusing functions taking "internal" via coincidental signature match - Mailing list pgsql-hackers

We have got a whole bunch of functions in the system that accept
arguments of type "internal", where the actual meaning of "internal"
varies wildly (it's generally some non-SQL-visible data structure).
While (I believe that) SQL users cannot call any such functions
directly, they could still cause all sorts of trouble if they could
persuade the system to call one with some other kind of "internal"
argument than the function is expecting.

Neither I nor the rest of the pgsql-security list could find any places
where there is a hole of this type today.  However there are several
commands accessible to non-superusers that skirt the edge of danger:

CREATE OPERATOR will accept as "restriction selectivity" function
any function having arguments (internal, oid, internal, int4),
and it'll accept as "join selectivity" function
any function having arguments (internal, oid, internal, int2).
It doesn't bother to check the return type, nor even whether
you have permission to call the function :-(

CREATE CONVERSION will accept as conversion function any function
having arguments (int4, int4, cstring, internal, int4).  It doesn't
check the return type, either, though at least it checks for EXECUTE
permission.  (Not that that helps much given that the default situation
is public execute permission.)

CREATE TYPE accepts receive functions having arguments either (internal)
or (internal,oid,int4).  Fortunately it insists that the result type be
the type being created, so it doesn't seem that any confusion is
possible.  However, it will also accept as "custom analyze" function
anything taking (internal) and returning bool.  We would have a problem
with boolrecv there, except that CREATE TYPE also demands ownership
of the function, and a non-superuser won't own boolrecv ... or,
probably, anything else accepting internal.

Now, so far as I can find there are not any functions in core or contrib
that accidentally match the signature requirements for selectivity
estimators or conversion functions.  It's a bit nervous-making though,
particularly since the signatures for GIST and GIN support functions
aren't totally nailed down (nor, indeed, verified by the system anyway).
But the big point here is not so much whether we have a problem today
as what might happen in future.  All of these system support functions
have call signatures that are subject to change.  (In fact, I'm about
to commit changes to allow an additional internal parameter for join
selectivity estimators.)  So it's not hard at all to think that as
we independently change different parts of the system, we might get
an accidental match of the expected signatures for functions intended to
do very different things.  And then we have a security issue if there
are non-privileged commands to tell the system to call one of these
functions in a particular way.  So it seems like we ought to take some
steps to compartmentalize things a bit better.

The cleanest solution I can think of is to invent some more pseudotypes
that act just like INTERNAL, and then to require non-privileged CREATE
commands to reference functions that take one of these types instead
of bare INTERNAL.  There is a backwards compatibility problem here,
of course, but it wouldn't affect anybody who hadn't written a custom
selectivity estimator, conversion, or analyze function.  Which is
probably only the PostGIS project.

Failing that, we could just try to keep a registry of possible
signatures for internal-accepting functions, and make sure we don't
accept any patches that cause conflicts.  This would avoid creating
backwards compatibility problems, but without any automatic enforcement
it seems pretty dangerous.  (Perhaps a new regression test in the spirit
of opr_sanity could help, though.)

Another thing that seems like a real good idea is to tighten up
the above-mentioned commands to check for a specific return type and
demand execute permissions.  Including the return type in the required
signature is in itself a big improvement in reducing the risk of
accidental matches.  This part we could do without creating any
compatibility issues.  I also thought about demanding ownership
rather than just execute permission on the functions.  That doesn't
seem like it'd fly for selectivity estimators, since it's customary
for user datatypes to re-use the built-in ones, but it might be a
real good idea for conversions.

Comments?
        regards, tom lane