AddressSanitizer: heap-use-after-free in Perl_pp_rv2sv #15893
Comments
From mtowalski@pentest.net.plHello, I've attached the poc and the asan log. Configure options: “./Configure -des -Dusedevel -DDEBUGGING -Dcc=clang -Doptimize=-O2 -Accflags="-fsanitize=address -fsanitize-coverage=edge" -Aldflags="-fsanitize=address -fsanitize-coverage=edge" -Alddlflags=-shared" Information about configuration: Distributor ID: Ubuntu Best Regards, |
From mtowalski@pentest.net.plperl: warning: Setting locale failed.
|
From @arcOn 25 February 2017 at 23:56, via RT <perl5-security-report@perl.org> wrote:
That asan log is: ==708==ERROR: AddressSanitizer: heap-use-after-free on address 0x619000009a78 is located 1016 bytes inside of 1024-byte region previously allocated by thread T0 here: which isn't terribly helpful. However, I get something much more ==75998==ERROR: AddressSanitizer: heap-use-after-free on address 0x619000007c70 is located 1008 bytes inside of 1024-byte region previously allocated by thread T0 here: That is: the freed memory was allocated by Perl_new_stackinfo, and The attached patch fixes this for me, and includes a slightly reduced Also, if someone can find a better reduction, that'd be great; I found -- |
From @arc0001-pp_rv2sv-SPAGAIN-after-doing-things-that-could-exten.patchFrom 3952d952ed32364a95c9bdd8fd28c2ecd986f999 Mon Sep 17 00:00:00 2001
From: Aaron Crane <arc@cpan.org>
Date: Sun, 26 Feb 2017 11:36:17 +0000
Subject: [PATCH] pp_rv2sv: SPAGAIN after doing things that could extend the
stack
---
pp.c | 1 +
t/op/ref.t | 14 +++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/pp.c b/pp.c
index 62316fc8b4..833dabb70a 100644
--- a/pp.c
+++ b/pp.c
@@ -403,6 +403,7 @@ PP(pp_rv2sv)
else if (PL_op->op_private & OPpDEREF)
sv = vivify_ref(sv, PL_op->op_private & OPpDEREF);
}
+ SPAGAIN; /* in case chasing soft refs reallocated the stack */
SETs(sv);
RETURN;
}
diff --git a/t/op/ref.t b/t/op/ref.t
index 65d50b67a2..0ab9883d3e 100644
--- a/t/op/ref.t
+++ b/t/op/ref.t
@@ -8,7 +8,7 @@ BEGIN {
use strict qw(refs subs);
-plan(236);
+plan(237);
# Test this first before we extend the stack with other operations.
# This caused an asan failure due to a bad write past the end of the stack.
@@ -820,6 +820,18 @@ for ("4eounthouonth") {
'[perl #109746] referential identity of \literal under threads+mad'
}
+# RT#130861: heap-use-after-free in pp_rv2sv, from asan fuzzing
+SKIP: {
+ skip_if_miniperl("no dynamic loading on miniperl, so can't load arybase", 1);
+ my $code = <<'EOF';
+@q = "-1ocT\xC2\xA8gZZZZZZZZZZZZZZZZZZZZZZZZZZZYT`gZYT`gZYT`gZYT`gZYT`gZYT`gZYT`gZY\xD4`cV\xC2\\m;(\\d+|.);g;
+\xC2\xA8xcV\\\xC2\xA8`ge\$_[0.07\x9E\xF6\xD8\xFCF\xE6U\xFA\x16\xF9E}VVVVVVV"=~m;(\d+|.);g;
+() = map $$$$$$_, @q;
+EOF
+ fresh_perl_like($code, qr/^Not a SCALAR reference /, { stderr => 1 },
+ 'rt#130861: heap uaf in pp_rv2sv');
+}
+
# Bit of a hack to make test.pl happy. There are 3 more tests after it leaves.
$test = curr_test();
curr_test($test + 3);
--
2.11.0
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Sun, 26 Feb 2017 04:13:44 -0800, arc wrote:
Here's a slightly different minification. Changes to the literal that preserve the number of matches against the regexp were safe, so compressing sequences of digits, replacing non-ASCII characters and removing the new lines from the literal worked for me. Changing the literal to a generated value ("1" . ("x" x 123)) didn't reproduce the issue. WRT arybase, perhaps require_tie_mod() needs to work with a new stack, perhaps gv_fetchpvn_flags() reallocating the stack is a bug. Tony |
From @tonycoz |
From @iabynOn Sun, Feb 26, 2017 at 08:22:01PM -0800, Tony Cook via RT wrote:
I attach an alternative patch which I suggest should supersede Aaron's one. I don't think it really counts as a security issue: it involves So I think this ticket should be moved to the public queue soonish, then -- |
From @iabyn0001-S_require_tie_mod-use-a-new-stack.patch>From cd763c7f0f5abd08207c7852a2cba1c24d2904b8 Mon Sep 17 00:00:00 2001
From: David Mitchell <davem@iabyn.com>
Date: Tue, 14 Mar 2017 09:19:15 +0000
Subject: [PATCH] S_require_tie_mod(): use a new stack
RT #130861
This function is used to load a module associated with various magic vars,
like $[ and %+. Since it can be called 'unexpectedly', it should use a new
stack. The issue in this ticket was equivalent to
my $var = '[';
$$var;
where the symbolic dereference triggered a run-time load of arybase.pm,
which grew the stack, invalidating the SP in pp_rv2sv.
Note that most of the stuff which S_require_tie_mod() calls, such as
load_module(), will do its own PUSHSTACK(); but S_require_tie_mod() also
does a bit of stack manipulation itself.
The test case includes a magic number, 125, which happens to be the exact
size necessary to trigger a stack realloc in S_require_tie_mod(). In later
perl versions this value may well change. But it seemed too expensive
to call fresh_perl_is() 100's of times with different values of $n.
This commit also adds a SPAGAIN to pp_rv2sv on the 'belt and braces'
principle.
This commit is based on an earlier effort by Aaron Crane.
---
gv.c | 2 ++
pp.c | 1 +
t/op/ref.t | 20 +++++++++++++++++++-
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/gv.c b/gv.c
index d32a9c5..581211f 100644
--- a/gv.c
+++ b/gv.c
@@ -1338,6 +1338,7 @@ S_require_tie_mod(pTHX_ GV *gv, const char varname, const char * name,
GV **gvp;
dSP;
+ PUSHSTACKi(PERLSI_MAGIC);
ENTER;
#define HV_FETCH_TIE_FUNC (GV **)hv_fetchs(stash, "_tie_it", 0)
@@ -1367,6 +1368,7 @@ S_require_tie_mod(pTHX_ GV *gv, const char varname, const char * name,
PUTBACK;
call_sv((SV *)*gvp, G_VOID|G_DISCARD);
LEAVE;
+ POPSTACK;
}
}
diff --git a/pp.c b/pp.c
index a640995..1275969 100644
--- a/pp.c
+++ b/pp.c
@@ -403,6 +403,7 @@ PP(pp_rv2sv)
else if (PL_op->op_private & OPpDEREF)
sv = vivify_ref(sv, PL_op->op_private & OPpDEREF);
}
+ SPAGAIN; /* in case chasing soft refs reallocated the stack */
SETs(sv);
RETURN;
}
diff --git a/t/op/ref.t b/t/op/ref.t
index 65d50b6..44047ae 100644
--- a/t/op/ref.t
+++ b/t/op/ref.t
@@ -8,7 +8,7 @@ BEGIN {
use strict qw(refs subs);
-plan(236);
+plan(237);
# Test this first before we extend the stack with other operations.
# This caused an asan failure due to a bad write past the end of the stack.
@@ -820,6 +820,24 @@ for ("4eounthouonth") {
'[perl #109746] referential identity of \literal under threads+mad'
}
+# RT#130861: heap-use-after-free in pp_rv2sv, from asan fuzzing
+SKIP: {
+ skip_if_miniperl("no dynamic loading on miniperl, so can't load arybase", 1);
+ # this value is critical - its just enough so that the stack gets
+ # grown which loading/calling arybase
+ my $n = 125;
+
+ my $code = <<'EOF';
+$ary = '[';
+my @a = map $$ary, 1..NNN;
+print "@a\n";
+EOF
+ $code =~ s/NNN/$n/g;
+ my @exp = ("0") x $n;
+ fresh_perl_is($code, "@exp", { stderr => 1 },
+ 'rt#130861: heap uaf in pp_rv2sv');
+}
+
# Bit of a hack to make test.pl happy. There are 3 more tests after it leaves.
$test = curr_test();
curr_test($test + 3);
--
2.4.11
|
From @iabynOn Tue, Mar 14, 2017 at 09:40:54AM +0000, Dave Mitchell wrote:
Now pushed as v5.27.0-118-g655f5b2. I'll move this ticket to the public -- |
|
@iabyn - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#130861 (status was 'resolved')
Searchable as RT130861$
The text was updated successfully, but these errors were encountered: