Horton

Thread

Commits

  1. Disallow execution of SPI functions during plperl function compilation.

  1. Bug plperl.c

    Mark Murawski <markm-lists@intellasoft.net> — 2022-02-22T17:49:26Z

    Affects Versions: 12 (and probably all others)

Steps to Reproduce:
- Run a query during plperl validation -- ie: inside a warn handler

CREATE OR REPLACE FUNCTION foo() RETURNS text
  LANGUAGE plperlu
AS $function$

use warnings;
use strict;

$SIG{'__WARN__'} = sub {
   my $pid = main::spi_exec_query('SELECT 
pg_backend_pid()')->{rows}[0]{pg_backend_pid};
};

my $foo;
my $foo;  # <---- causes a warning

$function$



2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG:  server process 
(PID 19702) was terminated by signal 11: Segmentation fault


#0   0x00007f32cfc33e65  in  plperl_spi_exec  (query=query@entry=0x55f7157b9290  "pg_backend_pid()",  limit=limit@entry=0)  at  plperl.c:3184
#1   0x00007f32cfc36bbe  in  XS__spi_exec_query  (my_perl=0x55f7157b7490,  cv=<optimized  out>)  at  SPI.xs:38
#2   0x00007f32cf9d85b1  in  Perl_pp_entersub  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#3   0x00007f32cf9ce896  in  Perl_runops_standard  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#4   0x00007f32cf943c85  in  Perl_call_sv  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#5   0x00007f32cf9aff22  in  ??  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#6   0x00007f32cf9b0ce1  in  Perl_vwarn  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#7   0x00007f32cf9b115f  in  Perl_warner  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#8   0x00007f32cf97df83  in  Perl_pad_add_name_pvn  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#9   0x00007f32cf92582d  in  Perl_allocmy  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#10  0x00007f32cf96600b  in  Perl_yylex  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#11  0x00007f32cf978e80  in  Perl_yyparse  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#12  0x00007f32cfa1240f  in  ??  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#13  0x00007f32cfa1dbed  in  Perl_pp_entereval  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#14  0x00007f32cf9ce896  in  Perl_runops_standard  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#15  0x00007f32cf943ea1  in  Perl_call_sv  ()  from  /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#16  0x00007f32cfc2dbf3  in  plperl_create_sub  (
     s=s@entry=0x55f7158d7858  "\n\nuse strict;\nuse warnings;\n\nuse IntellaConfig { ProgramName => 
'PgLiveQueueCreateViews' };\nuse SimpleSPI;\nuse PostgresInfo;\nuse 
UpgradeUtils;\n\nuse Data::Dumper;\nu se JSON;\n\nmy ($cmd) = $_[0] || 
'repla"...,  fn_oid=fn_oid@entry=39623,  prodesc=<optimized  out>,  prodesc=<optimized  out>)  at  plperl.c:2135
#17  0x00007f32cfc2e5d5  in  compile_plperl_function  (fn_oid=fn_oid@entry=39623,  is_trigger=is_trigger@entry=false,  is_event_trigger=is_event_trigger@entry=false)  at  plperl.c:2989
#18  0x00007f32cfc33c98  in  plperl_validator  (fcinfo=<optimized  out>)  at  plperl.c:2053
#19  0x000055f713965590  in  FunctionCall1Coll  (flinfo=0x7ffe96003c50,  collation=<optimized  out>,  arg1=<optimized  out>)  at  fmgr.c:1140
#20  0x000055f713965c4e  in  OidFunctionCall1Coll  (functionId=functionId@entry=24589,  collation=collation@entry=0,  arg1=arg1@entry=39623)  at  fmgr.c:1418
#21  0x000055f7136228a5  in  ProcedureCreate  (procedureName=<optimized  out>,  procNamespace=procNamespace@entry=39605,  replace=<optimized  out>,  returnsSet=returnsSet@entry=false,
     returnType=returnType@entry=25,  proowner=16385,  languageObjectId=24590,  languageValidator=24589,
     prosrc=0x55f7157c1408  "\n\nuse strict;\nuse warnings;\n\nuse IntellaConfig { ProgramName => 
'PgLiveQueueCreateViews' };\nuse SimpleSPI;\nuse PostgresInfo;\nuse 
UpgradeUtils;\n\nuse Data::Dumper;\nuse JSON;\n\nmy ($cmd) = $_[0] || 
'repla"...,  probin=0x0,  prokind=102  'f',  security_definer=false,  isLeakProof=false,  isStrict=false,  volatility=118  'v',  parallel=117  'u',  parameterTypes=0x55f7156f12d0,
     allParameterTypes=0,  parameterModes=0,  parameterNames=94519704883840,  parameterDefaults=0x0,  trftypes=0,  proconfig=0,  prosupport=0,  procost=procost@entry=100,  prorows=prorows@entry=0)
     at  pg_proc.c:726
#22  0x000055f71369a3d0  in  CreateFunction  (pstate=pstate@entry=0x55f7156f0bf0,  stmt=stmt@entry=0x55f7156d2e48)  at  functioncmds.c:1152
#23  0x000055f71384786e  in  ProcessUtilitySlow  (pstate=pstate@entry=0x55f7156f0bf0,  pstmt=pstmt@entry=0x55f7156d3188,
     queryString=queryString@entry=0x7f32cfc3e048  "CREATE OR REPLACE FUNCTION live_queue.create_views(cmd text)\n RETURNS 
text\n LANGUAGE plperlu\nAS $function$\n\nuse strict;\nuse 
warnings;\n\nuse Intella Config { ProgramName => 
'PgLiveQueueCreateViews' };\nus"...,  context=context@entry=PROCESS_UTILITY_TOPLEVEL,  params=params@entry=0x0,  queryEnv=queryEnv@entry=0x0,  completionTag=0x7ffe96004620  "",
     dest=0x55f7156d3268)  at  utility.c:1521
#24  0x000055f7138465d2  in  standard_ProcessUtility  (pstmt=0x55f7156d3188,
     queryString=0x7f32cfc3e048  "CREATE OR REPLACE FUNCTION live_queue.create_views(cmd text)\n RETURNS 
text\n LANGUAGE plperlu\nAS $function$\n\nuse strict;\nuse 
warnings;\n\nuse IntellaConfig { ProgramNa --Type <RET> for more, q to 
quit, c to continue without paging--
me => 'PgLiveQueueCreateViews' };\nus"...,  context=PROCESS_UTILITY_TOPLEVEL,  params=0x0,  queryEnv=0x0,  dest=0x55f7156d3268,  completionTag=0x7ffe96004620  "")  at  utility.c:927
#25  0x000055f7138449aa  in  PortalRunUtility  (portal=portal@entry=0x55f7157666f0,  pstmt=pstmt@entry=0x55f7156d3188,  isTopLevel=isTopLevel@entry=true,  setHoldSnapshot=setHoldSnapshot@entry=false,  dest=0x55f7156d3268,
     completionTag=0x7ffe96004620  "")  at  pquery.c:1171
#26  0x000055f713844af5  in  PortalRunMulti  (portal=portal@entry=0x55f7157666f0,  isTopLevel=isTopLevel@entry=true,  setHoldSnapshot=setHoldSnapshot@entry=false,  dest=dest@entry=0x55f7156d3268,
     altdest=altdest@entry=0x55f7156d3268,  completionTag=completionTag@entry=0x7ffe96004620  "")  at  pquery.c:1327
#27  0x000055f713844fa1  in  PortalRun  (portal=portal@entry=0x55f7157666f0,  count=count@entry=9223372036854775807,  isTopLevel=isTopLevel@entry=true,  run_once=run_once@entry=true,  dest=dest@entry=0x55f7156d3268,
     altdest=altdest@entry=0x55f7156d3268,  completionTag=0x7ffe96004620  "")  at  pquery.c:805
#28  0x000055f71384107d  in  exec_simple_query  (
     query_string=0x7f32cfc3e048  "CREATE OR REPLACE FUNCTION live_queue.create_views(cmd text)\n RETURNS 
text\n LANGUAGE plperlu\nAS $function$\n\nuse strict;\nuse 
warnings;\n\nuse IntellaConfig { ProgramName => 'PgLiveQueueCreateViews' 
};\nus"...)  at  postgres.c:1215
#29  0x000055f7138429d1  in  PostgresMain  (argc=<optimized  out>,  argv=argv@entry=0x55f715723a78,  dbname=<optimized  out>,  username=<optimized  out>)  at  postgres.c:4271
#30  0x000055f7137ce4d9  in  BackendRun  (port=0x55f715716ad0,  port=0x55f715716ad0)  at  postmaster.c:4510
#31  BackendStartup  (port=0x55f715716ad0)  at  postmaster.c:4193
#32  ServerLoop  ()  at  postmaster.c:1725
#33  0x000055f7137cf400  in  PostmasterMain  (argc=5,  argv=0x55f7156cd030)  at  postmaster.c:1398
#34  0x000055f71355bcca  in  main  (argc=5,  argv=0x55f7156cd030)  at  main.c:228' Culprit: plperl_spi_exec(char *query, int limit) 
....snip.... PG_TRY(); ...snip.... spi_rv = SPI_execute(query, 
current_call_data->prodesc->fn_readonly, limit); Fix: bool read_only = 
(current_call_data ? current_call_data->prodesc->fn_readonly : 0); 
...snip... spi_rv = SPI_execute(query, read_only, limit); Attached patch 
against 12.10

  2. Re: Bug plperl.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-22T19:48:22Z

    Mark Murawski <markm-lists@intellasoft.net> writes:
> Affects Versions: 12 (and probably all others)
> Steps to Reproduce:
> - Run a query during plperl validation -- ie: inside a warn handler
> 2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG:  server process 
> (PID 19702) was terminated by signal 11: Segmentation fault

I couldn't reproduce this, either in HEAD or 12.10.  However, maybe
there's something faulty about your example, because in my hands it
doesn't seem that plperl_spi_exec is reached at all.

If we do need to do something here, my inclination would be to
reject execution of any SPI operations during validation.  That's
not a case that's supposed to have any side-effects.

			regards, tom lane

  3. Re: Bug plperl.c

    Mark Murawski <markm-lists@intellasoft.net> — 2022-02-22T20:06:19Z

    Hi Tom,

Wow that was fast!

Upon further testing, my path isn't a full fix... there's other parts of 
the flow that needs current_call_data


I trimmed down my example from the guts of a complex application. Sorry 
I didn't re-test with the trimmed example.  But the use-case is very 
much the same... it has to do with running spi_exec_query during parse time.

I'll work on redoing the example to reproduce the crash.



On 2/22/22 14:48, Tom Lane wrote:
> Mark Murawski <markm-lists@intellasoft.net> writes:
>> Affects Versions: 12 (and probably all others)
>> Steps to Reproduce:
>> - Run a query during plperl validation -- ie: inside a warn handler
>> 2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG:  server process
>> (PID 19702) was terminated by signal 11: Segmentation fault
> I couldn't reproduce this, either in HEAD or 12.10.  However, maybe
> there's something faulty about your example, because in my hands it
> doesn't seem that plperl_spi_exec is reached at all.
>
> If we do need to do something here, my inclination would be to
> reject execution of any SPI operations during validation.  That's
> not a case that's supposed to have any side-effects.
>
> 			regards, tom lane

  4. Re: Bug plperl.c

    Mark Murawski <markm-lists@intellasoft.net> — 2022-02-22T20:27:09Z

    Hi Tom

Here you go:

CREATE OR REPLACE FUNCTION public.foo()
  RETURNS text
  LANGUAGE plperlu
AS $function$

use warnings;
use strict;

use lib '/tmp';
use foo;

my @a;
my @a;

$function$


---
/tmp/foo.pm
---

package foo;

use Data::Dumper;

$SIG{'__WARN__'} = sub {
    my $pid = main::spi_exec_query('SELECT pg_backend_pid()')->{rows};
   main::elog(main::NOTICE, 'Warning:' . Dumper(\@_));
   main::elog(main::NOTICE, 'foo: ' . $pid);
};

1;




On 2/22/22 15:06, Mark Murawski wrote:
> Hi Tom,
>
> Wow that was fast!
>
> Upon further testing, my path isn't a full fix... there's other parts 
> of the flow that needs current_call_data
>
>
> I trimmed down my example from the guts of a complex application. 
> Sorry I didn't re-test with the trimmed example.  But the use-case is 
> very much the same... it has to do with running spi_exec_query during 
> parse time.
>
> I'll work on redoing the example to reproduce the crash.
>
>
>
> On 2/22/22 14:48, Tom Lane wrote:
>> Mark Murawski <markm-lists@intellasoft.net> writes:
>>> Affects Versions: 12 (and probably all others)
>>> Steps to Reproduce:
>>> - Run a query during plperl validation -- ie: inside a warn handler
>>> 2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG:  server process
>>> (PID 19702) was terminated by signal 11: Segmentation fault
>> I couldn't reproduce this, either in HEAD or 12.10.  However, maybe
>> there's something faulty about your example, because in my hands it
>> doesn't seem that plperl_spi_exec is reached at all.
>>
>> If we do need to do something here, my inclination would be to
>> reject execution of any SPI operations during validation. That's
>> not a case that's supposed to have any side-effects.
>>
>>             regards, tom lane
>

  5. Re: Bug plperl.c

    Mark Murawski <markm-lists@intellasoft.net> — 2022-02-25T20:15:09Z

    HI Tom,

Were you able to reproduce using the updated example?

Thanks.



On 2/22/22 15:27, Mark Murawski wrote:
> Hi Tom
>
> Here you go:
>
> CREATE OR REPLACE FUNCTION public.foo()
>  RETURNS text
>  LANGUAGE plperlu
> AS $function$
>
> use warnings;
> use strict;
>
> use lib '/tmp';
> use foo;
>
> my @a;
> my @a;
>
> $function$
>
>
> ---
> /tmp/foo.pm
> ---
>
> package foo;
>
> use Data::Dumper;
>
> $SIG{'__WARN__'} = sub {
>    my $pid = main::spi_exec_query('SELECT pg_backend_pid()')->{rows};
>   main::elog(main::NOTICE, 'Warning:' . Dumper(\@_));
>   main::elog(main::NOTICE, 'foo: ' . $pid);
> };
>
> 1;
>
>
>
>
> On 2/22/22 15:06, Mark Murawski wrote:
>> Hi Tom,
>>
>> Wow that was fast!
>>
>> Upon further testing, my path isn't a full fix... there's other parts 
>> of the flow that needs current_call_data
>>
>>
>> I trimmed down my example from the guts of a complex application. 
>> Sorry I didn't re-test with the trimmed example. But the use-case is 
>> very much the same... it has to do with running spi_exec_query during 
>> parse time.
>>
>> I'll work on redoing the example to reproduce the crash.
>>
>>
>>
>> On 2/22/22 14:48, Tom Lane wrote:
>>> Mark Murawski <markm-lists@intellasoft.net> writes:
>>>> Affects Versions: 12 (and probably all others)
>>>> Steps to Reproduce:
>>>> - Run a query during plperl validation -- ie: inside a warn handler
>>>> 2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG: server process
>>>> (PID 19702) was terminated by signal 11: Segmentation fault
>>> I couldn't reproduce this, either in HEAD or 12.10.  However, maybe
>>> there's something faulty about your example, because in my hands it
>>> doesn't seem that plperl_spi_exec is reached at all.
>>>
>>> If we do need to do something here, my inclination would be to
>>> reject execution of any SPI operations during validation. That's
>>> not a case that's supposed to have any side-effects.
>>>
>>>             regards, tom lane
>>
>

  6. Re: Bug plperl.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-25T21:36:42Z

    Mark Murawski <markm-lists@intellasoft.net> writes:
> Were you able to reproduce using the updated example?

Sorry, this wasn't at the top of my to-do queue.  It does reproduce
for me, and I think what we need to do about it is the attached.
In the normal code paths, this change will disallow usage of SPI until
we have completed compile_plperl_function and have a valid "prodesc"
to look at.  I didn't care for your proposed workaround because

(1) it'd allow execution of non-read-only code during compilation
of a supposedly read-only function;

(2) it didn't patch the dozen or so other places where plperl SPI
functions could try to dereference prodesc;

(3) allowing code execution during function validation is, if not
an actual security hole, certainly on the hairy edge of being one.

I'm somewhat comforted about (3) because it seems the problem is only
reachable from plperlu not plperl.  It's still pretty scary though.

I realize that this solution might make your use-case rather awkward.
As far as function validation goes, you can still create your functions
by setting check_function_bodies = off.  If you feel you need to have
Perl code that executes during compilation otherwise, I'm not sure
what to tell you, except that it doesn't seem like a great idea.

I also noticed while looking at this that the relatively-recently-added
plperl_spi_commit and plperl_spi_rollback functions neglected to do
check_spi_usage_allowed(), so this fixes that too.

			regards, tom lane

  7. Re: Bug plperl.c

    Mark Murawski <markm-lists@intellasoft.net> — 2022-03-08T19:59:17Z

    Hi Tom,

No rush on the bug fix, just making sure you don't need anything else 
from me on the reproduction.

Yeah I realized my patch wasn't a full solution after sending it in... 
My test environment was a little wiggity, and I compiled and tested... 
but noticed I actually wasn't using the new build... (and thought that 
it was fixed with my change)

Based on the side-effects I think it does make sense to block queries 
entirely during parse


On 2/25/22 16:36, Tom Lane wrote:
> Mark Murawski <markm-lists@intellasoft.net> writes:
>> Were you able to reproduce using the updated example?
> Sorry, this wasn't at the top of my to-do queue.  It does reproduce
> for me, and I think what we need to do about it is the attached.
> In the normal code paths, this change will disallow usage of SPI until
> we have completed compile_plperl_function and have a valid "prodesc"
> to look at.  I didn't care for your proposed workaround because
>
> (1) it'd allow execution of non-read-only code during compilation
> of a supposedly read-only function;
>
> (2) it didn't patch the dozen or so other places where plperl SPI
> functions could try to dereference prodesc;
>
> (3) allowing code execution during function validation is, if not
> an actual security hole, certainly on the hairy edge of being one.
>
> I'm somewhat comforted about (3) because it seems the problem is only
> reachable from plperlu not plperl.  It's still pretty scary though.
>
> I realize that this solution might make your use-case rather awkward.
> As far as function validation goes, you can still create your functions
> by setting check_function_bodies = off.  If you feel you need to have
> Perl code that executes during compilation otherwise, I'm not sure
> what to tell you, except that it doesn't seem like a great idea.
>
> I also noticed while looking at this that the relatively-recently-added
> plperl_spi_commit and plperl_spi_rollback functions neglected to do
> check_spi_usage_allowed(), so this fixes that too.
>
> 			regards, tom lane
>

  8. Re: Bug plperl.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-08T21:19:39Z

    Mark Murawski <markm-lists@intellasoft.net> writes:
> No rush on the bug fix, just making sure you don't need anything else 
> from me on the reproduction.

Nope, it's dealt with, see

https://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=638300fef

			regards, tom lane