8000 Block environment variable mutations from trusted PL/Perl. · mbeccati/postgres@b7e3a52 · GitHub
[go: up one dir, main page]
Skip to content

Commit b7e3a52

Browse files
nmischnmisch
committed
Block environment variable mutations from trusted PL/Perl.
Many process environment variables (e.g. PATH), bypass the containment expected of a trusted PL. Hence, trusted PLs must not offer features that achieve setenv(). Otherwise, an attacker having USAGE privilege on the language often can achieve arbitrary code execution, even if the attacker lacks a database server operating system user. To fix PL/Perl, replace trusted PL/Perl %ENV with a tied hash that just replaces each modification attempt with a warning. Sites that reach these warnings should evaluate the application-specific implications of proceeding without the environment modification: Can the application reasonably proceed without the modification? If no, switch to plperlu or another approach. If yes, the application should change the code to stop attempting environment modifications. If that's too difficult, add "untie %main::ENV" in any code executed before the warning. For example, one might add it to the start of the affected function or even to the plperl.on_plperl_init setting. In passing, link to Perl's guidance about the Perl features behind the security posture of PL/Perl. Back-patch to v12 (all supported versions). Andrew Dunstan and Noah Misch Security: CVE-2024-10979
1 parent 220cea9 commit b7e3a52

File tree

7 files changed

+176
-2
lines changed

7 files changed

+176
-2
lines changed

‎doc/src/sgml/plperl.sgml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,6 +1093,19 @@ $$ LANGUAGE plperl;
10931093
be permitted to use this language.
10941094
</para>
10951095

1096+
<warning>
1097+
<para>
1098+
Trusted PL/Perl relies on the Perl <literal>Opcode</literal> module to
1099+
preserve security.
1100+
Perl
1101+
<ulink url="https://perldoc.perl.org/Opcode#WARNING">documents</ulink>
1102+
that the module is not effective for the trusted PL/Perl use case. If
1103+
your security needs are incompatible with the uncertainty in that warning,
1104+
consider executing <literal>REVOKE USAGE ON LANGUAGE plperl FROM
1105+
PUBLIC</literal>.
1106+
</para>
1107+
</warning>
1108+
10961109
<para>
10971110
Here is an example of a function that will not work because file
10981111
system operations are not allowed for security reasons:

‎src/pl/plperl/GNUmakefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ ifeq ($(PORTNAME), cygwin)
6060
SHLIB_LINK += -Wl,--export-all-symbols
6161
endif
6262

63-
REGRESS_OPTS = --dbname=$(PL_TESTDB)
63+
REGRESS_OPTS = --dbname=$(PL_TESTDB) --dlpath=$(top_builddir)/src/test/regress
6464
REGRESS = plperl_setup plperl plperl_lc plperl_trigger plperl_shared \
6565
plperl_elog plperl_util plperl_init plperlu plperl_array \
66-
plperl_call plperl_transaction
66+
plperl_call plperl_transaction plperl_env
6767
# if Perl can support two interpreters in one backend, 8000
6868
# test plperl-and-plperlu cases
6969
ifneq ($(PERL),)

‎src/pl/plperl/expected/plperl_env.out

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
--
2+
-- Test the environment setting
3+
--
4+
-- directory path and dlsuffix are passed to us in environment variables
5+
\getenv libdir PG_LIBDIR
6+
\getenv dlsuffix PG_DLSUFFIX
7+
\set regresslib :libdir '/regress' :dlsuffix
8+
CREATE FUNCTION get_environ()
9+
RETURNS text[]
10+
AS :'regresslib', 'get_environ'
11+
LANGUAGE C STRICT;
12+
-- fetch the process environment
13+
CREATE FUNCTION process_env () RETURNS text[]
14+
LANGUAGE plpgsql AS
15+
$$
16+
17+
declare
18+
res text[];
19+
tmp text[];
20+
f record;
21+
begin
22+
for f in select unnest(get_environ()) as t loop
23+
tmp := regexp_split_to_array(f.t, '=');
24+
if array_length(tmp, 1) = 2 then
25+
res := res || tmp;
26+
end if;
27+
end loop;
28+
return res;
29+
end
30+
31+
$$;
32+
-- plperl should not be able to affect the process environment
33+
DO
34+
$$
35+
$ENV{TEST_PLPERL_ENV_FOO} = "shouldfail";
36+
untie %ENV;
37+
$ENV{TEST_PLPERL_ENV_FOO} = "testval";
38+
my $penv = spi_exec_query("select unnest(process_env()) as pe");
39+
my %received;
40+
for (my $f = 0; $f < $penv->{processed}; $f += 2)
41+
{
42+
my $k = $penv->{rows}[$f]->{pe};
43+
my $v = $penv->{rows}[$f+1]->{pe};
44+
$received{$k} = $v;
45+
}
46+
unless (exists $received{TEST_PLPERL_ENV_FOO})
47+
{
48+
elog(NOTICE, "environ unaffected")
49+
}
50+
51+
$$ LANGUAGE plperl;
52+
WARNING: attempted alteration of $ENV{TEST_PLPERL_ENV_FOO} at line 12.
53+
NOTICE: environ unaffected

‎src/pl/plperl/meson.build

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,9 @@ tests += { A3E2
9494
'plperl_array',
9595
'plperl_call',
9696
'plperl_transaction',
97+
'plperl_env',
9798
],
99+
'regress_args': ['--dlpath', meson.build_root() / 'src/test/regress'],
98100
},
99101
}
100102

‎src/pl/plperl/plc_trusted.pl

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,27 @@ package PostgreSQL::InServer::safe; ## no critic (RequireFilenameMatchesPackage)
3030
require Carp::Heavy;
3131
require warnings;
3232
require feature if $] >= 5.010000;
33+
34+
#<<< protect next line from perltidy so perlcritic annotation works
35+
package PostgreSQL::InServer::WarnEnv; ## no critic (RequireFilenameMatchesPackage)
36+
#>>>
37+
38+
use strict;
39+
use warnings;
40+
use Tie::Hash;
41+
our @ISA = qw(Tie::StdHash);
42+
43+
sub STORE { warn "attempted alteration of \$ENV{$_[1]}"; }
44 F438 +
sub DELETE { warn "attempted deletion of \$ENV{$_[1]}"; }
45+
sub CLEAR { warn "attempted clearance of ENV hash"; }
46+
47+
# Remove magic property of %ENV. Changes to this will now not be reflected in
48+
# the process environment.
49+
*main::ENV = {%ENV};
50+
51+
# Block %ENV changes from trusted PL/Perl, and warn. We changed %ENV to just a
52+
# normal hash, yet the application may be expecting the usual Perl %ENV
53+
# magic. Blocking and warning avoids silent application breakage. The user can
54+
# untie or otherwise disable this, e.g. if the lost mutation is unimportant
55+
# and modifying the code to stop that mutation would be onerous.
56+
tie %main::ENV, 'PostgreSQL::InServer::WarnEnv', %ENV or die $!;

‎src/pl/plperl/sql/plperl_env.sql

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
--
2+
-- Test the environment setting
3+
--
4+
5+
-- directory path and dlsuffix are passed to us in environment variables
6+
\getenv libdir PG_LIBDIR
7+
\getenv dlsuffix PG_DLSUFFIX
8+
9+
\set regresslib :libdir '/regress' :dlsuffix
10+
11+
CREATE FUNCTION get_environ()
12+
RETURNS text[]
13+
AS :'regresslib', 'get_environ'
14+
LANGUAGE C STRICT;
15+
16+
-- fetch the process environment
17+
18+
CREATE FUNCTION process_env () RETURNS text[]
19+
LANGUAGE plpgsql AS
20+
$$
21+
22+
declare
23+
res text[];
24+
tmp text[];
25+
f record;
26+
begin
27+
for f in select unnest(get_environ()) as t loop
28+
tmp := regexp_split_to_array(f.t, '=');
29+
if array_length(tmp, 1) = 2 then
30+
res := res || tmp;
31+
end if;
32+
end loop;
33+
return res;
34+
end
35+
36+
$$;
37+
38+
-- plperl should not be able to affect the process environment
39+
40+
DO
41+
$$
42+
$ENV{TEST_PLPERL_ENV_FOO} = "shouldfail";
43+
untie %ENV;
44+
$ENV{TEST_PLPERL_ENV_FOO} = "testval";
45+
my $penv = spi_exec_query("select unnest(process_env()) as pe");
46+
my %received;
47+
for (my $f = 0; $f < $penv->{processed}; $f += 2)
48+
{
49+
my $k = $penv->{rows}[$f]->{pe};
50+
my $v = $penv->{rows}[$f+1]->{pe};
51+
$received{$k} = $v;
52+
}
53+
unless (exists $received{TEST_PLPERL_ENV_FOO})
54+
{
55+
elog(NOTICE, "environ unaffected")
56+
}
57+
58+
$$ LANGUAGE plperl;

‎src/test/regress/regress.c

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "parser/parse_coerce.h"
3838
#include "port/atomics.h"
3939
#include "storage/spin.h"
40+
#include "utils/array.h"
4041
#include "utils/builtins.h"
4142
#include "utils/geo_decls.h"
4243
#include "utils/memutils.h"
@@ -641,6 +642,29 @@ make_tuple_indirect(PG_FUNCTION_ARGS)
641642
PG_RETURN_POINTER(newtup->t_data);
642643
}
643644

645+
PG_FUNCTION_INFO_V1(get_environ);
646+
647+
Datum
648+
get_environ(PG_FUNCTION_ARGS)
649+
{
650+
extern char **environ;
651+
int nvals = 0;
652+
ArrayType *result;
653+
Datum *env;
654+
655+
for (char **s = environ; *s; s++)
656+
nvals++;
657+
658+
env = palloc(nvals * sizeof(Datum));
659+
660+
for (int i = 0; i < nvals; i++)
661+
env[i] = CStringGetTextDatum(environ[i]);
662+
663+
result = construct_array_builtin(env, nvals, TEXTOID);
664+
665+
PG_RETURN_POINTER(result);
666+
}
667+
644668
PG_FUNCTION_INFO_V1(regress_setenv);
645669

646670
Datum

0 commit comments

Comments
 (0)
0