tainting breakage with index() of a constant #9714

p5pRT · 2009-04-17T18:43:33Z

Migrated from rt.perl.org#64804 (status was 'resolved')

Searchable as RT64804$

p5pRT · 2009-04-17T18:43:35Z

From @ntyni

This is a bug report for perl from Niko Tyni <ntyni@debian.org>,
generated with the help of perlbug 1.36 running under perl 5.10.0.

As reported by Adrian Irving-Beer in <http://bugs.debian.org/291450>,
this unexpectedly throws a fatal taint error when invoked with
two arguments:

#!/usr/bin/perl -T
use constant C_A => $ARGV[0];
use constant C_B => $ARGV[1];
index(C_A, C_B);
open(FOO, "-|");

Reported with 5.8.4, verified with current blead, 5.10.0 and 5.8.8.

The first attached patch adds a TODO test reduced from this.

The second patch fixes it for me, but I'm unsure if this is more like
a workaround. Maybe tainted values shouldn't be inlined at all?

Flags:
category=core
severity=low

Site configuration information for perl 5.10.0:

Configured by Debian Project at Mon Apr 13 05:15:55 UTC 2009.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
Platform:
osname=linux, osvers=2.6.26-1-openvz-amd64, archname=x86_64-linux-gnu-thread-multi
uname='linux minerva 2.6.26-1-openvz-amd64 #1 smp fri mar 13 19:02:24 utc 2009 x86_64 gnulinux '
config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.10 -Darchlib=/usr/lib/perl/5.10 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.10.0 -Dsitearch=/usr/local/lib/perl/5.10.0 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.10.0 -Dd_dosuid -des'
hint=recommended, useposix=true, d_sigaction=define
useithreads=define, usemultiplicity=define
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=define, use64bitall=define, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2 -g',
cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include'
ccversion='', gccversion='4.3.3', gccosandvers=''
intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
alignbytes=8, prototype=define
Linker and Libraries:
ld='cc', ldflags =' -L/usr/local/lib'
libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
perllibs=-ldl -lm -lpthread -lc -lcrypt
libc=/lib/libc-2.9.so, so=so, useshrplib=true, libperl=libperl.so.5.10.0
gnulibc_version='2.9'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib'

Locally applied patches:

@INC for perl 5.10.0:
/etc/perl
/usr/local/lib/perl/5.10.0
/usr/local/share/perl/5.10.0
/usr/lib/perl5
/usr/share/perl5
/usr/lib/perl/5.10
/usr/share/perl/5.10
/usr/local/lib/site_perl
.

Environment for perl 5.10.0:
HOME=/home/niko
LANG=en_US.UTF-8
LANGUAGE (unset)
LC_CTYPE=fi_FI.UTF-8
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home/niko/bin:/home/niko/bin:/home/niko/bin:/usr/local/bin:/usr/bin:/bin:/usr/games:/sbin:/usr/sbin
PERL_BADLANG (unset)
SHELL=/bin/zsh

p5pRT · 2009-04-17T18:43:35Z

From @ntyni

0001-TODO-test-for-index-of-a-tainted-constant.patch

From 828a95e8f8385f6cecde9982ce08e7ae4502e1b6 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Fri, 17 Apr 2009 21:11:08 +0300
Subject: [PATCH] TODO test for index() of a tainted constant

As reported by Adrian Irving-Beer in <http://bugs.debian.org/291450>,
this unexpectedly throws a fatal taint error:

    #!/usr/bin/perl -T
    use constant C_A => $ARGV[0];
    use constant C_B => $ARGV[1];
    index(C_A, C_B);
    open(FOO, "-|");

The TODO test is reduced from the above.
---
 t/op/taint.t |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/t/op/taint.t b/t/op/taint.t
index 01ab368..12a931b 100755
--- 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 => 298;
+plan tests => 302;
 
 $| = 1;
 
@@ -1303,6 +1303,18 @@ foreach my $ord (78, 163, 256) {
     }
 }
 
+# tainted constants and index()
+# http://bugs.debian.org/291450
+{
+    ok(tainted $old_env_path, "initial taintedness");
+    BEGIN { no strict 'refs'; my $v = $old_env_path; *{"::C"} = sub () { $v }; }
+    ok(tainted C, "constant is tainted properly");
+    ok(!tainted "", "tainting not broken yet");
+    index(undef, C);
+    local $::TODO = 'breaks when fbm_compile() is called';
+    ok(!tainted "", "tainting still works after index() of the constant");
+}
+
 # This may bomb out with the alarm signal so keep it last
 SKIP: {
     skip "No alarm()"  unless $Config{d_alarm};
-- 
1.5.6.5

p5pRT · 2009-04-17T18:43:35Z

From @ntyni

0002-Don-t-fbm_compile-tainted-constants.patch

From c1a3981525d1b69783f3c01e054758cf652395fa Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Fri, 17 Apr 2009 21:21:32 +0300
Subject: [PATCH] Don't fbm_compile() tainted constants

As seen by the TODO test, calling fbm_compile() with a tainted
value would break subsequent tainting altogether.
---
 op.c         |    3 ++-
 t/op/taint.t |    1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/op.c b/op.c
index be98c3f..b96d277 100644
--- a/op.c
+++ b/op.c
@@ -7204,7 +7204,8 @@ Perl_ck_index(pTHX_ OP *o)
 	OP *kid = cLISTOPo->op_first->op_sibling;	/* get past pushmark */
 	if (kid)
 	    kid = kid->op_sibling;			/* get past "big" */
-	if (kid && kid->op_type == OP_CONST)
+	if (kid && kid->op_type == OP_CONST
+	&& !SvTAINTED((SV *)((SVOP*)kid)->op_sv))
 	    fbm_compile(((SVOP*)kid)->op_sv, 0);
     }
     return ck_fun(o);
diff --git a/t/op/taint.t b/t/op/taint.t
index 12a931b..f04fff2 100755
--- a/t/op/taint.t
+++ b/t/op/taint.t
@@ -1311,7 +1311,6 @@ foreach my $ord (78, 163, 256) {
     ok(tainted C, "constant is tainted properly");
     ok(!tainted "", "tainting not broken yet");
     index(undef, C);
-    local $::TODO = 'breaks when fbm_compile() is called';
     ok(!tainted "", "tainting still works after index() of the constant");
 }
 
-- 
1.5.6.5

p5pRT · 2011-06-28T16:12:13Z

From @iabyn

On Fri, Apr 17, 2009 at 11:43:36AM -0700, Niko Tyni wrote:

As reported by Adrian Irving-Beer in <http://bugs.debian.org/291450>,
this unexpectedly throws a fatal taint error when invoked with
two arguments:
    \#\!/usr/bin/perl \-T
    use constant C\_A => $ARGV\[0\];
    use constant C\_B => $ARGV\[1\];
    index$C\_A\, C\_B$;
    open$FOO\, "\-|"$;
Reported with 5.8.4, verified with current blead, 5.10.0 and 5.8.8.

The first attached patch adds a TODO test reduced from this.

Thanks, applied as 0d1104b

The second patch fixes it for me, but I'm unsure if this is more like
a workaround. Maybe tainted values shouldn't be inlined at all?

I applied an alternative fix as 3b36395.

--
"Do not dabble in paradox, Edward, it puts you in danger of fortuitous wit."
-- Lady Croom, "Arcadia"

p5pRT · 2011-06-28T16:12:13Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2011-06-28T16:12:44Z

@iabyn - Status changed from 'open' to 'resolved'

p5pRT · 2012-01-12T20:02:48Z

From bitcard8493823322@mstier.de

Bug affects recent stable version 5.14.2.

Maybe we should add a regression test.

p5pRT · 2012-01-13T12:41:43Z

From @iabyn

On Thu, Jan 12, 2012 at 12:02:49PM -0800, Mark via RT wrote:

Bug affects recent stable version 5.14.2.

Maybe we should add a regression test.

A regression test was added as part of the fix. The fix went into the
development branch (5.15.1), and presumably hasn't been back-ported to
maint-5.14.

--
You're only as old as you look.

p5pRT closed this as completed Jun 28, 2011

p5pRT added Severity Low hasPatch security type-core labels Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tainting breakage with index() of a constant #9714

tainting breakage with index() of a constant #9714

p5pRT commented Apr 17, 2009

p5pRT commented Apr 17, 2009

p5pRT commented Apr 17, 2009

p5pRT commented Apr 17, 2009

p5pRT commented Jun 28, 2011

p5pRT commented Jun 28, 2011

p5pRT commented Jun 28, 2011

p5pRT commented Jan 12, 2012

p5pRT commented Jan 13, 2012

tainting breakage with index() of a constant #9714

tainting breakage with index() of a constant #9714

Comments

p5pRT commented Apr 17, 2009

p5pRT commented Apr 17, 2009

From @ntyni

p5pRT commented Apr 17, 2009

From @ntyni

p5pRT commented Apr 17, 2009

From @ntyni

p5pRT commented Jun 28, 2011

From @iabyn

p5pRT commented Jun 28, 2011

p5pRT commented Jun 28, 2011

p5pRT commented Jan 12, 2012

From bitcard8493823322@mstier.de

p5pRT commented Jan 13, 2012

From @iabyn