From chris.travers@gmail.com

I tried to make sure this would be put in a place that has not changed between versions. It is a brief description of attacks against the current directory and safe coding strategies.

Feedback is welcome when you have time and bandwidth to give it (not expecting an immediate discussion).

From chris.travers@gmail.com

--- a/perlsec.pod	2016-08-05 06:22:29.062358445 +0200
+++ b/perlsec.pod	2016-08-05 06:21:54.754415959 +0200
@@ -445,6 +445,42 @@
 L<perlunicode> for details, and L<perlunicode/"Security Implications
 of Unicode"> for security implications in particular.
 
+=head2 Code Inclusion from the Current Working Directory
+
+The current directory in the load path are potentially dangerous and
+while taint mode provides some protection there are a number of key
+problems that can crop up.  Particularly when releasing published
+software, programs can be attacked in various ways.  Because of the
+complexity of the dependency tree in many programs, these problems 
+can be very difficult to notice.
+
+=over 4
+
+=item Falling back to current working directory for optional dependencies
+
+Current versions of Perl's C<@INC> contain the last entry as the current working
+directory.  This means that code that requires another module within an eval
+statement will eventually look in the current working directory to see if that
+file exists, but the file isn't required on the system, so if an attacker can
+place an optional dependency of any dependency of your script in the current
+working directory then, aside from Taint mode, your script may load and execute
+the contents of the script.  The safest solution is to use C<no lib '.'> at 
+the head of your Perl programs.
+
+
+=item Code Preload Injection Attacks
+
+If a script has the current working directory at a high priority (for example
+due to the use of C<use lib '.'> or C<-I.>, then if an attacker can place a 
+file with the same name as any dependency (optional or not) in the current 
+working directory or the program, then this file can transparently inject code 
+into the main program by loading and running code into memory, then unloading 
+itself and loading the correct module.  In general, these are somewhat
+dangerous with or without Taint Mode.
+
+=back
+
+
 =head2 Algorithmic Complexity Attacks
 
 Certain internal algorithms used in the implementation of Perl can

From @cpansprout

On Thu Aug 04 21​:26​:04 2016, chris.travers@​gmail.com wrote​:

I tried to make sure this would be put in a place that has not changed
between versions. It is a brief description of attacks against the
current directory and safe coding strategies.

Feedback is welcome when you have time and bandwidth to give it (not
expecting an immediate discussion).

I think this is a good idea. Here are several suggestions for rewording it​:

--- a/perlsec.pod 2016-08-05 06​:22​:29.062358445 +0200
+++ b/perlsec.pod 2016-08-05 06​:21​:54.754415959 +0200
@​@​ -445,6 +445,42 @​@​
L<perlunicode> for details, and L<perlunicode/"Security Implications
of Unicode"> for security implications in particular.

+=head2 Code Inclusion from the Current Working Directory
+
+The current directory in the load path are potentially dangerous and

s/are/is; s/and$/and,/;

+while taint mode provides some protection there are a number of key

s/protection/protection,/;

+problems that can crop up. Particularly when releasing published
+software, programs can be attacked in various ways. Because of the
+complexity of the dependency tree in many programs, these problems
+can be very difficult to notice.
+
+=over 4
+
+=item Falling back to current working directory for optional dependencies
+
+Current versions of Perl's C<@​INC> contain the last entry as the current working
+directory.

contain the current working directory as the last entry

This means that code that requires another module within an eval
+statement

‘eval’ is actually an expression. I would drop the word ‘statement,’ but I’m being pedantic.

will

will, if the required module is not installed, eventually...

eventually look in the current working directory to see if that
+file exists, but the file isn't required on the system, so if an attacker can

file exists. Since the file isn't required on the system, an attacker can

+place an optional dependency of any dependency of your script in the current
+working directory then, aside from Taint mode,

s/aside.*mode/regardless of taint mode/

your script may load and execute
+the contents of the script. The safest solution is to use C<no lib '.'> at

s/the script/the file/

+the head of your Perl programs.
+
+
+=item Code Preload Injection Attacks
+
+If a script has the current working directory at a high priority (for example
+due to the use of C<use lib '.'> or C<-I.>, then if an attacker can place a
+file with the same name as any dependency (optional or not) in the current
+working directory or the program, then this file can transparently inject code

Did you mean ‘of’ by ‘or’?

+into the main program by loading and running code into memory, then unloading
+itself and loading the correct module. In general, these are somewhat
+dangerous with or without Taint Mode.

s/Taint Mode/\L$&/

+
+=back
+
+
=head2 Algorithmic Complexity Attacks

Certain internal algorithms used in the implementation of Perl can

--

Father Chrysostomos

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

From @xsawyerx

On 08/05/2016 07​:25 AM, Father Chrysostomos via RT wrote​:

On Thu Aug 04 21​:26​:04 2016, chris.travers@​gmail.com wrote​:

I tried to make sure this would be put in a place that has not changed
between versions. It is a brief description of attacks against the
current directory and safe coding strategies.

Feedback is welcome when you have time and bandwidth to give it (not
expecting an immediate discussion).
I think this is a good idea. Here are several suggestions for rewording it​:

+1 for the documentation patch and for the rewording suggestions.

From @xsawyerx

On Fri Aug 05 03​:05​:00 2016, xsawyerx@​gmail.com wrote​:

+1 for the documentation patch and for the rewording suggestions.

I replied before finishing my thoughts.

First of all, there seems to be an agreement and push to remove '.' from @​INC's last entry in the next version, which would make this patch (except for back-porting) incorrect. It should read that the current version of perl does not have it, while previous versions do. We could make this change now, or wait for that patch to be introduced and then alter the text again. This also means we should probably mention there were steps taken in core programs and modules to mitigate this by removing it for their use.

I'm not that big on suggesting "no lib '.'" to prevent this. 1. This will no longer be part of perl coming 5.26, but also, since @​INC entries can be strings and subroutines, this really does not cover all cases ("./.", "./", ".//", "././" etc.) and can give a false sense of security in both this respect and in other ways to make sure of optional module load paths attack vector and other possible attack vectors relating to this.

What I +1 here is making a note about the security considerations of optional module loading *with* relative paths as a possible attack vector, and that this is why it has been removed since perl 5.26.

From @xsawyerx

On Fri Aug 05 03​:17​:09 2016, xsawyerx@​cpan.org wrote​:

[...]
optional module loading *with* relative paths as a possible attack

That's a misnomer, sorry. "relative paths" is what I meant.

From chris.travers@gmail.com

On Fri, Aug 5, 2016 at 12​:17 PM, Sawyer X via RT <perlbug-followup@​perl.org>
wrote​:

On Fri Aug 05 03​:05​:00 2016, xsawyerx@​gmail.com wrote​:

+1 for the documentation patch and for the rewording suggestions.

I replied before finishing my thoughts.

First of all, there seems to be an agreement and push to remove '.' from
@​INC's last entry in the next version, which would make this patch (except
for back-porting) incorrect. It should read that the current version of
perl does not have it, while previous versions do. We could make this
change now, or wait for that patch to be introduced and then alter the text
again. This also means we should probably mention there were steps taken in
core programs and modules to mitigate this by removing it for their use.

I'm not that big on suggesting "no lib '.'" to prevent this. 1. This will
no longer be part of perl coming 5.26, but also, since @​INC entries can be
strings and subroutines, this really does not cover all cases ("./.", "./",
".//", "././" etc.) and can give a false sense of security in both this
respect and in other ways to make sure of optional module load paths attack
vector and other possible attack vectors relating to this.

As I have had more time to think about what is appropriate for *this* piece
of documentation I agree.

The problem as I see it is a bit different though. I think the issue is
that the no lib '.' suggestion breaks a common command line switch people
use and that is appropriate only when people are willing to do that.
Discussing the tradeoff there is probably outside the scope of the perlsec
document. I am happy to recommend it personally but the medium does matter
and this is not a medium where one can responsibly discuss this I think.

What I +1 here is making a note about the security considerations of
optional module loading *with* relative paths as a possible attack vector,
and that this is why it has been removed since perl 5.26.

Agreed.

I will supply an updated patch with these suggestions addressed this weekend

--
Best Wishes,
Chris Travers

Efficito​: Hosted Accounting and ERP. Robust and Flexible. No vendor
lock-in.
http​://www.efficito.com/learn_more

From @khwilliamson

On Sun, 07 Aug 2016 21​:47​:53 -0700, chris.travers@​gmail.com wrote​:

On Fri, Aug 5, 2016 at 12​:17 PM, Sawyer X via RT <perlbug-
followup@​perl.org>
wrote​:

On Fri Aug 05 03​:05​:00 2016, xsawyerx@​gmail.com wrote​:

+1 for the documentation patch and for the rewording suggestions.

I replied before finishing my thoughts.

First of all, there seems to be an agreement and push to remove '.'
from
@​INC's last entry in the next version, which would make this patch
(except
for back-porting) incorrect. It should read that the current version
of
perl does not have it, while previous versions do. We could make this
change now, or wait for that patch to be introduced and then alter
the text
again. This also means we should probably mention there were steps
taken in
core programs and modules to mitigate this by removing it for their
use.

I'm not that big on suggesting "no lib '.'" to prevent this. 1. This
will
no longer be part of perl coming 5.26, but also, since @​INC entries
can be
strings and subroutines, this really does not cover all cases ("./.",
"./",
".//", "././" etc.) and can give a false sense of security in both
this
respect and in other ways to make sure of optional module load paths
attack
vector and other possible attack vectors relating to this.

As I have had more time to think about what is appropriate for *this*
piece
of documentation I agree.

The problem as I see it is a bit different though. I think the issue
is
that the no lib '.' suggestion breaks a common command line switch
people
use and that is appropriate only when people are willing to do that.
Discussing the tradeoff there is probably outside the scope of the
perlsec
document. I am happy to recommend it personally but the medium does
matter
and this is not a medium where one can responsibly discuss this I
think.

What I +1 here is making a note about the security considerations of
optional module loading *with* relative paths as a possible attack
vector,
and that this is why it has been removed since perl 5.26.

Agreed.

I will supply an updated patch with these suggestions addressed this
weekend

Would it be possible for you to supply this patch in the next week. That is our final deadline before the next official release, and it would be good to get this in.

--
Karl Williamson

From @jkeenan

On Sat, 11 Feb 2017 18​:36​:35 GMT, khw wrote​:

On Sun, 07 Aug 2016 21​:47​:53 -0700, chris.travers@​gmail.com wrote​:

On Fri, Aug 5, 2016 at 12​:17 PM, Sawyer X via RT <perlbug-
followup@​perl.org>
wrote​:

On Fri Aug 05 03​:05​:00 2016, xsawyerx@​gmail.com wrote​:

+1 for the documentation patch and for the rewording suggestions.

I replied before finishing my thoughts.

First of all, there seems to be an agreement and push to remove '.'
from
@​INC's last entry in the next version, which would make this patch
(except
for back-porting) incorrect. It should read that the current
version
of
perl does not have it, while previous versions do. We could make
this
change now, or wait for that patch to be introduced and then alter
the text
again. This also means we should probably mention there were steps
taken in
core programs and modules to mitigate this by removing it for their
use.

I'm not that big on suggesting "no lib '.'" to prevent this. 1.
This
will
no longer be part of perl coming 5.26, but also, since @​INC entries
can be
strings and subroutines, this really does not cover all cases
("./.",
"./",
".//", "././" etc.) and can give a false sense of security in both
this
respect and in other ways to make sure of optional module load
paths
attack
vector and other possible attack vectors relating to this.

As I have had more time to think about what is appropriate for *this*
piece
of documentation I agree.

The problem as I see it is a bit different though. I think the issue
is
that the no lib '.' suggestion breaks a common command line switch
people
use and that is appropriate only when people are willing to do that.
Discussing the tradeoff there is probably outside the scope of the
perlsec
document. I am happy to recommend it personally but the medium does
matter
and this is not a medium where one can responsibly discuss this I
think.

What I +1 here is making a note about the security considerations
of
optional module loading *with* relative paths as a possible attack
vector,
and that this is why it has been removed since perl 5.26.

Agreed.

I will supply an updated patch with these suggestions addressed this
weekend

Would it be possible for you to supply this patch in the next week.
That is our final deadline before the next official release, and it
would be good to get this in.

Since no updated patch was submitted during the balance of the 5.26 development cycle, and since there was additional documentation related to "no-dot-by-default-in-@​INC" subsequent to the discussion in this ticket, I am marking this ticket Rejected. Any new suggestions for documentation updates should be provided in a new RT.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@jkeenan - Status changed from 'open' to 'rejected'