-
Notifications
You must be signed in to change notification settings - Fork 559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CVE-2015-8607] XS File::Spec::canonpath loses taint #15084
Comments
From @xdgIt looks like the XS canonpath shipped with Perl 5.20 (corresponding to I.e. A patch to the PathTools tests to demonstrate this is here: This is a regression from PathTools-3.40. I have spot checked back to 3.12 As most File::Spec functions wash data through canonpath, this means that I believe this warrants a CVE report and will need backported fixes for David -- |
From @iabynOn Wed, Dec 09, 2015 at 12:06:10PM -0800, David Golden wrote:
I think its worth backporting to 5.20 and 5.22, but I don't think it needs Or to put it another way, I see taint as a bit like using clang's address -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @xdgOn Thu, Dec 10, 2015 at 7:26 AM, Dave Mitchell via RT <
Taint is the headline security mechanism in Perl. (See perlsec.) Breaking Even if the risk is small, it needs to be communicated to downstream David -- |
From @rjbs* David Golden <xdg@xdg.me> [2015-12-10T08:31:18]
I think a CVE is appropriate here, much as I loathe dealing with this process. Do we have a proposed fix? My guess is that it won't be a huge deal to fix it, -- |
From @tonycozOn Mon Dec 14 14:57:07 2015, perl.security@rjbs.manxome.org wrote:
Yes, it's simple, attached. Tony |
From @tonycoz0001-perl-126862-ensure-File-Spec-canonpath-preserves-tai.patchFrom b6307f728a4f842a54ea96959e386c7daa92ece1 Mon Sep 17 00:00:00 2001
From: Tony Cook <[email protected]>
Date: Tue, 15 Dec 2015 10:56:54 +1100
Subject: [perl #126862] ensure File::Spec::canonpath() preserves taint
Previously the unix specific XS implementation of canonpath() would
return an untainted path when supplied a tainted path.
For the empty string case, newSVpvs() already sets taint as needed on
its result.
---
dist/PathTools/Cwd.xs | 1 +
dist/PathTools/t/taint.t | 19 ++++++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/dist/PathTools/Cwd.xs b/dist/PathTools/Cwd.xs
index 9d4dcf0..3d018dc 100644
--- a/dist/PathTools/Cwd.xs
+++ b/dist/PathTools/Cwd.xs
@@ -535,6 +535,7 @@ THX_unix_canonpath(pTHX_ SV *path)
*o = 0;
SvPOK_on(retval);
SvCUR_set(retval, o - SvPVX(retval));
+ SvTAINT(retval);
return retval;
}
diff --git a/dist/PathTools/t/taint.t b/dist/PathTools/t/taint.t
index 309b3e5..48f8c5b 100644
--- a/dist/PathTools/t/taint.t
+++ b/dist/PathTools/t/taint.t
@@ -12,7 +12,7 @@ use Test::More;
BEGIN {
plan(
${^TAINT}
- ? (tests => 17)
+ ? (tests => 21)
: (skip_all => "A perl without taint support")
);
}
@@ -34,3 +34,20 @@ foreach my $func (@Functions) {
# Previous versions of Cwd tainted $^O
is !tainted($^O), 1, "\$^O should not be tainted";
+
+{
+ # [perl #126862] canonpath() loses taint
+ my $tainted = substr($ENV{PATH}, 0, 0);
+ # yes, getcwd()'s result should be tainted, and is tested above
+ # but be sure
+ ok tainted(File::Spec->canonpath($tainted . Cwd::getcwd)),
+ "canonpath() keeps taint on non-empty string";
+ ok tainted(File::Spec->canonpath($tainted)),
+ "canonpath() keeps taint on empty string";
+
+ (Cwd::getcwd() =~ /^(.*)/);
+ my $untainted = $1;
+ ok !tainted($untainted), "make sure our untainted value is untainted";
+ ok !tainted(File::Spec->canonpath($untainted)),
+ "canonpath() doesn't add taint to untainted string";
+}
--
2.1.4
|
From @tonycoz0002-bump-the-PathTools-version.patchFrom ca65c886d2b1c0c8557b9ba72cc443a510d25ec5 Mon Sep 17 00:00:00 2001
From: Tony Cook <[email protected]>
Date: Tue, 15 Dec 2015 11:08:32 +1100
Subject: bump the PathTools version
which is duplicated in many files
---
dist/PathTools/Cwd.pm | 2 +-
dist/PathTools/lib/File/Spec.pm | 2 +-
dist/PathTools/lib/File/Spec/AmigaOS.pm | 2 +-
dist/PathTools/lib/File/Spec/Cygwin.pm | 2 +-
dist/PathTools/lib/File/Spec/Epoc.pm | 2 +-
dist/PathTools/lib/File/Spec/Functions.pm | 2 +-
dist/PathTools/lib/File/Spec/Mac.pm | 2 +-
dist/PathTools/lib/File/Spec/OS2.pm | 2 +-
dist/PathTools/lib/File/Spec/Unix.pm | 2 +-
dist/PathTools/lib/File/Spec/VMS.pm | 2 +-
dist/PathTools/lib/File/Spec/Win32.pm | 2 +-
11 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/dist/PathTools/Cwd.pm b/dist/PathTools/Cwd.pm
index 64618f9..0d35806 100644
--- a/dist/PathTools/Cwd.pm
+++ b/dist/PathTools/Cwd.pm
@@ -3,7 +3,7 @@ use strict;
use Exporter;
use vars qw(@ISA @EXPORT @EXPORT_OK $VERSION);
-$VERSION = '3.60';
+$VERSION = '3.61';
my $xs_version = $VERSION;
$VERSION =~ tr/_//d;
diff --git a/dist/PathTools/lib/File/Spec.pm b/dist/PathTools/lib/File/Spec.pm
index f416908..bcbec2d 100644
--- a/dist/PathTools/lib/File/Spec.pm
+++ b/dist/PathTools/lib/File/Spec.pm
@@ -3,7 +3,7 @@ package File::Spec;
use strict;
use vars qw(@ISA $VERSION);
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
my %module = (MacOS => 'Mac',
diff --git a/dist/PathTools/lib/File/Spec/AmigaOS.pm b/dist/PathTools/lib/File/Spec/AmigaOS.pm
index f979f2f..7d02ad5 100644
--- a/dist/PathTools/lib/File/Spec/AmigaOS.pm
+++ b/dist/PathTools/lib/File/Spec/AmigaOS.pm
@@ -4,7 +4,7 @@ use strict;
use vars qw(@ISA $VERSION);
require File::Spec::Unix;
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
@ISA = qw(File::Spec::Unix);
diff --git a/dist/PathTools/lib/File/Spec/Cygwin.pm b/dist/PathTools/lib/File/Spec/Cygwin.pm
index 558a742..9dd176b 100644
--- a/dist/PathTools/lib/File/Spec/Cygwin.pm
+++ b/dist/PathTools/lib/File/Spec/Cygwin.pm
@@ -4,7 +4,7 @@ use strict;
use vars qw(@ISA $VERSION);
require File::Spec::Unix;
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
@ISA = qw(File::Spec::Unix);
diff --git a/dist/PathTools/lib/File/Spec/Epoc.pm b/dist/PathTools/lib/File/Spec/Epoc.pm
index afca637..0c640ed 100644
--- a/dist/PathTools/lib/File/Spec/Epoc.pm
+++ b/dist/PathTools/lib/File/Spec/Epoc.pm
@@ -3,7 +3,7 @@ package File::Spec::Epoc;
use strict;
use vars qw($VERSION @ISA);
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
require File::Spec::Unix;
diff --git a/dist/PathTools/lib/File/Spec/Functions.pm b/dist/PathTools/lib/File/Spec/Functions.pm
index 276ddcf..9badcf2 100644
--- a/dist/PathTools/lib/File/Spec/Functions.pm
+++ b/dist/PathTools/lib/File/Spec/Functions.pm
@@ -5,7 +5,7 @@ use strict;
use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $VERSION);
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
require Exporter;
diff --git a/dist/PathTools/lib/File/Spec/Mac.pm b/dist/PathTools/lib/File/Spec/Mac.pm
index 4da700c..06e73c8 100644
--- a/dist/PathTools/lib/File/Spec/Mac.pm
+++ b/dist/PathTools/lib/File/Spec/Mac.pm
@@ -4,7 +4,7 @@ use strict;
use vars qw(@ISA $VERSION);
require File::Spec::Unix;
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
@ISA = qw(File::Spec::Unix);
diff --git a/dist/PathTools/lib/File/Spec/OS2.pm b/dist/PathTools/lib/File/Spec/OS2.pm
index fad1198..d6d2f48 100644
--- a/dist/PathTools/lib/File/Spec/OS2.pm
+++ b/dist/PathTools/lib/File/Spec/OS2.pm
@@ -4,7 +4,7 @@ use strict;
use vars qw(@ISA $VERSION);
require File::Spec::Unix;
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
@ISA = qw(File::Spec::Unix);
diff --git a/dist/PathTools/lib/File/Spec/Unix.pm b/dist/PathTools/lib/File/Spec/Unix.pm
index 94e4351..5f92112 100644
--- a/dist/PathTools/lib/File/Spec/Unix.pm
+++ b/dist/PathTools/lib/File/Spec/Unix.pm
@@ -3,7 +3,7 @@ package File::Spec::Unix;
use strict;
use vars qw($VERSION);
-$VERSION = '3.60';
+$VERSION = '3.61';
my $xs_version = $VERSION;
$VERSION =~ tr/_//d;
diff --git a/dist/PathTools/lib/File/Spec/VMS.pm b/dist/PathTools/lib/File/Spec/VMS.pm
index b050bf2..ecb239d 100644
--- a/dist/PathTools/lib/File/Spec/VMS.pm
+++ b/dist/PathTools/lib/File/Spec/VMS.pm
@@ -4,7 +4,7 @@ use strict;
use vars qw(@ISA $VERSION);
require File::Spec::Unix;
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
@ISA = qw(File::Spec::Unix);
diff --git a/dist/PathTools/lib/File/Spec/Win32.pm b/dist/PathTools/lib/File/Spec/Win32.pm
index 8839800..447cbf5 100644
--- a/dist/PathTools/lib/File/Spec/Win32.pm
+++ b/dist/PathTools/lib/File/Spec/Win32.pm
@@ -5,7 +5,7 @@ use strict;
use vars qw(@ISA $VERSION);
require File::Spec::Unix;
-$VERSION = '3.60';
+$VERSION = '3.61';
$VERSION =~ tr/_//d;
@ISA = qw(File::Spec::Unix);
--
2.1.4
|
From @rjbs* Tony Cook via RT <perl5-security-report@perl.org> [2015-12-14T19:09:48]
Thanks, I'll begin the CVE and vendor notification process tomorrow, with a -- |
From @rjbs* Tony Cook via RT <perl5-security-report@perl.org> [2015-12-14T19:09:48]
I have assigned this CVE-2015-8607. I will email vendors tomorrow at lunchtime, unless there are objections between Beginning in PathTools 3.47 and/or perl 5.20.0, the File::Spec::canonpath() This defect was found and reported by David Golden of MongoDB, and the -- |
From @rjbsThis ticket has been moved to the public queue, the embargo has expired, and patches have been pushed to the main repository. I have made a new release of PathTools, available any minute now. As PathTools can be updated outside of perl, I am marking this ticket resolved, rather than pending release. -- |
From [Unknown Contact. See original ticket]This ticket has been moved to the public queue, the embargo has expired, and patches have been pushed to the main repository. I have made a new release of PathTools, available any minute now. As PathTools can be updated outside of perl, I am marking this ticket resolved, rather than pending release. -- |
|
@rjbs - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#126862 (status was 'resolved')
Searchable as RT126862$
The text was updated successfully, but these errors were encountered: