Re: Misusing functions taking "internal" via coincidental signature match

On Aug 15, 2008, at 15:12, Tom Lane wrote:

> 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.

+1 # cleanliness++

> 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.)

That won't help pgFoundry modules, though. And since there's been so  
much talk about having improved third-party module support, it seems  
to me that we ought to try to discourage security holes in such  
modules, too.

> 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.

I did find this strange:

CREATE OR REPLACE FUNCTION citextrecv(internal)
RETURNS citext
AS 'textrecv'
LANGUAGE 'internal' STABLE STRICT;

CREATE OR REPLACE FUNCTION citextsend(citext)
RETURNS bytea
AS 'textsend'
LANGUAGE 'internal' STABLE STRICT;

It'd make a lot more sense to me if bytea was passed to citextrcv,  
rather than internal, even if, internally, it used internal (so to  
speak). Now you didn't mention CREATE FUNCTION as being one of the  
places where this comes up, but it seems to me that it would be  
valuable to hide the internal struct, and force SQL users to use only  
well-defined types.

Just my $0.02, FWIW.

Best,

David