Cwd::fast_abs_path's untaint should allow for multiline directories #12624

p5pRT · 2012-12-01T16:47:35Z

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

Searchable as RT115962$

p5pRT · 2012-12-01T16:47:36Z

From joel.a.berger@gmail.com

This is a bug report for perl from joel.a.berger@gmail.com,
generated with the help of perlbug 1.39 running under perl 5.16.0.

From f735a80fac061ee49ac9a1d6552de229d6e2cc10 Mon Sep 17 00:00:00 2001
From: Joel Berger <joel.a.berger@gmail.com>
Date: Sat, 1 Dec 2012 10:15:36 -0600
Subject: [PATCH] Cwd::fast_abs_path's untaint should allow for multiline
directories
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.7.9.5"

This is a multi-part message in MIME format.
--------------1.7.9.5
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit

This bug was noticed via Perl-Toolchain-Gang/File-chdir#3 and testing has led to this being the cause.
The problem is worse on some platforms (notably cygwin in this case) when abs_path is implemented by fast_abs_path.
Since File::chdir tests for proper behavior when a directory contains a newline, this bug then breaks File::chdir (one of my favorites and very useful xplatform tool).

Yes this should have tests, but since it will involve creating a directory with a newline, I thought I would do better to leave that to someone with better knowledge than I.

dist/Cwd/Cwd.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

--------------1.7.9.5
Content-Type: text/x-patch; name="0001-Cwd-fast_abs_path-s-untaint-should-allow-for-multili.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Cwd-fast_abs_path-s-untaint-should-allow-for-multili.patch"

Inline Patch

diff --git a/dist/Cwd/Cwd.pm b/dist/Cwd/Cwd.pm
index f772bf4..888c505 100644
--- a/dist/Cwd/Cwd.pm
+++ b/dist/Cwd/Cwd.pm
@@ -624,8 +624,8 @@ sub fast_abs_path {
 
     # Detaint else we'll explode in taint mode.  This is safe because
     # we're not doing anything dangerous with it.
-    ($path) = $path =~ /(.*)/;
-    ($cwd)  = $cwd  =~ /(.*)/;
+    ($path) = $path =~ /(.*)/s;
+    ($cwd)  = $cwd  =~ /(.*)/s;
 
     unless (-e $path) {
  	_croak("$path: No such file or directory");

--------------1.7.9.5--


---

Flags: category=core severity=medium

Site configuration information for perl 5.16.0:

Configured by joel at Tue May 22 11:06:05 CDT 2012.

Summary of my perl5 (revision 5 version 16 subversion 0) configuration:

Platform:
osname=linux, osvers=3.2.0-23-generic, archname=x86_64-linux
uname='linux joel-ubuntu 3.2.0-23-generic #36-ubuntu smp tue apr 10 20:39:51 utc 2012 x86_64 x86_64 x86_64 gnulinux '
config_args='-de -Dprefix=/home/joel/perl5/perlbrew/perls/perl-5.16.0'
hint=recommended, useposix=true, d_sigaction=define
useithreads=undef, usemultiplicity=undef
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=define, use64bitall=define, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2',
cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
ccversion='', gccversion='4.6.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 =' -fstack-protector -L/usr/local/lib'
libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
libs=-lnsl -ldl -lm -lcrypt -lutil -lc
perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
libc=, so=so, useshrplib=false, libperl=libperl.a
gnulibc_version='2.15'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

Locally applied patches:

@INC for perl 5.16.0:
/home/joel/perl5/perlbrew/perls/perl-5.16.0/lib/site_perl/5.16.0/x86_64-linux
/home/joel/perl5/perlbrew/perls/perl-5.16.0/lib/site_perl/5.16.0
/home/joel/perl5/perlbrew/perls/perl-5.16.0/lib/5.16.0/x86_64-linux
/home/joel/perl5/perlbrew/perls/perl-5.16.0/lib/5.16.0
.

Environment for perl 5.16.0:
HOME=/home/joel
LANG=en_US.UTF-8
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/usr/local/heroku/bin:/home/joel/perl5/perlbrew/bin:/home/joel/perl5/perlbrew/perls/perl-5.16.0/bin:/home/joel/.rvm/gems/ruby-1.9.2-p320/bin:/home/joel/.rvm/gems/ruby-1.9.2-p320@global/bin:/home/joel/.rvm/rubies/ruby-1.9.2-p320/bin:/home/joel/.rvm/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/home/joel/bin:/home/joel/.rvm/bin
PERLBREW_BASHRC_VERSION=0.42
PERLBREW_HOME=/home/joel/.perlbrew
PERLBREW_MANPATH=/home/joel/perl5/perlbrew/perls/perl-5.16.0/man
PERLBREW_PATH=/home/joel/perl5/perlbrew/bin:/home/joel/perl5/perlbrew/perls/perl-5.16.0/bin
PERLBREW_PERL=perl-5.16.0
PERLBREW_ROOT=/home/joel/perl5/perlbrew
PERLBREW_VERSION=0.42
PERL_BADLANG (unset)
SHELL=/bin/bash

p5pRT · 2012-12-02T16:58:32Z

From @jkeenan

On Sat Dec 01 08:47:36 2012, joel.a.berger@gmail.com wrote:

This is a bug report for perl from joel.a.berger@gmail.com,
generated with the help of perlbug 1.39 running under perl 5.16.0.

[snip]

This bug was noticed via https://github.com/dagolden/file-
chdir/issues/3 and testing has led to this being the cause.
The problem is worse on some platforms (notably cygwin in this case)
when abs_path is implemented by fast_abs_path.
Since File::chdir tests for proper behavior when a directory contains
a newline, this bug then breaks File::chdir (one of my favorites
and very useful xplatform tool).

Yes this should have tests, but since it will involve creating a
directory with a newline, I thought I would do better to leave that
to someone with better knowledge than I.

Apart from the mechanics of creating a directory with a newline in its
name, what should the test be testing?

p5pRT · 2012-12-02T16:58:32Z

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

p5pRT · 2012-12-02T17:11:43Z

From joel.a.berger@gmail.com

As seen in the bug report on File::chdir (
Perl-Toolchain-Gang/File-chdir#3 (comment)),
simply calling Cwd::fast_abs_path when in such a directory dies. As I say
there, it is even worse since this function is aliased as Cwd::abs_path on
some platforms. Therefore the tests should be that fast_abs_path doesn't
die when inside a directory with a newline in it.

Further, I haven't search the rest of the codebase but I suspect that all
untaint patterns like /(.*)/ are broken without the /s.

Joel

On Sun, Dec 2, 2012 at 10:58 AM, James E Keenan via RT <
perlbug-followup@perl.org> wrote:

On Sat Dec 01 08:47:36 2012, joel.a.berger@gmail.com wrote:

This is a bug report for perl from joel.a.berger@gmail.com,
generated with the help of perlbug 1.39 running under perl 5.16.0.

[snip]

This bug was noticed via https://github.com/dagolden/file-
chdir/issues/3 and testing has led to this being the cause.
The problem is worse on some platforms (notably cygwin in this case)
when abs_path is implemented by fast_abs_path.
Since File::chdir tests for proper behavior when a directory contains
a newline, this bug then breaks File::chdir (one of my favorites
and very useful xplatform tool).

Yes this should have tests, but since it will involve creating a
directory with a newline, I thought I would do better to leave that
to someone with better knowledge than I.

Apart from the mechanics of creating a directory with a newline in its
name, what should the test be testing?

p5pRT · 2012-12-03T11:37:52Z

From @epa

Could I suggest that an untaint() builtin would be a good idea?

p5pRT · 2012-12-03T16:55:03Z

From @cpansprout

On Sun Dec 02 09:11:43 2012, joel.a.berger@gmail.com wrote:

As seen in the bug report on File::chdir (
Perl-Toolchain-Gang/File-chdir#3 (comment)),
simply calling Cwd::fast_abs_path when in such a directory dies. As I say
there, it is even worse since this function is aliased as Cwd::abs_path on
some platforms. Therefore the tests should be that fast_abs_path doesn't
die when inside a directory with a newline in it.

Further, I haven't search the rest of the codebase but I suspect that all
untaint patterns like /(.*)/ are broken without the /s.

Such as File::Path:

for ($arg->{cwd}) { /\A(.*)\Z/; $_ = $1 } # untaint

I tried writing a test for your bug using File::Temp, and ran into this
problem, which I have reported at
<https://rt.cpan.org/Ticket/Display.html?id=81665>.

In the end I wrote a test without using File::Temp (commit 52ee8d0)
and applied your patch as 9f28c63. Thank you.

--

Father Chrysostomos

p5pRT · 2012-12-03T16:55:03Z

From [Unknown Contact. See original ticket]

On Sun Dec 02 09:11:43 2012, joel.a.berger@gmail.com wrote:

As seen in the bug report on File::chdir (
Perl-Toolchain-Gang/File-chdir#3 (comment)),
simply calling Cwd::fast_abs_path when in such a directory dies. As I say
there, it is even worse since this function is aliased as Cwd::abs_path on
some platforms. Therefore the tests should be that fast_abs_path doesn't
die when inside a directory with a newline in it.

Further, I haven't search the rest of the codebase but I suspect that all
untaint patterns like /(.*)/ are broken without the /s.

Such as File::Path:

for ($arg->{cwd}) { /\A(.*)\Z/; $_ = $1 } # untaint

I tried writing a test for your bug using File::Temp, and ran into this
problem, which I have reported at
<https://rt.cpan.org/Ticket/Display.html?id=81665>.

In the end I wrote a test without using File::Temp (commit 52ee8d0)
and applied your patch as 9f28c63. Thank you.

--

Father Chrysostomos

p5pRT · 2012-12-03T16:55:03Z

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

p5pRT closed this as completed Dec 3, 2012

p5pRT added Severity Medium distro-All hasPatch type-library labels Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cwd::fast_abs_path's untaint should allow for multiline directories #12624

Cwd::fast_abs_path's untaint should allow for multiline directories #12624

p5pRT commented Dec 1, 2012

p5pRT commented Dec 1, 2012

p5pRT commented Dec 2, 2012

p5pRT commented Dec 2, 2012

p5pRT commented Dec 2, 2012

p5pRT commented Dec 3, 2012

p5pRT commented Dec 3, 2012

p5pRT commented Dec 3, 2012

p5pRT commented Dec 3, 2012

Cwd::fast_abs_path's untaint should allow for multiline directories #12624

Cwd::fast_abs_path's untaint should allow for multiline directories #12624

Comments

p5pRT commented Dec 1, 2012

p5pRT commented Dec 1, 2012

From joel.a.berger@gmail.com

p5pRT commented Dec 2, 2012

From @jkeenan

p5pRT commented Dec 2, 2012

p5pRT commented Dec 2, 2012

From joel.a.berger@gmail.com

p5pRT commented Dec 3, 2012

From @epa

p5pRT commented Dec 3, 2012

From @cpansprout

p5pRT commented Dec 3, 2012

From [Unknown Contact. See original ticket]

p5pRT commented Dec 3, 2012