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 03:39:33PM -0400, Robert Haas wrote:
> On Fri, Jul 20, 2012 at 1:52 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > 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.

How about a compromise of blocking GUC and STRICT changes while allowing
everything else? We add PGC_USERSET GUCs in most releases. As long as
non-superuser owners of trusted-language functions can change attached GUC
settings, review for each new GUC really ought to consider that scenario.
That will be easy to forget. I'm already wary about allowing changes to GUCs
like sql_inheritance and search_path. By contrast, the list of ALTER FUNCTION
alterations has grown slowly; the last addition before PostgreSQL 9.2 came in
PostgreSQL 8.3. Anyone implementing a new alteration will be modifying
AlterFunction() and have ample opportunity to notice from surrounding code the
need to identify a suitable permissions check. Also, like you say, the other
existing alterations are clearly safe.

Thanks,
nm