[perl #45167] Taint removal by sprintf

Under some circumstances the value returned by sprintf wasn't tainted,
even though its args were. While trying to fix this, I also came across
a second bug (which made fixing the first bug very confusing!) where
the TARG of the sprintf op, after getting tainted once, permanently
retained taint magic, which depending on circumstances, wasn't always set
to untainted (mg_len =0)

The original bug basically boiled down to parts of Perl_sv_vcatpvfn()
directly manipulating the target with SvGROW() / Copy(), which failed
to taint the target. Other parts used sv_catsv(), which did. So for
example:
    "%s%s" failed, (only SvGROW)
    "%s %s" worked (the space char was appended using sv_catsv).

diff --git a/pp.c b/pp.c
index 5876cfd..857ebec 100644 (file)
--- a/pp.c
+++ b/pp.c
@@ -3439,6 +3439,7 @@ PP(pp_sprintf)
     dVAR; dSP; dMARK; dORIGMARK; dTARGET;
     if (SvTAINTED(MARK[1]))
        TAINT_PROPER("sprintf");
+    SvTAINTED_off(TARG);
     do_sprintf(TARG, SP-MARK, MARK+1);
     TAINT_IF(SvTAINTED(TARG));
     SP = ORIGMARK;

diff --git a/sv.c b/sv.c
index b6c03ed..5759b2b 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -10431,6 +10431,7 @@ Perl_sv_vcatpvfn(pTHX_ SV *const sv, const char *const pat, const STRLEN patlen,
            goto vector;
        }
     }
+    SvTAINT(sv);
 }
 
 /* =========================================================================

diff --git a/t/op/taint.t b/t/op/taint.t
index c947044..b4c8bfe 100644 (file)
--- a/t/op/taint.t
+++ b/t/op/taint.t
@@ -17,7 +17,7 @@ use Config;
 use File::Spec::Functions;
 
 BEGIN { require './test.pl'; }
-plan tests => 307;
+plan tests => 319;
 
 $| = 1;
 
@@ -1347,6 +1347,33 @@ foreach my $ord (78, 163, 256) {
     ok $i == 2, "tied STORE called correct number of times";
 }
 
+# Bug RT #45167 the return value of sprintf sometimes wasn't tainted
+# when the args were tainted. This only occured on the first use of
+# sprintf; after that, its TARG has taint magic attached, so setmagic
+# at the end works.  That's why there are multiple sprintf's below, rather
+# than just one wrapped in an inner loop. Also, any plantext betwerrn
+# fprmat entires would correctly cause tainting to get set. so test with
+# "%s%s" rather than eg "%s %s".
+
+{
+    for my $var1 ($TAINT, "123") {
+       for my $var2 ($TAINT0, "456") {
+           my @s;
+           push @s, sprintf '%s', $var1, $var2;
+           push @s, sprintf ' %s', $var1, $var2;
+           push @s, sprintf '%s%s', $var1, $var2;
+           for (0..2) {
+               ok( !(
+                       tainted($s[$_]) xor
+                       (tainted($var1) || ($_==2 && tainted($var2)))
+                   ),
+                   "sprintf fmt$_, '$var1', '$var2'");
+           }
+       }
+    }
+}
+
+
 
 # This may bomb out with the alarm signal so keep it last
 SKIP: {