Skip to content

Commit

Permalink
RT #67962: $1 treated as tainted in untainted match
Browse files Browse the repository at this point in the history
Fix the issue in the following:

    use re 'taint';
    $tainted =~ /(...)/;
    # $1 now correctly tainted
    $untainted =~ s/(...)/$1/;
    # $untainted now incorrectly tainted

The problem stems from when $1 is updated.

pp_substcont, which is called after the replacement expression has been
evaluated, checks the returned expression for taintedness, and if so,
taints the variable being substituted. For a substitution like
s/(...)/x$1/ this works fine: the expression "x".$1 causes $1's get magic
to be called, which sets $1 based on the recent match, and is marked as
not tainted.  Thus the returned expression is untainted. In the variant
s/(...)/$1/, the returned value on the stack is $1 itself, and its get
magic hasn't been called yet. So it still has the tainted flag from the
previous pattern.

The solution is to mg_get the returned expression *before* testing for
taintedness.
  • Loading branch information
iabyn committed Mar 25, 2010
1 parent fd69380 commit 447ee13
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
4 changes: 3 additions & 1 deletion pp_ctl.c
Expand Up @@ -278,9 +278,11 @@ PP(pp_substcont)
if (cx->sb_iters > cx->sb_maxiters)
DIE(aTHX_ "Substitution loop");

SvGETMAGIC(TOPs); /* possibly clear taint on $1 etc: #67962 */

if (!(cx->sb_rxtainted & 2) && SvTAINTED(TOPs))
cx->sb_rxtainted |= 2;
sv_catsv(dstr, POPs);
sv_catsv_nomg(dstr, POPs);
/* XXX: adjust for positive offsets of \G for instance s/(.)\G//g with positive pos() */
s -= RX_GOFS(rx);

Expand Down
18 changes: 17 additions & 1 deletion t/op/taint.t
Expand Up @@ -17,7 +17,7 @@ use Config;
use File::Spec::Functions;

BEGIN { require './test.pl'; }
plan tests => 321;
plan tests => 325;

$| = 1;

Expand Down Expand Up @@ -1380,6 +1380,22 @@ foreach my $ord (78, 163, 256) {
}


# Bug RT #67962: old tainted $1 gets treated as tainted
# in next untainted # match

{
use re 'taint';
"abc".$TAINT =~ /(.*)/; # make $1 tainted
ok(tainted($1), '$1 should be tainted');

my $untainted = "abcdef";
ok(!tainted($untainted), '$untainted should be untainted');
$untainted =~ s/(abc)/$1/;
ok(!tainted($untainted), '$untainted should still be untainted');
$untainted =~ s/(abc)/x$1/;
ok(!tainted($untainted), '$untainted should yet still be untainted');
}


# This may bomb out with the alarm signal so keep it last
SKIP: {
Expand Down

0 comments on commit 447ee13

Please sign in to comment.