Thread: Is this a bug, possible security hole, or wrong assumption?
1. I have a query that looks like this: SELECT 1 FROM v_offers WHERE (f_isvalidoffer(v_offers.offer, 747) = 1); 2. The f_isvalidoffer() functions does this: ... SELECT INTO target v_offers.store WHERE v_offers.offer = $1; IF NOT FOUND THEN RAISE EXCEPTION ''f_isvalidoffer: Offer % does not exist'', $1 END IF; ... 3. v_offers is a view, which is defined as this: SELECT * FROM offers WHERE deactive IS NULL; 4. offers is a table which is defined as this: CREATE TABLE offers ( offer int4 not null, store int4 not null, active timestamp not null default now(), deactive timestamp ); 5. The data in offers is (with X being timestamps): offer | store | active | deactive ------+-------+--------+--------- 1 1 X 2 1 X 3 1 X X 6. SELECT offer from offers correctly yields: 1 2 3 7. SELECT offer from v_offer correctly yields: 1 2 8. The query in #1 will raise an exception because: explain select 1 from v_offers where (f_isvalidoffer(v_offers.offer, 747) = 1); NOTICE: QUERY PLAN: Scan on offers (cost=0.00..25.00 rows=1 width=0) EXPLAIN What appears to me is that the rewriter is just tacking the IS NULL test onto the parsed query. As a result, a function is called with data from a view before the evaluation of IS NULL removes those rows from the selection process. Is that right? If so, is that a security problem? I have heard people recommend the use of views as a way of implementing row security. If I have a table salaries: CREATE TABLE salaries ( userid name not null, amount float8 not null ); CREATE VIEW v_salaries AS SELECT * FROM salaries WHERE userid = CURRENT_USER(); I then grant SELECT on v_salaries to user1, so that user1 can see his salary. But if user1 writes a function like: CREATE FUNCTION f_logsalary(name, float8) RETURNS int4 AS ' BEGIN INSERT INTO logs VALUES ($1, $2); RETURN 1; END; ' LANGUAGE 'plpgsql'; executes a select like: SELECT 1 FROM v_salaries WHERE (f_logsalary(v_salaries.userid, v_salaries.salary) = 1); user1 can then see everyone's salaries by querying logs. Any hints? Mike Mascari mascarm@mascari.com
Mike Mascari <mascarm@mascari.com> writes:
> What appears to me is that the rewriter is just tacking the IS NULL test
> onto the parsed query. As a result, a function is called with data from
> a view before the evaluation of IS NULL removes those rows from the
> selection process. Is that right? If so, is that a security problem?
You're essentially asking for a guarantee about the order of evaluation
of WHERE clauses. There is no such guarantee, and won't be because it
would be a crippling blow to performance. For example, consider
create table tab (k int primary key, d text);
create view v as select * from tab where d is not null;
select * from v where k = 42;
If the not-null clause must be evaluated before the outer where,
then this query will be unable to use an indexscan on k. See related
discussion a week or so ago (in pgsql-general if memory serves).
We could possibly tweak the optimizer so that the where-clauses pulled
up from the view are evaluated first in cases where there is no
plan-driven reason to do it the other way 'round, but I doubt this would
provide much security.
regards, tom lane
"Sander Steffann" <sander@steffann.nl> writes:
> But he is right in that his trick works. This proves that views can not be
> safely used for security, which is an important thing to realise...
A different way to look at it is that the privilege of creating
functions shouldn't be handed out willy-nilly. The trick of hiding
recording operations in a function can be used in other ways besides
this one.
regards, tom lane
Hi, > Mike Mascari <mascarm@mascari.com> writes: > > What appears to me is that the rewriter is just tacking the IS NULL test > > onto the parsed query. As a result, a function is called with data from > > a view before the evaluation of IS NULL removes those rows from the > > selection process. Is that right? If so, is that a security problem? > > You're essentially asking for a guarantee about the order of evaluation > of WHERE clauses. There is no such guarantee, and won't be because it > would be a crippling blow to performance. But he is right in that his trick works. This proves that views can not be safely used for security, which is an important thing to realise... Sander.
Tom Lane wrote: > > "Sander Steffann" <sander@steffann.nl> writes: > > But he is right in that his trick works. This proves that views can not be > > safely used for security, which is an important thing to realise... > > A different way to look at it is that the privilege of creating > functions shouldn't be handed out willy-nilly. The trick of hiding > recording operations in a function can be used in other ways besides > this one. Tom, If a user has permissions to write PL/SQL functions, and the statistics collector is running with STATS_COMMAND_STRING = true, could not that user "log" other users' queries using the same technique I described by querying pg_stat_activity? If so, isn't the labeling of PL/SQL (or PL/Tcl, PL/Perl, etc.) as 'TRUSTED' an overstatement? ;-) Mike Mascari mascarm@mascari.com
Tom Lane wrote: > > Mike Mascari <mascarm@mascari.com> writes: > > What appears to me is that the rewriter is just tacking the IS NULL test > > onto the parsed query. As a result, a function is called with data from > > a view before the evaluation of IS NULL removes those rows from the > > selection process. Is that right? If so, is that a security problem? > > You're essentially asking for a guarantee about the order of evaluation > of WHERE clauses. There is no such guarantee, and won't be because it > would be a crippling blow to performance. For example, consider > > create table tab (k int primary key, d text); > create view v as select * from tab where d is not null; > select * from v where k = 42; > > If the not-null clause must be evaluated before the outer where, > then this query will be unable to use an indexscan on k. See related > discussion a week or so ago (in pgsql-general if memory serves). > > We could possibly tweak the optimizer so that the where-clauses pulled > up from the view are evaluated first in cases where there is no > plan-driven reason to do it the other way 'round, but I doubt this would > provide much security. It seems to me that the condition which must be satisfied is this: If the attribute of a view is used in a user-defined function, then the conditional expressions associated with the WHERE condition of the view *must* be evaluated before the user-defined function is called (if ever). That would not limit the use of an index scan in the above example. Other RDBMS allow for both server-side functions and the use of views for security. In fact, SQL92 states (as an example): In each catalog in an SQL-environment, there is a schema, the Information Schema, with the name INFORMATION_SCHEMA, containing a number of view descriptors, one base table descriptor, and several domain descriptors. The data accessible through these views is a representation of all of the descriptors in all of the schemas in that catalog. The <query expression> of each view ensures that a given user can access only those rows of the view that represent descriptors on which he has privileges. Now obviously PostgreSQL does not yet have the INFORMATION_SCHEMA, but the statement implies that view implementations ought to be able to provide for row security... Mike Mascari mascarm@mascari.com
Mike Mascari <mascarm@mascari.com> writes:
> If a user has permissions to write PL/SQL functions, and the statistics
> collector is running with STATS_COMMAND_STRING = true, could not that
> user "log" other users' queries using the same technique I described by
> querying pg_stat_activity?
Not unless he's superuser.
regards, tom lane
I wrote: > > Tom Lane wrote: > > > > You're essentially asking for a guarantee about the order of evaluation > > of WHERE clauses. There is no such guarantee, and won't be because it > > would be a crippling blow to performance. > > It seems to me that the condition which must be satisfied is this: > > If the attribute of a view is used in a user-defined function, then the > conditional expressions associated with the WHERE condition of the view > *must* be evaluated before the user-defined function is called (if > ever). That would not limit the use of an index scan in the above > example. Other RDBMS allow for both server-side functions and the use of > views for security. I apologize. The pg_stat_activity view is a good example of using views atop functions to provide security. Its not exactly obvious, but it can be done. And with the SRFs coming, I suppose fixing views is a pretty low priority... Mike Mascari mascarm@mascari.com
Mike Mascari <mascarm@mascari.com> writes:
> I apologize. The pg_stat_activity view is a good example of using views
> atop functions to provide security. Its not exactly obvious, but it can
> be done. And with the SRFs coming, I suppose fixing views is a pretty
> low priority...
I've applied the attached patch, which changes the behavior in your
example case. However, in general I do not think it's possible or
desirable to guarantee anything about order of evaluation of WHERE
clauses.
regards, tom lane
*** src/backend/optimizer/plan/planner.c.orig Sat May 18 14:49:41 2002
--- src/backend/optimizer/plan/planner.c Thu Jun 13 11:01:09 2002
***************
*** 656,662 ****
if (childlen <= 1 || (childlen + myothers) <= geqo_rels / 2)
{
newlist = nconc(newlist, subf->fromlist);
! f->quals = make_and_qual(f->quals, subf->quals);
}
else
newlist = lappend(newlist, child);
--- 656,662 ----
if (childlen <= 1 || (childlen + myothers) <= geqo_rels / 2)
{
newlist = nconc(newlist, subf->fromlist);
! f->quals = make_and_qual(subf->quals, f->quals);
}
else
newlist = lappend(newlist, child);