Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

pack "A*" and pack "a*" untaint data in 5.10.0 #9280

Closed p5pRT closed 15 years ago

p5pRT commented 16 years ago

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

Searchable as RT52552$

p5pRT commented 16 years ago

From cstith@gmail.com

Created by chris@localhost.(none)

The following code leaves $x tainted after the pack() in 5.8.8 and according to perlsec\, but it does not under 5.10.0 for some reason. The "A*" template and the "a*" template both do this.

perl -wTe 'use Scalar​::Util qw( tainted ); $x = $ARGV[0]; print "tainted!\n" if tainted( $x ); $x = pack "a*"\, $x; print "No longer +tainted!\n" unless tainted ($x); eval( $x ); ' 'print "hello\, world\n";'

I think the former behavior is proper. If not\, then the docs need to be updated with the new behavior and a caveat.

I'd like to thank "ambrus" on Perlmonks for noticing something was amiss. I'd like to thank Tye McQueen ("tye" on PM) for suggesting the direct taint testing from Scalar​::Util.

Christopher E. Stith

Perl Info ``` Flags: category=core severity=high Site configuration information for perl 5.10.0: Configured by chris at Thu Mar 13 21:34:04 CDT 2008. Summary of my perl5 (revision 5 version 10 subversion 0) configuration: Platform: osname=linux, osvers=2.6.22.18-desktop-1mdv, archname=i686-linux uname='linux localhost 2.6.22.18-desktop-1mdv #1 smp mon feb 11 13:53:50 est 2008 i686 amd athlon(tm) processor gnulinux ' config_args='-ds -e' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=undef, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-fno-strict-aliasing -pipe -I/usr/local/include' ccversion='', gccversion='4.2.2 20071128 (prerelease) (4.2.2-3.1mdv2008.0)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -ldl -lm -lcrypt -lutil -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=/lib/libc-2.6.1.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.6.1' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib' Locally applied patches: @INC for perl 5.10.0: /usr/local/lib/perl5/5.10.0/i686-linux /usr/local/lib/perl5/5.10.0 /usr/local/lib/perl5/site_perl/5.10.0/i686-linux /usr/local/lib/perl5/site_perl/5.10.0 . Environment for perl 5.10.0: HOME=/home/chris LANG=en_US.UTF-8 LANGUAGE=en_US.UTF-8:en_US:en LC_ADDRESS=en_US.UTF-8 LC_COLLATE=en_US.UTF-8 LC_CTYPE=en_US.UTF-8 LC_IDENTIFICATION=en_US.UTF-8 LC_MEASUREMENT=en_US.UTF-8 LC_MESSAGES=en_US.UTF-8 LC_MONETARY=en_US.UTF-8 LC_NAME=en_US.UTF-8 LC_NUMERIC=en_US.UTF-8 LC_PAPER=en_US.UTF-8 LC_SOURCED=1 LC_TELEPHONE=en_US.UTF-8 LC_TIME=en_US.UTF-8 LD_LIBRARY_PATH=/home/chris/GNUstep/Library/Libraries:/usr/lib LOGDIR (unset) PATH=/home/chris/GNUstep/Tools:/usr/bin:/usr/bin:/bin:/usr/local/bin:/usr/X11R6/bin/:/usr/games:/usr/lib/qt3//bin:/home/chris/bin:/usr/lib/qt3//bin PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 16 years ago

From @ikegami

PerlMonks thread on the topic​: http​://www.perlmonks.org/?node_id=678463

On Mon\, Apr 7\, 2008 at 11​:45 AM\, via RT Chris \perlbug\-followup@​perl\.org wrote​:

# New Ticket Created by Chris # Please include the string​: [perl #52552] # in the subject line of all future correspondence about this issue. # \<URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=52552 >

To​: perlbug@​perl.org Subject​: pack "a*" and pack "A*" untaint data in 5.10.0 Reply-To​: chris@​localhost.(none) Message-Id​: \5\.10\.0\_3444\_1207582753@&#8203;localhost

This is a bug report for perl from chris@​localhost.(none)\, generated with the help of perlbug 1.36 running under perl 5.10.0.

----------------------------------------------------------------- [Please enter your report here]

The following code leaves $x tainted after the pack() in 5.8.8 and according to perlsec\, but it does not under 5.10.0 for some reason. The "A*" template and the "a*" template both do this.

perl -wTe 'use Scalar​::Util qw( tainted ); $x = $ARGV[0]; print "tainted!\n" if tainted( $x ); $x = pack "a*"\, $x; print "No longer +tainted!\n" unless tainted ($x); eval( $x ); ' 'print "hello\, world\n";'

I think the former behavior is proper. If not\, then the docs need to be updated with the new behavior and a caveat.

I'd like to thank "ambrus" on Perlmonks for noticing something was amiss. I'd like to thank Tye McQueen ("tye" on PM) for suggesting the direct taint testing from Scalar​::Util.

Christopher E. Stith

p5pRT commented 16 years ago

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

p5pRT commented 16 years ago

From @andk

On Tue\, 8 Apr 2008 04​:04​:21 -0400\, "Eric Brine" \ikegami@&#8203;adaelis\.com said​:

  > PerlMonks thread on the topic​:   > http​://www.perlmonks.org/?node_id=678463

And I just added my usual binary search results to the perlmonks thread. 24010 is the patch to "blame".

-- andreas PS​: hey binsect users\, where are you\, this was a challenge​:P

p5pRT commented 16 years ago

From @nwc10

On Tue\, Apr 08\, 2008 at 09​:34​:22PM +0200\, Andreas J. Koenig wrote​:

On Tue\, 8 Apr 2008 04​:04​:21 -0400\, "Eric Brine" \ikegami@&#8203;adaelis\.com said​:

PerlMonks thread on the topic​: http​://www.perlmonks.org/?node_id=678463

And I just added my usual binary search results to the perlmonks thread. 24010 is the patch to "blame".

Change 24010 by rgs@​bloom on 2005/03/08 17​:53​:50

  Subject​: Encoding neutral unpack   From​: perl5-porters@​ton.iguana.be (Ton Hospel)   Date​: Sun\, 6 Mar 2005 18​:29​:38 +0000 (UTC)   Message-Id​: \d0fi6i$k06$1@&#8203;post\.home\.lunix

Affected files ...

... //depot/perl/embed.fnc#146 edit ... //depot/perl/embed.h#454 edit ... //depot/perl/genpacksizetables.pl#6 edit ... //depot/perl/lib/charnames.t#28 edit ... //depot/perl/perl.h#568 edit ... //depot/perl/pod/perldiag.pod#394 edit ... //depot/perl/pod/perlfunc.pod#455 edit ... //depot/perl/pod/perlunicode.pod#135 edit ... //depot/perl/pod/perluniintro.pod#62 edit ... //depot/perl/pp_pack.c#72 edit ... //depot/perl/proto.h#493 edit ... //depot/perl/t/op/pack.t#103 edit ... //depot/perl/t/op/utftaint.t#3 edit

mmm. Not a small change.

http​://public.activestate.com/cgi-bin/perlbrowse?patch_num=24010&show_patch=Show+Patch

Nicholas Clark

p5pRT commented 15 years ago

From @nwc10

Dave notes​:

both bleed and maint still have a taint bug not present in 5.8.8 (confirmed still broken 23/4/09)

p5pRT commented 15 years ago

From @rgs

Solved by :

commit 3c4fb04a912b266806354630dd98a7e36a830fbe Author​: Robin Barker \Robin\.Barker@&#8203;npl\.co\.uk Date​: Tue Jun 23 14​:51​:45 2009 +0200

  Fix for RT #52552.  
  This patch only taints for pack('a'/'A') which was the original bug. I   guess the previous behaviour (pre-5.10.0) tainted on all tainted input.   That more general behaviour may be recoverable - not sure what we want.

pp_pack.c | 1 + t/op/taint.t | 15 ++++++++++++++- 2 files changed\, 15 insertions(+)\, 1 deletions(-)

p5pRT commented 15 years ago

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

p5pRT commented 15 years ago

From p5p@spam.wizbit.be

Also see http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2009-06/ msg00689.html

' From​: Dave Mitchell On Mon\, Jun 22\, 2009 at 10​:52​:26PM +0100\, Robin Barker wrote​:

Fix for RT #52552.

This patch only taints for pack('a'/'A') which was the original bug.
I guess the previous behaviour (pre-5.10.0) tainted on all tainted input. That more general behaviour may be recoverable - not sure what we want.

yes\, I think if $expr is tainted\, then pack('...'\, $expr) should be tainted too\, for all (or most) '...'. This seems to be the 5.8.x behaviour\, lost in 5.10.0 and blead/maint. '