pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBug plperl.c
8+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2022-02-25 4936747c47 638300fef5 ac910bb232 9b2d762a28 0f7b62f29d 31befa6be6 Disallow execution of SPI functions during plperl function compilation.
* Bug plperl.c
@ 2022-02-22 17:49 Mark Murawski <markm-lists@intellasoft.net>
0 siblings, 1 reply; 8+ messages in thread
From: Mark Murawski @ 2022-02-22 17:49 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
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
Attachments:
postgres-12.10-plperl-warn-handler-crash.patch (text/x-patch, 5.1K) download
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index f5738d0dc9..e4b7ce057f 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -3048,205 +3048,206 @@ plperl_hash_from_tuple(HeapTuple tuple, TupleDesc tupdesc, bool include_generate
for (i = 0; i < tupdesc->natts; i++)
{
Datum attr;
bool isnull,
typisvarlena;
char *attname;
Oid typoutput;
Form_pg_attribute att = TupleDescAttr(tupdesc, i);
if (att->attisdropped)
continue;
if (att->attgenerated)
{
/* don't include unless requested */
if (!include_generated)
continue;
}
attname = NameStr(att->attname);
attr = heap_getattr(tuple, i + 1, tupdesc, &isnull);
if (isnull)
{
/*
* Store (attname => undef) and move on. Note we can't use
* &PL_sv_undef here; see "AVs, HVs and undefined values" in
* perlguts for an explanation.
*/
hv_store_string(hv, attname, newSV(0));
continue;
}
if (type_is_rowtype(att->atttypid))
{
SV *sv = plperl_hash_from_datum(attr);
hv_store_string(hv, attname, sv);
}
else
{
SV *sv;
Oid funcid;
if (OidIsValid(get_base_element_type(att->atttypid)))
sv = plperl_ref_from_pg_array(attr, att->atttypid);
else if ((funcid = get_transform_fromsql(att->atttypid, current_call_data->prodesc->lang_oid, current_call_data->prodesc->trftypes)))
sv = (SV *) DatumGetPointer(OidFunctionCall1(funcid, attr));
else
{
char *outputstr;
/* XXX should have a way to cache these lookups */
getTypeOutputInfo(att->atttypid, &typoutput, &typisvarlena);
outputstr = OidOutputFunctionCall(typoutput, attr);
sv = cstr2sv(outputstr);
pfree(outputstr);
}
hv_store_string(hv, attname, sv);
}
}
return newRV_noinc((SV *) hv);
}
static void
check_spi_usage_allowed(void)
{
/* see comment in plperl_fini() */
if (plperl_ending)
{
/* simple croak as we don't want to involve PostgreSQL code */
croak("SPI functions can not be used in END blocks");
}
}
HV *
plperl_spi_exec(char *query, int limit)
{
HV *ret_hv;
/*
* Execute the query inside a sub-transaction, so we can cope with errors
* sanely
*/
MemoryContext oldcontext = CurrentMemoryContext;
ResourceOwner oldowner = CurrentResourceOwner;
check_spi_usage_allowed();
BeginInternalSubTransaction(NULL);
/* Want to run inside function's memory context */
MemoryContextSwitchTo(oldcontext);
PG_TRY();
{
int spi_rv;
+ /* current_call_data is not available during validation, */
+ bool read_only = (current_call_data ? current_call_data->prodesc->fn_readonly : 0);
pg_verifymbstr(query, strlen(query), false);
- spi_rv = SPI_execute(query, current_call_data->prodesc->fn_readonly,
- limit);
+ spi_rv = SPI_execute(query, read_only, limit);
ret_hv = plperl_spi_execute_fetch_result(SPI_tuptable, SPI_processed,
spi_rv);
/* Commit the inner transaction, return to outer xact context */
ReleaseCurrentSubTransaction();
MemoryContextSwitchTo(oldcontext);
CurrentResourceOwner = oldowner;
}
PG_CATCH();
{
ErrorData *edata;
/* Save error info */
MemoryContextSwitchTo(oldcontext);
edata = CopyErrorData();
FlushErrorState();
/* Abort the inner transaction */
RollbackAndReleaseCurrentSubTransaction();
MemoryContextSwitchTo(oldcontext);
CurrentResourceOwner = oldowner;
/* Punt the error to Perl */
croak_cstr(edata->message);
/* Can't get here, but keep compiler quiet */
return NULL;
}
PG_END_TRY();
return ret_hv;
}
static HV *
plperl_spi_execute_fetch_result(SPITupleTable *tuptable, uint64 processed,
int status)
{
dTHX;
HV *result;
check_spi_usage_allowed();
result = newHV();
hv_store_string(result, "status",
cstr2sv(SPI_result_code_string(status)));
hv_store_string(result, "processed",
(processed > (uint64) UV_MAX) ?
newSVnv((NV) processed) :
newSVuv((UV) processed));
if (status > 0 && tuptable)
{
AV *rows;
SV *row;
uint64 i;
/* Prevent overflow in call to av_extend() */
if (processed > (uint64) AV_SIZE_MAX)
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("query result has too many rows to fit in a Perl array")));
rows = newAV();
av_extend(rows, processed);
for (i = 0; i < processed; i++)
{
row = plperl_hash_from_tuple(tuptable->vals[i], tuptable->tupdesc, true);
av_push(rows, row);
}
hv_store_string(result, "rows",
newRV_noinc((SV *) rows));
}
SPI_freetuptable(tuptable);
return result;
}
/*
* plperl_return_next catches any error and converts it to a Perl error.
* We assume (perhaps without adequate justification) that we need not abort
* the current transaction if the Perl code traps the error.
*/
void
plperl_return_next(SV *sv)
{
MemoryContext oldcontext = CurrentMemoryContext;
PG_TRY();
{
plperl_return_next_internal(sv);
}
PG_CATCH();
{
ErrorData *edata;
/* Must reset elog.c's state */
* Re: Bug plperl.c
@ 2022-02-22 19:48 Tom Lane <tgl@sss.pgh.pa.us>
parent: Mark Murawski <markm-lists@intellasoft.net>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2022-02-22 19:48 UTC (permalink / raw)
To: Mark Murawski <markm-lists@intellasoft.net>; +Cc: pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Bug plperl.c
@ 2022-02-22 20:06 Mark Murawski <markm-lists@intellasoft.net>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 8+ messages in thread
From: Mark Murawski @ 2022-02-22 20:06 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Bug plperl.c
@ 2022-02-22 20:27 Mark Murawski <markm-lists@intellasoft.net>
parent: Mark Murawski <markm-lists@intellasoft.net>
0 siblings, 1 reply; 8+ messages in thread
From: Mark Murawski @ 2022-02-22 20:27 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
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
>
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Bug plperl.c
@ 2022-02-25 20:15 Mark Murawski <markm-lists@intellasoft.net>
parent: Mark Murawski <markm-lists@intellasoft.net>
0 siblings, 1 reply; 8+ messages in thread
From: Mark Murawski @ 2022-02-25 20:15 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
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
>>
>
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Bug plperl.c
@ 2022-02-25 21:36 Tom Lane <tgl@sss.pgh.pa.us>
parent: Mark Murawski <markm-lists@intellasoft.net>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2022-02-25 21:36 UTC (permalink / raw)
To: Mark Murawski <markm-lists@intellasoft.net>; +Cc: pgsql-bugs@lists.postgresql.org
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
Attachments:
prevent-spi-execution-during-plperl-compilation.patch (text/x-diff, 1.3K) download
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 81d9c46e00..3f221ee01b 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -3097,6 +3097,21 @@ check_spi_usage_allowed(void)
/* simple croak as we don't want to involve PostgreSQL code */
croak("SPI functions can not be used in END blocks");
}
+
+ /*
+ * Disallow SPI usage if we're not executing a fully-compiled plperl
+ * function. It might seem impossible to get here in that case, but there
+ * are cases where Perl will try to execute code during compilation. If
+ * we proceed we are likely to crash trying to dereference the prodesc
+ * pointer. Working around that might be possible, but it seems unwise
+ * because it'd allow code execution to happen while validating a
+ * function, which is undesirable.
+ */
+ if (current_call_data == NULL || current_call_data->prodesc == NULL)
+ {
+ /* simple croak as we don't want to involve PostgreSQL code */
+ croak("SPI functions can not be used during function compilation");
+ }
}
@@ -3961,6 +3976,8 @@ plperl_spi_commit(void)
{
MemoryContext oldcontext = CurrentMemoryContext;
+ check_spi_usage_allowed();
+
PG_TRY();
{
SPI_commit();
@@ -3986,6 +4003,8 @@ plperl_spi_rollback(void)
{
MemoryContext oldcontext = CurrentMemoryContext;
+ check_spi_usage_allowed();
+
PG_TRY();
{
SPI_rollback();
* Re: Bug plperl.c
@ 2022-03-08 19:59 Mark Murawski <markm-lists@intellasoft.net>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 8+ messages in thread
From: Mark Murawski @ 2022-03-08 19:59 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
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
>
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Bug plperl.c
@ 2022-03-08 21:19 Tom Lane <tgl@sss.pgh.pa.us>
parent: Mark Murawski <markm-lists@intellasoft.net>
0 siblings, 0 replies; 8+ messages in thread
From: Tom Lane @ 2022-03-08 21:19 UTC (permalink / raw)
To: Mark Murawski <markm-lists@intellasoft.net>; +Cc: pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2022-03-08 21:19 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-22 17:49 Bug plperl.c Mark Murawski <markm-lists@intellasoft.net>
2022-02-22 19:48 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-02-22 20:06 ` Mark Murawski <markm-lists@intellasoft.net>
2022-02-22 20:27 ` Mark Murawski <markm-lists@intellasoft.net>
2022-02-25 20:15 ` Mark Murawski <markm-lists@intellasoft.net>
2022-02-25 21:36 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-03-08 19:59 ` Mark Murawski <markm-lists@intellasoft.net>
2022-03-08 21:19 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox