From @xdg

While investigating a Mojolicious security issue (
mojolicious/mojo#738 (comment)), some
File​::Spec flaws came to light​:

(1) File​::Spec​::Win32​::canonpath changes "..." to "..\.."

This is apparently a legacy DOS (COMMAND.COM) convention that is no longer
supported by Windows. It also seems like the kind of lost-trivia that
would surprise people who wouldn't think
to defend against an attack that uses it. It would allow an attacker on
Windows to specify "..." in a path, which could be validated incorrectly on
input before subsequent File​::Spec manipulation translates that to "..\.."
leading to an attack.

I propose removing that "..." translation behavior and that we backport
that to Perls in the security window.

(2) File​::Spec's "no_upwards" method is either buggy or poorly documented.
The docs say​:

  Given a list of file names, strip out those that refer to a parent
  directory. (Does not strip symlinks, only '.', '..', and equivalents.)

In fact, it *only* removes "." and ".." and will happily leave
"../../../../../etc/passwd" and the like. I don't know what "and
equivalents" is supposed to mean -- it is only implemented in
File​::Spec​::Unix and thus doesn't even account for other languages notion
of a parent directory.

It appears intended to be used like this​:

  @​dirs = File​::Spec->no_upwards( readdir $dirhandle );

I propose changing the documentation to clarify that only "." and ".." are
removed and give a usage example so people don't think it's a general
purpose tool for avoiding cross-directory attacks. I don't think that
needs to be backported, but if we backport #1, we might as well include
this.

Regards,
David

--
David Golden <xdg@​xdg.me> Twitter/IRC​: @​xdg

From @iabyn

On Tue, Feb 03, 2015 at 08​:11​:52AM -0800, David Golden wrote​:

While investigating a Mojolicious security issue (
mojolicious/mojo#738 (comment)), some
File​::Spec flaws came to light​:

(1) File​::Spec​::Win32​::canonpath changes "..." to "..\.."

This is apparently a legacy DOS (COMMAND.COM) convention that is no longer
supported by Windows. It also seems like the kind of lost-trivia that
would surprise people who wouldn't think
to defend against an attack that uses it. It would allow an attacker on
Windows to specify "..." in a path, which could be validated incorrectly on
input before subsequent File​::Spec manipulation translates that to "..\.."
leading to an attack.

I propose removing that "..." translation behavior and that we backport
that to Perls in the security window.

(2) File​::Spec's "no_upwards" method is either buggy or poorly documented.
The docs say​:

Given a list of file names\, strip out those that refer to a parent
directory\. \(Does not strip symlinks\, only '\.'\, '\.\.'\, and equivalents\.\)

In fact, it *only* removes "." and ".." and will happily leave
"../../../../../etc/passwd" and the like. I don't know what "and
equivalents" is supposed to mean -- it is only implemented in
File​::Spec​::Unix and thus doesn't even account for other languages notion
of a parent directory.

It appears intended to be used like this​:

 @​dirs = File​::Spec\->no\_upwards\( readdir $dirhandle \);

I propose changing the documentation to clarify that only "." and ".." are
removed and give a usage example so people don't think it's a general
purpose tool for avoiding cross-directory attacks. I don't think that
needs to be backported, but if we backport #1, we might as well include
this.

Hmmm, this ticket appears to have been studiously ignored for 2 years!

I personally don't feel qualified to pontificate on win323 issues, but
changing the documentation seems a plausible workaround.

Can you supply the wording?

--
"You may not work around any technical limitations in the software"
  -- Windows Vista license

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

From @xdg

See here​: https://gist.github.com/xdg/0d4840a388db440ea232638fafbd66e9

On Sat, Feb 4, 2017 at 11​:20 AM, Dave Mitchell via RT <
perl5-security-report@​perl.org> wrote​:

On Tue, Feb 03, 2015 at 08​:11​:52AM -0800, David Golden wrote​:

While investigating a Mojolicious security issue (
mojolicious/mojo#738 (comment)), some
File​::Spec flaws came to light​:

(1) File​::Spec​::Win32​::canonpath changes "..." to "..\.."

This is apparently a legacy DOS (COMMAND.COM) convention that is no
longer
supported by Windows. It also seems like the kind of lost-trivia that
would surprise people who wouldn't think
to defend against an attack that uses it. It would allow an attacker on
Windows to specify "..." in a path, which could be validated incorrectly
on
input before subsequent File​::Spec manipulation translates that to
"..\.."
leading to an attack.

I propose removing that "..." translation behavior and that we backport
that to Perls in the security window.

(2) File​::Spec's "no_upwards" method is either buggy or poorly
documented.
The docs say​:

Given a list of file names\, strip out those that refer to a parent
directory\. \(Does not strip symlinks\, only '\.'\, '\.\.'\, and

equivalents.)

In fact, it *only* removes "." and ".." and will happily leave
"../../../../../etc/passwd" and the like. I don't know what "and
equivalents" is supposed to mean -- it is only implemented in
File​::Spec​::Unix and thus doesn't even account for other languages notion
of a parent directory.

It appears intended to be used like this​:

 @​dirs = File​::Spec\->no\_upwards\( readdir $dirhandle \);

I propose changing the documentation to clarify that only "." and ".."
are
removed and give a usage example so people don't think it's a general
purpose tool for avoiding cross-directory attacks. I don't think that
needs to be backported, but if we backport #1, we might as well include
this.

Hmmm, this ticket appears to have been studiously ignored for 2 years!

I personally don't feel qualified to pontificate on win323 issues, but
changing the documentation seems a plausible workaround.

Can you supply the wording?

--
"You may not work around any technical limitations in the software"
-- Windows Vista license

--
David Golden <xdg@​xdg.me> Twitter/IRC/GitHub​: @​xdg

From @iabyn

On Thu, Feb 23, 2017 at 02​:19​:33PM -0500, David Golden wrote​:

See here​: https://gist.github.com/xdg/0d4840a388db440ea232638fafbd66e9

That looks good to me. I see no reason for you not to apply that to blead
now.

Having re-read the ticket more closely, I see that part (1) about '...'
wasn't a documentation issue but rather a suggestion for a change in
behaviour. Again, I'm not a Win32 person, but it seems to me that '...' is
a relatively low-risk issue, so I suggest this ticket is kept open and in
the security queue, for some Win32-ish person to fix after 5.26.0.

On Sat, Feb 4, 2017 at 11​:20 AM, Dave Mitchell via RT <
perl5-security-report@​perl.org> wrote​:

On Tue, Feb 03, 2015 at 08​:11​:52AM -0800, David Golden wrote​:

While investigating a Mojolicious security issue (
mojolicious/mojo#738 (comment)), some
File​::Spec flaws came to light​:

(1) File​::Spec​::Win32​::canonpath changes "..." to "..\.."

This is apparently a legacy DOS (COMMAND.COM) convention that is no
longer
supported by Windows. It also seems like the kind of lost-trivia that
would surprise people who wouldn't think
to defend against an attack that uses it. It would allow an attacker on
Windows to specify "..." in a path, which could be validated incorrectly
on
input before subsequent File​::Spec manipulation translates that to
"..\.."
leading to an attack.

I propose removing that "..." translation behavior and that we backport
that to Perls in the security window.

(2) File​::Spec's "no_upwards" method is either buggy or poorly
documented.
The docs say​:

Given a list of file names\, strip out those that refer to a parent
directory\. \(Does not strip symlinks\, only '\.'\, '\.\.'\, and

equivalents.)

In fact, it *only* removes "." and ".." and will happily leave
"../../../../../etc/passwd" and the like. I don't know what "and
equivalents" is supposed to mean -- it is only implemented in
File​::Spec​::Unix and thus doesn't even account for other languages notion
of a parent directory.

It appears intended to be used like this​:

 @​dirs = File​::Spec\->no\_upwards\( readdir $dirhandle \);

I propose changing the documentation to clarify that only "." and ".."
are
removed and give a usage example so people don't think it's a general
purpose tool for avoiding cross-directory attacks. I don't think that
needs to be backported, but if we backport #1, we might as well include
this.

Hmmm, this ticket appears to have been studiously ignored for 2 years!

I personally don't feel qualified to pontificate on win323 issues, but
changing the documentation seems a plausible workaround.

Can you supply the wording?

--
"You may not work around any technical limitations in the software"
-- Windows Vista license

--
David Golden <xdg@​xdg.me> Twitter/IRC/GitHub​: @​xdg

--
Please note that ash-trays are provided for the use of smokers,
whereas the floor is provided for the use of all patrons.
  -- Bill Royston

From @iabyn

On Mon, Feb 27, 2017 at 12​:59​:41PM +0000, Dave Mitchell wrote​:

On Thu, Feb 23, 2017 at 02​:19​:33PM -0500, David Golden wrote​:

See here​: https://gist.github.com/xdg/0d4840a388db440ea232638fafbd66e9

That looks good to me. I see no reason for you not to apply that to blead
now.

For the record, that's been added as v5.25.10-45-gdaa6050

--
Diplomacy is telling someone to go to hell in such a way that they'll
look forward to the trip

From @tonycoz

On Mon, 27 Feb 2017 05​:00​:15 -0800, davem wrote​:

Having re-read the ticket more closely, I see that part (1) about
'...'
wasn't a documentation issue but rather a suggestion for a change in
behaviour. Again, I'm not a Win32 person, but it seems to me that
'...' is
a relatively low-risk issue, so I suggest this ticket is kept open and
in
the security queue, for some Win32-ish person to fix after 5.26.0.

Maybe the attached.

It doesn't try to preserve the ... behaviour for NetWare and Symbian.

Tony

From @tonycoz

From 5729b13ebe0634f0c9954e4ecb30cf0ec6511597 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Thu, 1 Nov 2018 13:43:17 +1100
Subject: (perl #123724) don't translate ... to ..\.. on Win32

Apparently this was a command.com thing that Win32 doesn't support.
---
 dist/PathTools/lib/File/Spec/Win32.pm | 10 ----------
 dist/PathTools/t/Spec.t               |  4 ++--
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/dist/PathTools/lib/File/Spec/Win32.pm b/dist/PathTools/lib/File/Spec/Win32.pm
index 43f96b892f..b87e529dd1 100644
--- a/dist/PathTools/lib/File/Spec/Win32.pm
+++ b/dist/PathTools/lib/File/Spec/Win32.pm
@@ -407,16 +407,6 @@ sub _canon_cat				# @path -> path
 	       )+			# performance boost -- I do not know why
 	     }{\\}gx;
 
-    # XXX I do not know whether more dots are supported by the OS supporting
-    #     this ... annotation (NetWare or symbian but not MSWin32).
-    #     Then .... could easily become ../../.. etc:
-    # Replace \.\.\. by (\.\.\.+)  and substitute with
-    # { $1 . ".." . "\\.." x (length($2)-2) }gex
-	     				# ... --> ../..
-    $path =~ s{ (\A|\\)			# at begin or after a slash
-    		\.\.\.
-		(?=\\|\z) 		# at end or followed by slash
-	     }{$1..\\..}gx;
     					# xx\yy\..\zz --> xx\zz
     while ( $path =~ s{(?:
 		(?:\A|\\)		# at begin or after a slash
diff --git a/dist/PathTools/t/Spec.t b/dist/PathTools/t/Spec.t
index 76ade747c7..5f6719067a 100644
--- a/dist/PathTools/t/Spec.t
+++ b/dist/PathTools/t/Spec.t
@@ -277,9 +277,9 @@ my @tests = (
 [ "Win32->canonpath('//a/b/../../c')",  '\\\\a\\b\\c'         ],
 [ "Win32->canonpath('//a/b/c/../d')",   '\\\\a\\b\\d'         ],
 [ "Win32->canonpath('//a/b/c/../../d')",'\\\\a\\b\\d'         ],
-[ "Win32->canonpath('//a/b/c/.../d')",  '\\\\a\\b\\d'         ],
+[ "Win32->canonpath('//a/b/c/.../d')",  '\\\\a\\b\\c\\...\\d' ],
 [ "Win32->canonpath('/a/b/c/../../d')", '\\a\\d'              ],
-[ "Win32->canonpath('/a/b/c/.../d')",   '\\a\\d'              ],
+[ "Win32->canonpath('/a/b/c/.../d')",   '\\a\\b\\c\\...\\d'   ],
 [ "Win32->canonpath('\\../temp\\')",    '\\temp'              ],
 [ "Win32->canonpath('\\../')",          '\\'                  ],
 [ "Win32->canonpath('\\..\\')",         '\\'                  ],
-- 
2.14.1.windows.1

From @tonycoz

On Wed, 31 Oct 2018 20​:12​:06 -0700, tonyc wrote​:

On Mon, 27 Feb 2017 05​:00​:15 -0800, davem wrote​:

Having re-read the ticket more closely, I see that part (1) about
'...'
wasn't a documentation issue but rather a suggestion for a change in
behaviour. Again, I'm not a Win32 person, but it seems to me that
'...' is
a relatively low-risk issue, so I suggest this ticket is kept open and
in
the security queue, for some Win32-ish person to fix after 5.26.0.

Maybe the attached.

Applied as 97fc2df.

Tony

@tonycoz - Status changed from 'open' to 'pending release'

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.30.0, this and 160 other issues have been
resolved.

Perl 5.30.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.30.0

If you find that the problem persists, feel free to reopen this ticket.

@khwilliamson - Status changed from 'pending release' to 'resolved'