Horton

Thread

  1. invoker function security issues

    Virender Singla <virender.cse@gmail.com> — 2022-06-08T11:57:32Z

    I believe functions in Postgres follow a late binding approach and hence
nested function dependencies are resolved using search_path at run time.
This way a user can override nested functions in its schema and change the
behaviour of wrapper functions. However, a more serious issue is when
functional Indexes (with nested function calls) are created on a table and
then the data inserted in Indexes could be entirely dependent on which user
is inserting the data (by overriding nested function).

I performed a couple of test cases where data inserted is dependent on the
user overriding nested functions. I understand this is not the best
practice to scatter functions/indexes/tables in different different schemas
and use such kind schema setup but I still expect Postgres to save us from
such data inconsistencies issues by using early binding for functional
Indexes. In fact Postgres does that linking for a single function Index
(where no nested function are there) and qualifies the function used in the
Index with its schema name and also it works in cases where all
functions, table, Indexes are present in the same schema.

However still there are cases where functional Indexes are created on
extension functions (For Ex - cube extension) which are present in
different schemas and then those cube functions are defined as invoker
security type with nested functions calls without any schema qualification.

Issue that would arise with late binding for functional Indexes is that
when we are migrating such tables/indexes/data from one database to another
(using pg_dump/pg_restore or any other method) data can be changed
depending on which user we are using for import.
(These tests i performed using invoker functions, i think definer functions
produce correct behavior). One way would be to define search_path for such
nested functions.


   1. =========Case1======
   2.
   3. ##Table and functions are in different schemas.
   4.
   5. Session1::
   6. User:Postgres
   7.
   8. create user idxusr1 with password '*****';
   9. grant idxusr1 to postgres;
   10. create schema idxusr1 AUTHORIZATION idxusr1;
   11.
   12. create user idxusr2 with password '*****';
   13. grant idxusr2 to postgres;
   14. create schema idxusr2 AUTHORIZATION idxusr2;
   15.
   16. Session2::
   17. User:idxusr1
   18.
   19. set search_path to idxusr1,public;
   20.
   21. CREATE FUNCTION sumcall(int, int) RETURNS int LANGUAGE SQL
IMMUTABLE STRICT PARALLEL SAFE AS 'SELECT ($1+$2)';
   22.
   23. CREATE FUNCTION wrapsum(int, int) RETURNS int LANGUAGE SQL
IMMUTABLE STRICT PARALLEL SAFE AS 'SELECT sumcall($1,$2)';
   24.
   25. ##create table in another schema
   26.
   27. create table public.test(n1 int);
   28. create unique index idxtst on public.test(idxusr1.wrapsum(n1,1));
   29.
   30. grant insert on table public.test to idxusr2;
   31.
   32. postgres=> insert into test values(1);
   33. INSERT 0 1
   34. postgres=> insert into test values(1);
   35. ERROR:  duplicate key value violates unique constraint "idxtst"
   36. DETAIL:  Key (wrapsum(n1, 1))=(2) already exists.
   37.
   38. Session3::
   39. User:idxusr2
   40.
   41. set search_path to idxusr2,public;
   42.
   43. CREATE FUNCTION sumcall(int, int) RETURNS int LANGUAGE SQL
IMMUTABLE STRICT PARALLEL SAFE AS 'SELECT ($1 - $2)';
   44.
   45. postgres=> insert into test values(1);
   46. INSERT 0 1
   47. postgres=> insert into test values(1);
   48. ERROR:  duplicate key value violates unique constraint "idxtst"
   49. DETAIL:  Key (idxusr1.wrapsum(n1, 1))=(0) already exists.
   50.
   51. ======Case2==========
   52.
   53.  ##Functions are in different schemas.
   54.
   55. Session1::
   56. User:Postgres
   57.
   58. create user idxusr1 with password '*****';
   59. grant idxusr1 to postgres;
   60. create schema idxusr1 AUTHORIZATION idxusr1;
   61.
   62. create user idxusr2 with password '*****';
   63. grant idxusr2 to postgres;
   64. create schema idxusr2 AUTHORIZATION idxusr2;
   65.
   66. Session2::
   67. User:idxusr1
   68.
   69. set search_path to idxusr1,public;
   70.
   71. ##create internal function in own schema and wrapper function
in another schema.
   72.
   73. CREATE FUNCTION sumcall(int, int) RETURNS int LANGUAGE SQL
IMMUTABLE STRICT PARALLEL SAFE AS 'SELECT ($1+$2)';
   74.
   75. CREATE FUNCTION public.wrapsum(int, int) RETURNS int LANGUAGE
SQL IMMUTABLE STRICT PARALLEL SAFE AS 'SELECT sumcall($1,$2)';
   76.
   77. create table test(n1 int);
   78. create unique index idxtst on test(public.wrapsum(n1,1));
   79.
   80. grant usage on schema idxusr1 to idxusr2;
   81. grant insert on table test to idxusr2;
   82. postgres=> insert into test values(1);
   83. INSERT 0 1
   84. postgres=> insert into test values(1);
   85. ERROR:  duplicate key value violates unique constraint "idxtst"
   86. DETAIL:  Key (wrapsum(n1, 1))=(2) already exists.
   87.
   88. Session3::
   89. User:idxusr2
   90.
   91. set search_path to idxusr2,public;
   92.
   93. CREATE FUNCTION sumcall(int, int) RETURNS int LANGUAGE SQL
IMMUTABLE STRICT PARALLEL SAFE AS 'SELECT ($1 - $2)';
   94.
   95. postgres=> insert into idxusr1.test values(1);
   96. INSERT 0 1
   97. postgres=> insert into idxusr1.test values(1);
   98. ERROR:  duplicate key value violates unique constraint "idxtst"
   99. DETAIL:  Key (wrapsum(n1, 1))=(0) already exists.
   100. postgres=>

  2. Re: invoker function security issues

    David G. Johnston <david.g.johnston@gmail.com> — 2022-06-08T15:54:34Z

    On Wed, Jun 8, 2022 at 7:29 AM Virender Singla <virender.cse@gmail.com>
wrote:

>  but I still expect Postgres to save us from such data inconsistencies
> issues by using early binding for functional Indexes.
>

Well, if the functions you are writing are "black boxes" to PostgreSQL this
expectation seems unreasonable.  As of v14 at least you have the option to
write a SQL standard function definition which is whose parsed expression
and dependencies are saved instead of a black box of text.


> However still there are cases where functional Indexes are created on
> extension functions (For Ex - cube extension) which are present in
> different schemas and then those cube functions are defined as invoker
> security type with nested functions calls without any schema qualification.
>

Right, because if you try doing that in a security definer context the lack
of a schema qualification will provoke a function not found error due to
the sanitized search_path  (IIRC, if it doesn't then the two cases should
behave identically...)

One way would be to define search_path for such nested functions.
>

Which the user is well advised to do, or, better, just schema-qualify all
object references.  This can get a bit annoying for operators, in which
case an explicit, localized, search_path becomes more appealing.

The tools are available for one to protect themself.  I suspect the
historical baggage we carry prevents the server from being more aggressive
in enforcing the use of these tools since not all cases where they are not
used are problematic and there is lots of legacy code working just fine.
The security lockdown for this dynamic has already happened by basically
admitting that the idea of a "public" schema at the front of the default
search_path was a poor decision.  And while I see that there is possibly
room for improvement here if desired, it is, for me, acceptable for the
project to put the responsibility of not executing problematic code in the
hands of the DBA.

I'm curious how "EXECUTE" command and dynamic SQL fit into your POV here
(specifically in function bodies).  Right now, with "black box inside of
black box" mechanics it isn't really an issue but if you want to not keep
function bodies as black boxes now dynamic SQL becomes the top-most late
binding point.

David J.