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.

diff --git a/gv.c b/gv.c
index 81fa5de..ea41e78 100644 (file)
--- a/gv.c
+++ b/gv.c
@@ -1339,6 +1339,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)
@@ -1368,6 +1369,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 6df2101..5305521 100644 (file)
--- 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 (file)
--- 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);