Обсуждение: Permissions checks for range-type support functions
It strikes me that we are rather lacking in $SUBJECT. There are a couple of distinct issues: 1. Since no permissions checks are applied at runtime, a nefarious person could bypass ACL_EXECUTE checks for any function that happens to match the signature for a subtype_diff function. Just create a range type that specifies that function as subtype_diff, and think up an index operation that will call it with the arguments you want. In principle one could bypass permissions checks on canonical functions as well, though in practice I think that's uninteresting because of the restrictions on the function signature. In the analogous situation for base types, we don't worry about this because only superusers can define base types; we just presume (and document) that the superuser is effectively granting public access permissions on all functions referenced in a base type definition. This will clearly not fly for range types. The nearest analogy I can see for non-superusers is use of functions within aggregate definitions. There, we check that the aggregate definer has permission to call the referenced function at aggregate definition time, and then recheck it at the start of any query that uses the aggregate. (So, the aggregate definer can give away permission to call the target function, but he could do that anyway by creating a SECURITY DEFINER wrapper function.) For the range-type support functions, it would be simple enough to check call permission at range-type definition time. But I really don't want to put in a run-time check, because there doesn't seem to be a very practical way to do it less often than every call, and besides that it's not very clear who to blame an index operation on. Is it good enough to test this only at range-type creation time? The implication of course is that you might not be able to revoke execute permission from a bad guy, short of dropping your function. This might be all right given the relatively narrow cross-section of functions that could be called this way. 2. The ANALYZE option is flat out dangerous, because it allows any function with the signature "f(internal) returns bool" to be called as though it's a typanalyze function. There are a couple of such functions in the catalogs already, and either of them will probably crash the backend if invoked as typanalyze on a range column. Again, this isn't a hazard for the existing use with base types, because only superusers (presumed to be responsible adults) can define base types. But for range types it's an easy DOS attack for less responsible persons. I'm inclined to think that the right solution for this one is to drop the ANALYZE option altogether, and just have DefineRange automatically install a system-wide typanalyze function for ranges. Under what circumstances is range-type-specific knowledge going to be needed for typanalyze, anyway? Especially since the functions that would use the results will be system-wide selectivity functions associated with the ANYRANGE operators. regards, tom lane
I wrote: > For the range-type support functions, it would be simple enough to check > call permission at range-type definition time. But I really don't want > to put in a run-time check, because there doesn't seem to be a very > practical way to do it less often than every call, and besides that it's > not very clear who to blame an index operation on. Is it good enough to > test this only at range-type creation time? The implication of course is > that you might not be able to revoke execute permission from a bad guy, > short of dropping your function. This might be all right given the > relatively narrow cross-section of functions that could be called this > way. On further reflection I think we can get away with this, because of the additional check that's already in there that insists the support functions be IMMUTABLE. That means they can't have any side-effects, which makes the potential security consequences of unauthorized calls very minimal. It's conceivable that the return value of a misused subtype_diff function could be interesting (think some sort of decryption function); but the system doesn't offer any way for a user to see that return value. It will only factor into GiST index page split decisions, and in a pretty indirect way at that. So I don't see any interesting security risk for a misused subtype_diff function; and there's not likely to be any meaningful hole for abusing canonical functions either, due to the restrictions on argument/result datatype. I still think we ought to add a creation-time permission check, just for pro-forma purposes. > 2. The ANALYZE option is flat out dangerous, But this is still true. regards, tom lane
On Tue, Nov 22, 2011 at 6:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > 2. The ANALYZE option is flat out dangerous, because it allows any > function with the signature "f(internal) returns bool" to be called as > though it's a typanalyze function. There are a couple of such functions > in the catalogs already, and either of them will probably crash the > backend if invoked as typanalyze on a range column. It's always seemed mildly insane to me that we don't distinguish between different flavors of "internal". That seems like an accident waiting to happen. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Excerpts from Robert Haas's message of mié nov 23 11:01:50 -0300 2011: > On Tue, Nov 22, 2011 at 6:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > 2. The ANALYZE option is flat out dangerous, because it allows any > > function with the signature "f(internal) returns bool" to be called as > > though it's a typanalyze function. There are a couple of such functions > > in the catalogs already, and either of them will probably crash the > > backend if invoked as typanalyze on a range column. > > It's always seemed mildly insane to me that we don't distinguish > between different flavors of "internal". That seems like an accident > waiting to happen. Well, before we had INTERNAL, there was only OPAQUE which conflated even more things that we now distinguish (at least trigger and cstring, not sure if there were others). -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes: > Excerpts from Robert Haas's message of mié nov 23 11:01:50 -0300 2011: >> It's always seemed mildly insane to me that we don't distinguish >> between different flavors of "internal". That seems like an accident >> waiting to happen. > Well, before we had INTERNAL, there was only OPAQUE which conflated even > more things that we now distinguish (at least trigger and cstring, not > sure if there were others). Yeah, we previously subdivided OPAQUE to get rid of exactly this type of problem. Not sure if it's worth going further. The case that would be problematic would be if we had two different calling contexts that invoked internal-using functions, both accessible for untrusted users to set up which function gets called, and sharing identical function signatures. ATM I believe the only calling contexts that untrusted users can control are operator selectivity functions (restriction and join flavors) and encoding conversion functions; and those three cases have signatures that are distinct from each other and from all privileged cases. But this certainly is something that could accidentally get broken in future. I don't think we want to split INTERNAL into the number of distinct pseudotypes that would be required to cover everything, but it might be worth inventing a couple more for the selectivity cases, if we ever change those APIs again. regards, tom lane