Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

On Fri, Jul 20, 2012 at 1:52 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Jun 12, 2012 at 5:12 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Yeah, the just-code-defensively option is worth considering too.
>
>> After rereading this thread, I think I agree with Kevin as well. ...
>> Having said that, I do believe that answer is to some extent a
>> cop-out.
>
> I agree with that --- doing nothing at all doesn't seem like the best
> option here.
>
>> ... on the flip side, the C code could - not to
>> put too fine a point on it - be relying on just about anything.
>
> And with that too. The STRICT option is a fairly obvious security
> hazard, but who's to say there are not others? I think it'd be easier
> to make a case for forbidding a non-superuser from altering *any*
> property of a C function. I'd rather start from the point of allowing
> only what is clearly safe than disallowing only what is clearly unsafe.

That seems like a fairly drastic overreaction. Are you going to ban
renaming it or changing the owner, which are in completely different
code paths? Yuck. Even if you only ban it for the main ALTER
FUNCTION code path, it seems pretty draconian, because it looks to me
like nearly everything else that's there is perfectly safe. I mean,
assuming the guy who wrote the C code didn't do anything completely
insane or malicious, setting GUCs or whatever should be perfectly OK.
Honestly, if you want to change something in the code, I'm not too
convinced that there's anything better than what Noah proposed
originally.

> Taking a step or two back, I think that the real use-case we should
> be considering here is allowing non-superusers to own (or at least
> install) extensions that contain C functions. We would probably want
> the non-superuser to be able to drop the extension again, maybe
> ALTER EXTENSION SET SCHEMA, maybe ALTER EXTENSION OWNER ... and likely
> not too darn much else. Fooling with any of the contained objects
> doesn't seem like something we want to permit, since it's likely that
> something like a datatype is going to have dependencies on not just
> specific objects' properties but their interrelationships.

Moreover, it breaks dump-and-restore.

> One possible approach to that is to say that the nominal owner of such
> an extension only owns the extension itself, and ownership of the
> contained objects is held by, say, the bootstrap superuser. There are
> other ways too of course, but this way would bypass the problem of
> figuring out how to restrict what an object's nominal owner can do
> to it.

I don't particularly care for that solution; it seems like a kludge.
I've kind of wondered whether we ought to have checks in all the ALTER
routines that spit up if you try to ALTER an extension member from any
place other than an extension upgrade script... but that still
wouldn't prevent the extension owner from dropping the members out of
the extension and then modifying them afterwards. I'm not sure we
want to prevent that in general, but maybe there could be some
locked-down mode that has that effect.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company