Flaws in Perl code due to unsafe module load path

[p5pRT]

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

Searchable as RT127834$

[p5pRT]

From @lightsey

Hi there\,

I need some guidance from the Perl and Debian security teams on how to handle vulnerabilities related to Perl's inclusion of the current working directory in its default module load path.

Todd raised this issue on the p5p list in 2012 when he was updating cPanel to Perl 5.14. The cPanel Security Team (my day job) took note of the problem late last year while we were auditing for local file read and write attacks. The problem was pervasive in cPanel's codebase and we resolved the issues with the assumption that this was a well established Perl behavior and cPanel's code was responsible for mitigating the risks. This work led to the discovery of several other interesting flaws in open source code and we've been working with the upstream authors to get those resolved.

I started putting together a conference talk to summarize all of the work cPanel had put into this effort\, the flaws we found and the tools we used. At that same time\, Todd was putting together a patch submission to revisit the 2012 decision to leave the '.' in @​INC in Perl. The more we've looked at this problem though\, the more convinced we've become that this needs to be handled and resolved as a vulnerability in Perl itself.

The major problem with this behavior is that it unexpectedly puts a user at risk whenever they execute any Perl scripts from a directory that is writable by other accounts on the system. For instance\, if I'm logged in as root and I change directory into /tmp or an account's home directory\, I can run any shell commands that are written in C\, Python or Ruby without fear. The same isn't true for any shell commands that are written in Perl\, since a significant proportion of Perl scripts will execute code in the current working directory whenever they are run.

For example\, if a user on a shared system creates the file /tmp/Pod/Perldoc/Toterm.pm\, and then I log in as root\, change directory to /tmp\, and run "perldoc perlrun"\, it will execute the code they have placed in the file.

The most severe example I've found on Debian is that apt-get will load and execute the /tmp/Log/Agent.pm file regardless of the directory it is started from since it automatically changes directory to /tmp. To reproduce this​:

mkdir -p /tmp/Log echo '`wall hello`;die;' >/tmp/Log/Agent.pm sudo apt-get update

A very simplistic analysis of my personal Debian laptop shows that over 40% of the perl scripts have this behavior. You can run the same analysis with this ugly shell command​:

grep '^#!/usr/bin/perl' /usr/*bin/* | cut -d '​:' -f 1 | xargs -n1 /bin/sh -c 'TEST="$0"; if [ "`strace -f perl -c \"$TEST\" 2>&1 | grep '\''stat(\"\..*\.pm\"'\''`" != "" ] ; then echo "Vulnerable​: $TEST"; else echo "Not vulnerable​: $TEST"; fi'

Or to test an individual script you can do something like this​:

strace -f /usr/bin/perldoc perldoc 2>&1 | grep 'stat("\..*\.pm"'

This kind of analysis definitely underestimates the number of affected scripts though.

So...with all that explanation said... My question is\, which codebase is supposed to fix this issue? I figure there are four possible answers to which piece of code was responsible for preventing these types of attacks​:

1) The Perl interpreter for placing the '.' in @​INC.

2) Any module that fails to remove the '.' from @​INC before attempting to load other modules that are not specified as hard requirements.

3) Any script that loads any modules that end up exhibiting this behavior.

4) End users must avoid running Perl scripts in directories that other users can modify. The only vulnerable code is code that moves to an unsafe location on its own.

At cPanel we started off with approach #3\, then ended up leaning towards #1. I can probably provide a fairly good list of places that need to be fixed in Debian once I know exactly where to focus.

I haven't contacted Debian's Perl team about this problem. Please CC whoever should be informed from that team if they are not already on the perl-security or debian-security lists.

[p5pRT]

From @fweimer

* John Lightsey​:

So...with all that explanation said... My question is\, which codebase is supposed to fix this issue? I figure there are four possible answers to which piece of code was responsible for preventing these types of attacks​:

1) The Perl interpreter for placing the '.' in @​INC.

'.' in @​INC is certainly handy for development\, so that you can access modules from the same directory as your script.

Maybe it could make sense to extract the full path to the executable and use that?

This would still put at risk applications which write temporary scripts to locations such as /tmp and /var/tmp\, but it would cover more common scenarios.

[p5pRT]

From @demerphq

Maybe we should handle this with a command line operation.

So

perl -Q script.pl

would not add ".".

People that want to write scripts that dont use the cwd could add the -Q to the shebang line.

I hate to make security opt-in tho.

Yves

On 5 April 2016 at 05​:23\, John Lightsey \perl5\-security\-report@​perl\.org wrote​:

# New Ticket Created by John Lightsey # Please include the string​: [perl #127834] # in the subject line of all future correspondence about this issue. # \<URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127834 >

Hi there\,

I need some guidance from the Perl and Debian security teams on how to handle vulnerabilities related to Perl's inclusion of the current working directory in its default module load path.

Todd raised this issue on the p5p list in 2012 when he was updating cPanel to Perl 5.14. The cPanel Security Team (my day job) took note of the problem late last year while we were auditing for local file read and write attacks. The problem was pervasive in cPanel's codebase and we resolved the issues with the assumption that this was a well established Perl behavior and cPanel's code was responsible for mitigating the risks. This work led to the discovery of several other interesting flaws in open source code and we've been working with the upstream authors to get those resolved.

I started putting together a conference talk to summarize all of the work cPanel had put into this effort\, the flaws we found and the tools we used. At that same time\, Todd was putting together a patch submission to revisit the 2012 decision to leave the '.' in @​INC in Perl. The more we've looked at this problem though\, the more convinced we've become that this needs to be handled and resolved as a vulnerability in Perl itself.

The major problem with this behavior is that it unexpectedly puts a user at risk whenever they execute any Perl scripts from a directory that is writable by other accounts on the system. For instance\, if I'm logged in as root and I change directory into /tmp or an account's home directory\, I can run any shell commands that are written in C\, Python or Ruby without fear. The same isn't true for any shell commands that are written in Perl\, since a significant proportion of Perl scripts will execute code in the current working directory whenever they are run.

For example\, if a user on a shared system creates the file /tmp/Pod/Perldoc/Toterm.pm\, and then I log in as root\, change directory to /tmp\, and run "perldoc perlrun"\, it will execute the code they have placed in the file.

The most severe example I've found on Debian is that apt-get will load and execute the /tmp/Log/Agent.pm file regardless of the directory it is started from since it automatically changes directory to /tmp. To reproduce this​:

mkdir -p /tmp/Log echo '`wall hello`;die;' >/tmp/Log/Agent.pm sudo apt-get update

A very simplistic analysis of my personal Debian laptop shows that over 40% of the perl scripts have this behavior. You can run the same analysis with this ugly shell command​:

grep '^#!/usr/bin/perl' /usr/*bin/* | cut -d '​:' -f 1 | xargs -n1 /bin/sh -c 'TEST="$0"; if [ "`strace -f perl -c \"$TEST\" 2>&1 | grep '\''stat(\"\..*\.pm\"'\''`" != "" ] ; then echo "Vulnerable​: $TEST"; else echo "Not vulnerable​: $TEST"; fi'

Or to test an individual script you can do something like this​:

strace -f /usr/bin/perldoc perldoc 2>&1 | grep 'stat("\..*\.pm"'

This kind of analysis definitely underestimates the number of affected scripts though.

So...with all that explanation said... My question is\, which codebase is supposed to fix this issue? I figure there are four possible answers to which piece of code was responsible for preventing these types of attacks​:

1) The Perl interpreter for placing the '.' in @​INC.

2) Any module that fails to remove the '.' from @​INC before attempting to load other modules that are not specified as hard requirements.

3) Any script that loads any modules that end up exhibiting this behavior.

4) End users must avoid running Perl scripts in directories that other users can modify. The only vulnerable code is code that moves to an unsafe location on its own.

At cPanel we started off with approach #3\, then ended up leaning towards #1. I can probably provide a fairly good list of places that need to be fixed in Debian once I know exactly where to focus.

I haven't contacted Debian's Perl team about this problem. Please CC whoever should be informed from that team if they are not already on the perl-security or debian-security lists.

-- perl -Mre=debug -e "/just|another|perl|hacker/"

[p5pRT]

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

[p5pRT]

From carnil@debian.org

Hi\,

On Tue\, Apr 05\, 2016 at 08​:59​:46AM +0200\, Florian Weimer wrote​:

* John Lightsey​:

So...with all that explanation said... My question is\, which codebase is supposed to fix this issue? I figure there are four possible answers to which piece of code was responsible for preventing these types of attacks​:

1) The Perl interpreter for placing the '.' in @​INC.

'.' in @​INC is certainly handy for development\, so that you can access modules from the same directory as your script.

Maybe it could make sense to extract the full path to the executable and use that?

One addition/context​: Just recently #588017 got activity​: https://bugs.debian.org/588017#76, which refer/leads to https://rt.perl.org/Public/Bug/Display.html?id=127810

Regards\, Salvatore

[p5pRT]

From @iabyn

On Mon\, Apr 04\, 2016 at 08​:23​:27PM -0700\, John Lightsey wrote​:

I need some guidance from the Perl and Debian security teams on how to handle vulnerabilities related to Perl's inclusion of the current working directory in its default module load path.

My personal feeling is that perl should *not* include '.' in @​INC by default\, but that a Configure option (i.e. effectively the inverse of todd's proposed -Dfortify inc) whould make a perl with the old behaviour restored.

I think we are in the same situation where UNIXy OSes gradually stopped making '.' be included by default in $PATH. It probably annoyed some people and broke some things\, but the workarounds were trivial and people soon got used to it.

In the same way\, if we change the default\, the good thing is that it's usually pretty obvious that there's a fault - the module fails to load and the script dies. Fixing is trivial\, e.g. '-I.' or 'unshift @​INC\, ".".

The converse is not true - its not easy to spot that a script isn't secure. Any script that optionally loads a module\, e.g. tries an XS version and falls back to a pure-perl version can be hijacked if the optional module isn't available on that systemm and if the script is run from a directory that the attacker has write access to (such as tmp).

Very few people writing a boring everyday admin script are going to be aware of such a subtlety and make their scripts immune to it.

What I would like to see is​:

1) Todd's changes to cpan/ modules to make them dot-agnostic get pushed as tickets to the relevant distributions ASAP; 2) early in the 5.25.x development cycle\, once those distributions have been updated\, released and pulled into bleed\, that we switch @​INC to be no-dot by default 3) If it all turns into a disaster\, we can always back out before 5.26 is released.

-- "You're so sadly neglected\, and often ignored. A poor second to Belgium\, When going abroad."   -- Monty Python\, "Finland"

[p5pRT]

From @rgarcia

On 5 April 2016 at 10​:07\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

On Mon\, Apr 04\, 2016 at 08​:23​:27PM -0700\, John Lightsey wrote​:

I need some guidance from the Perl and Debian security teams on how to handle vulnerabilities related to Perl's inclusion of the current working directory in its default module load path.

My personal feeling is that perl should *not* include '.' in @​INC by default\, but that a Configure option (i.e. effectively the inverse of todd's proposed -Dfortify inc) whould make a perl with the old behaviour restored.

I think we are in the same situation where UNIXy OSes gradually stopped making '.' be included by default in $PATH. It probably annoyed some people and broke some things\, but the workarounds were trivial and people soon got used to it.

In the same way\, if we change the default\, the good thing is that it's usually pretty obvious that there's a fault - the module fails to load and the script dies. Fixing is trivial\, e.g. '-I.' or 'unshift @​INC\, ".".

The converse is not true - its not easy to spot that a script isn't secure. Any script that optionally loads a module\, e.g. tries an XS version and falls back to a pure-perl version can be hijacked if the optional module isn't available on that systemm and if the script is run from a directory that the attacker has write access to (such as tmp).

Very few people writing a boring everyday admin script are going to be aware of such a subtlety and make their scripts immune to it.

What I would like to see is​:

1) Todd's changes to cpan/ modules to make them dot-agnostic get pushed as tickets to the relevant distributions ASAP; 2) early in the 5.25.x development cycle\, once those distributions have been updated\, released and pulled into bleed\, that we switch @​INC to be no-dot by default 3) If it all turns into a disaster\, we can always back out before 5.26 is released.

I'd like to add that -T also removes . from @​INC\, so anything that would break without . would also break with -T. One more argument to simply ditch . from @​INC.

(I was under the impression that running perl as root was also removing . but it seems I'm wrong)

[p5pRT]

From @timbunce

On Tue\, Apr 05\, 2016 at 09​:07​:24AM +0100\, Dave Mitchell wrote​:

What I would like to see is​:

1) Todd's changes to cpan/ modules to make them dot-agnostic get pushed as tickets to the relevant distributions ASAP; 2) early in the 5.25.x development cycle\, once those distributions have been updated\, released and pulled into bleed\, that we switch @​INC to be no-dot by default 3) If it all turns into a disaster\, we can always back out before 5.26 is released.

+1

Tim.

[p5pRT]

From @jhi

On Tuesday-201604-05 4​:48\, Tim Bunce wrote​:

On Tue\, Apr 05\, 2016 at 09​:07​:24AM +0100\, Dave Mitchell wrote​:

What I would like to see is​:

1) Todd's changes to cpan/ modules to make them dot-agnostic get pushed as tickets to the relevant distributions ASAP; 2) early in the 5.25.x development cycle\, once those distributions have been updated\, released and pulled into bleed\, that we switch @​INC to be no-dot by default 3) If it all turns into a disaster\, we can always back out before 5.26 is released.

+1

Tim.

+1

[p5pRT]

From @lightsey

On Tue\, 2016-04-05 at 09​:37 +0200\, Salvatore Bonaccorso wrote​:

Hi\,

One addition/context​: Just recently #588017 got activity​: https://bugs.debian.org/588017#76, which refer/leads to https://rt.perl.org/Public/Bug/Display.html?id=127810

Right\, that bug report basically shows the full public lifespan of the issue. Debian raised it on p5p in 2010\, Todd raised it again on p5p in 2012\, and Todd submitted a patch for the issue recently. There are also several bodies of code that more or less inherited this behavior from Perl and have already been publicly fixed. The ones I'm aware of are Ruby\, Perl6\, Module​::Signature\, EXIM\, and cPanel.

Some of the responses to my email weren't sent to all of the parties I originally CC'd. The summary of the missing bits is that the perl- security team would like to switch Todd's patch from defaulting off to defaulting on\, and get it incorporated into Perl 5.26 if possible.

I agree that its a great idea going forward\, but it still leaves my original question unanswered.

If fixing Perl 5.26+ is the only solution\, it will result in a very long window where Perl scripts remain vulnerable to these problems. For some scripts this is a relatively obscure risk\, but for others it's a meaningful risk that should be addressed with a more direct solution.

For instance\, the behavior in the apt-get tools is a local root exploit without any unusual preconditions. Taking the apt-get scripts in isolation from the rest of the Perl ecosystem\, the vulnerability is trivial to fix at all levels (the Perl interpreter\, the Storable and Encode modules\, the various apt-get Perl helper scripts.)

I would expect that leaving this vulnerability in place for several years while Debian slowly obsoletes all versions of Perl before 5.26 isn't going to be a reasonable approach. The likelihood of someone else noticing the same attacks during that timeframe seems too high to me.

It's also difficult to keep fixing flaws like this in isolation without inadvertently shining a brighter spotlight on the underlying risk to all existing Perl scripts.

It would be very helpful to know which of the various pieces of code are responsible for addressing the risk with Perl before 5.26? The interpreter\, the modules\, or the script?

[p5pRT]

From zefram@fysh.org

John Lightsey wrote​:

It would be very helpful to know which of the various pieces of code are responsible for addressing the risk with Perl before 5.26? The interpreter\, the modules\, or the script?

Patching older interpreters would change their behaviour in a breaking way\, so isn't a great idea. It's also infeasible for a lot of people; we'd like to have a solution that people who don't build their own perl can apply.

Modules are the wrong place to do this. It is not for a module to question the @​INC that it sees; even if it were\, there's no clear criterion for which modules ought to disrespect a "." entry. Generally\, modules are not aware of the security requirements and other situational aspects of the running program.

That leaves the script\, i.e.\, the top-level program. This is the right place. The top level knows what the overall purpose of running the program is\, knows what aspects of the environment can be trusted\, and what kind of security requirements can apply. It also has the capability to control the @​INC that will apply to almost all module loading that occurs as part of the program's compilation and execution. (It doesn't control PERL5OPT=-MFoo type stuff\, but that's much less of a security worry.)

We should recommend that (all?) perl programs begin with an invocation such as

  BEGIN { pop @​INC if $INC[-1] eq "."; }

This invocation is portable back to very old perls\, and will be safe to retain on future perls that don't put "." into @​INC.

-zefram

[p5pRT]

From @lightsey

On Wed\, 2016-04-06 at 10​:33 -0700\, Zefram via RT wrote​:

John Lightsey wrote​:

It would be very helpful to know which of the various pieces of code are responsible for addressing the risk with Perl before 5.26? The interpreter\, the modules\, or the script?

Patching older interpreters would change their behaviour in a breaking way\, so isn't a great idea.  It's also infeasible for a lot of people; we'd like to have a solution that people who don't build their own perl can apply.

Modules are the wrong place to do this.  It is not for a module to question the @​INC that it sees; even if it were\, there's no clear criterion for which modules ought to disrespect a "." entry.  Generally\, modules are not aware of the security requirements and other situational aspects of the running program.

That leaves the script\, i.e.\, the top-level program.  This is the right place.  The top level knows what the overall purpose of running the program is\, knows what aspects of the environment can be trusted\, and what kind of security requirements can apply.  It also has the capability to control the @​INC that will apply to almost all module loading that occurs as part of the program's compilation and execution. (It doesn't control PERL5OPT=-MFoo type stuff\, but that's much less of a security worry.)

One compelling reason to avoid this approach is just the numbers. There are a small handful of common modules that trigger this behavior over and over in most scripts.

For example\, by default my Debian box has 136 perl scripts that do a module load from the current working directory under a simple perl -c test.

If I fix the two most common modules that cause this (Storable loading Log​::Agent and Encode loading Encode​::ConfigLocal) by localizing and filtering @​INC before they load these optional dependencies\, the number of vulnerable scripts drops to 12.

It's much simpler to change 2 modules than 124 scripts as a security fix. The behavior of Storable and Encode does change as a result\, but not in a fashion anyone is likely dependent on\, and technically the scripts aren't guaranteed to be "safe". It's a simple way to get a lot of coverage with a very small set of changes though.

[p5pRT]

From @tonycoz

I've merged the extra tickets created into the main ticket.

Please make sure [perl #127834] is in the subject when you reply to this ticket.

Tony

[p5pRT]

From @tonycoz

On Tue Apr 05 01​:07​:50 2016\, davem wrote​:

1) Todd's changes to cpan/ modules to make them dot-agnostic get pushed as tickets to the relevant distributions ASAP; 2) early in the 5.25.x development cycle\, once those distributions have been updated\, released and pulled into bleed\, that we switch @​INC to be no-dot by default 3) If it all turns into a disaster\, we can always back out before 5.26 is released.

This.

I don't think Todd's PERL_USE_UNSAFE_INC environment variable should be included - if someone wants . in @​INC they can set PERL5LIB\, or add -I. to the #! line. The behaviour isn't exactly the same\, but I expect it's close enough for most of the cases that expect . in @​INC.

Tony

[p5pRT]

From @Leont

On Tue\, Apr 5\, 2016 at 5​:23 AM\, John Lightsey \< perl5-security-report@​perl.org> wrote​:

Hi there\,

I need some guidance from the Perl and Debian security teams on how to handle vulnerabilities related to Perl's inclusion of the current working directory in its default module load path.

Todd raised this issue on the p5p list in 2012 when he was updating cPanel to Perl 5.14. The cPanel Security Team (my day job) took note of the problem late last year while we were auditing for local file read and write attacks. The problem was pervasive in cPanel's codebase and we resolved the issues with the assumption that this was a well established Perl behavior and cPanel's code was responsible for mitigating the risks. This work led to the discovery of several other interesting flaws in open source code and we've been working with the upstream authors to get those resolved.

I started putting together a conference talk to summarize all of the work cPanel had put into this effort\, the flaws we found and the tools we used. At that same time\, Todd was putting together a patch submission to revisit the 2012 decision to leave the '.' in @​INC in Perl. The more we've looked at this problem though\, the more convinced we've become that this needs to be handled and resolved as a vulnerability in Perl itself.

The major problem with this behavior is that it unexpectedly puts a user at risk whenever they execute any Perl scripts from a directory that is writable by other accounts on the system. For instance\, if I'm logged in as root and I change directory into /tmp or an account's home directory\, I can run any shell commands that are written in C\, Python or Ruby without fear. The same isn't true for any shell commands that are written in Perl\, since a significant proportion of Perl scripts will execute code in the current working directory whenever they are run.

For example\, if a user on a shared system creates the file /tmp/Pod/Perldoc/Toterm.pm\, and then I log in as root\, change directory to /tmp\, and run "perldoc perlrun"\, it will execute the code they have placed in the file.

The most severe example I've found on Debian is that apt-get will load and execute the /tmp/Log/Agent.pm file regardless of the directory it is started from since it automatically changes directory to /tmp. To reproduce this​:

mkdir -p /tmp/Log echo '`wall hello`;die;' >/tmp/Log/Agent.pm sudo apt-get update

A very simplistic analysis of my personal Debian laptop shows that over 40% of the perl scripts have this behavior. You can run the same analysis with this ugly shell command​:

grep '^#!/usr/bin/perl' /usr/*bin/* | cut -d '​:' -f 1 | xargs -n1 /bin/sh -c 'TEST="$0"; if [ "`strace -f perl -c \"$TEST\" 2>&1 | grep '\''stat(\"\..*\.pm\"'\''`" != "" ] ; then echo "Vulnerable​: $TEST"; else echo "Not vulnerable​: $TEST"; fi'

Or to test an individual script you can do something like this​:

strace -f /usr/bin/perldoc perldoc 2>&1 | grep 'stat("\..*\.pm"'

This kind of analysis definitely underestimates the number of affected scripts though.

So...with all that explanation said... My question is\, which codebase is supposed to fix this issue? I figure there are four possible answers to which piece of code was responsible for preventing these types of attacks​:

1) The Perl interpreter for placing the '.' in @​INC.

2) Any module that fails to remove the '.' from @​INC before attempting to load other modules that are not specified as hard requirements.

3) Any script that loads any modules that end up exhibiting this behavior.

4) End users must avoid running Perl scripts in directories that other users can modify. The only vulnerable code is code that moves to an unsafe location on its own.

At cPanel we started off with approach #3\, then ended up leaning towards #1. I can probably provide a fairly good list of places that need to be fixed in Debian once I know exactly where to focus.

I haven't contacted Debian's Perl team about this problem. Please CC whoever should be informed from that team if they are not already on the perl-security or debian-security lists.

I've never liked this behavior TBH. I've recommended using taint mode on system tools for exactly this reason (the other differences matter much less to me in most cases)\, even though it's a pain. Yet at the same time\, I fear the backwards compatibility consequences of this; it will be significant.

It's a mess :-(

Leon

[p5pRT]

From @toddr

On Apr 6\, 2016\, at 1​:49 PM\, John Lightsey \john@&#8203;nixnuts\.net wrote​:

On Wed\, 2016-04-06 at 10​:33 -0700\, Zefram via RT wrote​:

John Lightsey wrote​:

It would be very helpful to know which of the various pieces of code are responsible for addressing the risk with Perl before 5.26? The interpreter\, the modules\, or the script?

Patching older interpreters would change their behaviour in a breaking way\, so isn't a great idea. It's also infeasible for a lot of people; we'd like to have a solution that people who don't build their own perl can apply.

Modules are the wrong place to do this. It is not for a module to question the @​INC that it sees; even if it were\, there's no clear criterion for which modules ought to disrespect a "." entry. Generally\, modules are not aware of the security requirements and other situational aspects of the running program.

That leaves the script\, i.e.\, the top-level program. This is the right place. The top level knows what the overall purpose of running the program is\, knows what aspects of the environment can be trusted\, and what kind of security requirements can apply. It also has the capability to control the @​INC that will apply to almost all module loading that occurs as part of the program's compilation and execution. (It doesn't control PERL5OPT=-MFoo type stuff\, but that's much less of a security worry.)

One compelling reason to avoid this approach is just the numbers. There are a small handful of common modules that trigger this behavior over and over in most scripts.

For example\, by default my Debian box has 136 perl scripts that do a module load from the current working directory under a simple perl -c test.

If I fix the two most common modules that cause this (Storable loading Log​::Agent and Encode loading Encode​::ConfigLocal) by localizing and filtering @​INC before they load these optional dependencies\, the number of vulnerable scripts drops to 12.

It's much simpler to change 2 modules than 124 scripts as a security fix. The behavior of Storable and Encode does change as a result\, but not in a fashion anyone is likely dependent on\, and technically the scripts aren't guaranteed to be "safe". It's a simple way to get a lot of coverage with a very small set of changes though.

So this exchange gets to the heart of the problem Zefram argues the module shouldn't be manipulating @​INC. But as JD points out if we apply this simple fix\, all sorts of code is magically secure​:

Storable does​:

if (eval { local $SIG{__DIE__}; require Log​::Agent; 1 }) {   Log​::Agent->import;   }

In this case you could change it to this and fix Storable's issue​:

if (eval { local @​INC= grep {$_ ne "." } @​INC; local $SIG{__DIE__}; require Log​::Agent; 1 }) {   Log​::Agent->import;   }

[p5pRT]

From zefram@fysh.org

Todd Rinaldo wrote​:

In this case you could change it to this and fix Storable's issue​:

if (eval { local @​INC= grep {$_ ne "." } @​INC; local $SIG{__DIE__}; require Log​::Agent; 1 }) {

This kind of patch\, like the formulation that I proposed for top-level programs\, wouldn't work on OSes where the current directory is denoted by a pathname other than ".". In general that is a problem for modules\, which tend to be expected to be widely portable. Storable actually already has some dependence on Unix-like pathname semantics built in\, but only in a way that's difficult to invoke\, so in practice it's mostly portable\, and the portability would take a significant hit from adding this @​INC hack. This is much less of an issue for top-level program code\, which is much more likely to be OS-specific.

-zefram

[p5pRT]

From @craigberry

On Sat\, Apr 9\, 2016 at 11​:12 AM\, Zefram \zefram@&#8203;fysh\.org wrote​:

Todd Rinaldo wrote​:

In this case you could change it to this and fix Storable's issue​:

if (eval { local @​INC= grep {$_ ne "." } @​INC; local $SIG{__DIE__}; require Log​::Agent; 1 }) {

This kind of patch\, like the formulation that I proposed for top-level programs\, wouldn't work on OSes where the current directory is denoted by a pathname other than ".".

FWIW\, the @​INC entry for current working directory is '.' on both VMS and Windows. On Windows because that's the native format for current working directory. On VMS because (for the last five years or so) all paths are converted to Unix format before storing in @​INC. Not sure if there are any non-Unix\, non-Windows\, non-VMS filename syntaxes that need worrying about. Amiga?

[p5pRT]

From @jhi

On Saturday-201604-09 12​:53\, Craig A. Berry wrote​:

Not sure if there are any non-Unix\, non-Windows\, non-VMS filename syntaxes that need worrying about. Amiga?

I asked Andy Broad and the UNIX emulation layer used in AmigaOS does translate "." in INC to current directory. (FWIW\, the native form would be CURRDIR​: or through the emulation layer\, /CURRDIR.)

Though to quote Andy​: "Securitywise\, on the amiga the horse bolted before then barn was even built\, let alone the doors locked...."\, being essentially a single user system.

[p5pRT]

From zefram@fysh.org

Jarkko Hietaniemi wrote​:

Though to quote Andy​: "Securitywise\, on the amiga the horse bolted before then barn was even built\, let alone the doors locked...."\, being essentially a single user system.

Consistency of behaviour between systems is to be preferred\, even when it's not a matter of security.

-zefram

[p5pRT]

From @jhi

On Saturday-201604-09 20​:38\, Zefram wrote​:

Jarkko Hietaniemi wrote​:

Though to quote Andy​: "Securitywise\, on the amiga the horse bolted before then barn was even built\, let alone the doors locked...."\, being essentially a single user system.

Consistency of behaviour between systems is to be preferred\, even when it's not a matter of security.

Sure. Nobody was arguing the opposite.

-zefram

[p5pRT]

From @toddr

On Apr 9\, 2016\, at 11​:12 AM\, Zefram via RT \perl5\-security\-report@&#8203;perl\.org wrote​:

Todd Rinaldo wrote​:

In this case you could change it to this and fix Storable's issue​:

if (eval { local @​INC= grep {$_ ne "." } @​INC; local $SIG{__DIE__}; require Log​::Agent; 1 }) {

This kind of patch\, like the formulation that I proposed for top-level programs\, wouldn't work on OSes where the current directory is denoted by a pathname other than ".". In general that is a problem for modules\, which tend to be expected to be widely portable. Storable actually already has some dependence on Unix-like pathname semantics built in\, but only in a way that's difficult to invoke\, so in practice it's mostly portable\, and the portability would take a significant hit from adding this @​INC hack. This is much less of an issue for top-level program code\, which is much more likely to be OS-specific.

Based on the replies already\, . in @​INC is the same on all supported systems\, right? So this is a possible fix?

Todd

[p5pRT]

From @tonycoz

On Mon Apr 11 15​:56​:51 2016\, toddr@​cpanel.net wrote​:

Based on the replies already\, . in @​INC is the same on all supported systems\, right? So this is a possible fix?

Considering your last patch set\, I think​:

a) we should hide the unsafe_inc options a little deeper (require -Accflags=-D...)\, or not have it at all.

b) we shouldn't have the environment variable\, adjusting PERL5LIB is suitable for most purposes.

Tony

[p5pRT]

From @Leont

On Tue\, Apr 12\, 2016 at 3​:02 AM\, Tony Cook via RT \< perl5-security-report@​perl.org> wrote​:

Considering your last patch set\, I think​:

a) we should hide the unsafe_inc options a little deeper (require -Accflags=-D...)\, or not have it at all.

I'd go for hiding. Making it impossible seems unperlish\, but we do want to disrecommend this strongly.

b) we shouldn't have the environment variable\, adjusting PERL5LIB is suitable for most purposes.

PERL5LIB added to the front of @​INC\, while . is currently added to the back; this is a fundamentally different thing to me. I do believe we need a way to explicitly say to add it back in. To me a command-line flag is more appropriate than an environmental flag\, if only because you can use PERL5OPT to make it an environmental flag if necessary.

Leon

[p5pRT]

From @lightsey

On Tue\, 2016-04-12 at 01​:27 -0700\, Leon Timmermans via RT wrote​:

On Tue\, Apr 12\, 2016 at 3​:02 AM\, Tony Cook via RT \< perl5-security-report@​perl.org> wrote​:

Considering your last patch set\, I think​:

a) we should hide the unsafe_inc options a little deeper (require -Accflags=-D...)\, or not have it at all.

I'd go for hiding. Making it impossible seems unperlish\, but we do want to disrecommend this strongly.

b) we shouldn't have the environment variable\, adjusting PERL5LIB is suitable for most purposes.

PERL5LIB added to the front of @​INC\, while . is currently added to the back; this is a fundamentally different thing to me. I do believe we need a way to explicitly say to add it back in. To me a command-line flag is more appropriate than an environmental flag\, if only because you can use PERL5OPT to make it an environmental flag if necessary.

The downside to this approach is that the code using it must be aware of the version of Perl that will inherit the PERL5OPT setting. Any version of Perl before the new flag is added will break if it inherits the PERL5OPT setting.

Using a new environmental variable that returned the original @​INC configuration kept the original proposed patch very small because the versions of Perl that already had '.' in @​INC would ignore it.

[p5pRT]

From @toddr

On Apr 12\, 2016\, at 11​:18 AM\, John Lightsey via RT \perl5\-security\-report@&#8203;perl\.org wrote​:

On Tue\, 2016-04-12 at 01​:27 -0700\, Leon Timmermans via RT wrote​:

On Tue\, Apr 12\, 2016 at 3​:02 AM\, Tony Cook via RT \< perl5-security-report@​perl.org> wrote​:

Considering your last patch set\, I think​:

a) we should hide the unsafe_inc options a little deeper (require -Accflags=-D...)\, or not have it at all.

I'd go for hiding. Making it impossible seems unperlish\, but we do want to disrecommend this strongly.

b) we shouldn't have the environment variable\, adjusting PERL5LIB is suitable for most purposes.

PERL5LIB added to the front of @​INC\, while . is currently added to the back; this is a fundamentally different thing to me. I do believe we need a way to explicitly say to add it back in. To me a command-line flag is more appropriate than an environmental flag\, if only because you can use PERL5OPT to make it an environmental flag if necessary.

The downside to this approach is that the code using it must be aware of the version of Perl that will inherit the PERL5OPT setting. Any version of Perl before the new flag is added will break if it inherits the PERL5OPT setting.

Using a new environmental variable that returned the original @​INC configuration kept the original proposed patch very small because the versions of Perl that already had '.' in @​INC would ignore it. \<signature.asc>

IMO This whole conversation about what to do for 5.26 needs to move to RT 127810 so it will do better as a public discussion. This thread seems like a better place for us to discuss the proper way to fix 5.24 and below and how to coordinate the security disclosure that's going to have to come with it\, right?

Todd

[p5pRT]

From @lightsey

On Tue\, 2016-04-12 at 09​:33 -0700\, Todd Rinaldo via RT wrote​:

  IMO This whole conversation about what to do for 5.26 needs to move to RT 127810 so it will do better as a public discussion. This thread seems like a better place for us to discuss the proper way to fix 5.24 and below and how to coordinate the security disclosure that's going to have to come with it\, right?

The discussion about what to do with 5.24 and earlier seems to have died off here.

It's probably unrealistic to attempt coordinating simultaneous fixes for all of the individual Perl scripts that are unsafe to run in a directory that another user has write access to going backwards.

I would think of this more like XXE attacks or the RLIM_NPROC setuid() attacks where an unsafe but documented behavior made large bodies of code vulnerable to a specific attack scenario that was not considered when the behavior was established.

I'm guessing\, that the Debian security team does want to fix all of the apt related Perl scripts before the problem is discussed publicly.

Does the Perl security team want to fix the scripts it maintains that have the same problems?

I'd really like to present this stuff publicly to make Perl developers and security testers aware of the risks\, but I don't want to do so until both teams are satisfied that they have fixed everything they feel is important enough to fix.

What's a reasonable timeframe before publicly discussing attacks against apt\, perldoc\, and the LWP tools?

[p5pRT]

From @demerphq

On 15 April 2016 at 21​:25\, John Lightsey \lightsey@&#8203;debian\.org wrote​:

On Tue\, 2016-04-12 at 09​:33 -0700\, Todd Rinaldo via RT wrote​:

IMO This whole conversation about what to do for 5.26 needs to move to RT 127810 so it will do better as a public discussion. This thread seems like a better place for us to discuss the proper way to fix 5.24 and below and how to coordinate the security disclosure that's going to have to come with it\, right?

The discussion about what to do with 5.24 and earlier seems to have died off here.

I think we are still figuring things out. Ric\, our point man for these things has been busy with a new release\, and has decided to step down from his role as a leader in our community\, so we are a bit at loose ends right now.

It's probably unrealistic to attempt coordinating simultaneous fixes for all of the individual Perl scripts that are unsafe to run in a directory that another user has write access to going backwards.

I have been thinking about this. I guess it means we are vulnerable in any script where use lib is required to run\, as well as any script that depends on finding a file in "."\, or any script which does an "eval require" or "eval use" right?

Is that right?

I would think of this more like XXE attacks or the RLIM_NPROC setuid() attacks where an unsafe but documented behavior made large bodies of code vulnerable to a specific attack scenario that was not considered when the behavior was established.

I'm guessing\, that the Debian security team does want to fix all of the apt related Perl scripts before the problem is discussed publicly.

I am just thinking that a lot of these bugs can be fixed by distribution of all dependencies. That would fix any code that does "eval use/eval require".

Does the Perl security team want to fix the scripts it maintains that have the same problems?

I think we do.

I'd really like to present this stuff publicly to make Perl developers and security testers aware of the risks\, but I don't want to do so until both teams are satisfied that they have fixed everything they feel is important enough to fix.

I think this is already being half discussed in public\, although I don't think that the public thread has made obvious references to the security dimension.

I worry that discussing the fix for 5.26 in a public thread is a good idea actually.

What's a reasonable timeframe before publicly discussing attacks against apt\, perldoc\, and the LWP tools?

IMO we would definitely like more time before this happens.

Cheers\, yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

[p5pRT]

From @lightsey

On Sat\, 2016-04-16 at 15​:57 +0200\, demerphq wrote​:

On 15 April 2016 at 21​:25\, John Lightsey \lightsey@&#8203;debian\.org wrote​:

On Tue\, 2016-04-12 at 09​:33 -0700\, Todd Rinaldo via RT wrote​:

IMO This whole conversation about what to do for 5.26 needs to move to RT 127810 so it will do better as a public discussion. This thread seems like a better place for us to discuss the proper way to fix 5.24 and below and how to coordinate the security disclosure that's going to have to come with it\, right?

The discussion about what to do with 5.24 and earlier seems to have died off here.

I think we are still figuring things out. Ric\, our point man for these things has been busy with a new release\, and has decided to step down from his role as a leader in our community\, so we are a bit at loose ends right now.

It's probably unrealistic to attempt coordinating simultaneous fixes for all of the individual Perl scripts that are unsafe to run in a directory that another user has write access to going backwards.

I have been thinking about this. I guess it means we are vulnerable in any script where use lib is required to run\, as well as any script that depends on finding a file in "."\, or any script which does an "eval require" or "eval use" right?

Is that right?

More or less\, yes...

I'd think of it this way​:

- All Perl scripts start off with the risk of loading a module from the current directory.

- The Perl script can eliminate this risk entirely by turning on taint mode or filtering @​INC in a BEGIN block before loading any modules.

- If the Perl script doesn't eliminate the risk directly\, then the risk becomes an actual flaw if the script or one of the modules it uses attempts to load a module that doesn't exist in any of the normal library paths.

- Flawed scripts are widespread now because certain core modules try to load optional modules that typically are not installed (Storable\, Encode\, and Pod have this behavior.)

In terms of ease of deployment and overall effect\, you get much better bang for the buck by simply fixing the core modules so that they don't look for optional dependencies in '.'.

Filtering @​INC in Storable before it tries to load Log​::Agent and in Encode before it tries to load Encode​::ConfigLocal switches 90% of the "flawed" scripts on my system to just being "at risk". It's likely that changing a dozen or so common modules in this fashion would push that number up to 99% or higher. It doesn't guarantee that subsequent updates to other modules won't reintroduce a similar flawed behavior\, but it has a very large immediate effect through very small set of changes.

I would think of this more like XXE attacks or the RLIM_NPROC setuid() attacks where an unsafe but documented behavior made large bodies of code vulnerable to a specific attack scenario that was not considered when the behavior was established.

I'm guessing\, that the Debian security team does want to fix all of the apt related Perl scripts before the problem is discussed publicly.

I am just thinking that a lot of these bugs can be fixed by distribution of all dependencies. That would fix any code that does "eval use/eval require".

Does the Perl security team want to fix the scripts it maintains that have the same problems?

I think we do.

I'd really like to present this stuff publicly to make Perl developers and security testers aware of the risks\, but I don't want to do so until both teams are satisfied that they have fixed everything they feel is important enough to fix.

I think this is already being half discussed in public\, although I don't think that the public thread has made obvious references to the security dimension.

I worry that discussing the fix for 5.26 in a public thread is a good idea actually.

Me too. I'd much prefer that the immediate issues with Perl scripts in 5.24 and earlier are addressed before getting too deep into discussions about what sort of things the Perl interpreter should be doing to prevent these kinds of issues in the future.

It's difficult to explain why the @​INC changes for 5.26 are important without also making it obvious how the current behavior can be misused.

You'll have to forgive me and Todd for these two discussions running concurrently. We both worked on this problem in different areas for a long time before I clued into the fact that half of the Linux systems in existence could be rooted with it.

The main goal in looking for ways to abuse the @​INC behavior in 5.24 and earlier was to justify its removal in 5.26. The subject was raised on P5P before and the risk was discounted as theoretical in the past.

What's a reasonable timeframe before publicly discussing attacks against apt\, perldoc\, and the LWP tools?

IMO we would definitely like more time before this happens.

Cheers\, yves

[p5pRT]

From @fweimer

* John Lightsey​:

Filtering @​INC in Storable before it tries to load Log​::Agent and in Encode before it tries to load Encode​::ConfigLocal switches 90% of the "flawed" scripts on my system to just being "at risk". It's likely that changing a dozen or so common modules in this fashion would push that number up to 99% or higher. It doesn't guarantee that subsequent updates to other modules won't reintroduce a similar flawed behavior\, but it has a very large immediate effect through very small set of changes.

It's also an option that has reduced risk of breaking legitimate user/developer setups which may rely on @​INC containing .

So from a distribution point-of-view\, it is more work than a single patch to Perl itself\, but I think it is still preferable for us over removal of . from the default @​INC.

[p5pRT]

From @demerphq

On 16 April 2016 at 19​:30\, Florian Weimer \fw@&#8203;deneb\.enyo\.de wrote​:

* John Lightsey​:

Filtering @​INC in Storable before it tries to load Log​::Agent and in Encode before it tries to load Encode​::ConfigLocal switches 90% of the "flawed" scripts on my system to just being "at risk". It's likely that changing a dozen or so common modules in this fashion would push that number up to 99% or higher. It doesn't guarantee that subsequent updates to other modules won't reintroduce a similar flawed behavior\, but it has a very large immediate effect through very small set of changes.

It's also an option that has reduced risk of breaking legitimate user/developer setups which may rely on @​INC containing .

So from a distribution point-of-view\, it is more work than a single patch to Perl itself\, but I think it is still preferable for us over removal of . from the default @​INC.

Note that any distribution could achieve the same result without changing ANY code by simply always distributing Log​::Agent with their Perl builds. This is a backwards compatible approach too. Any existing system can ameliorate their risk by simply installing the missing optional dependencies all the time. Yes it results in distribution bloat\, but it is also safe and easy.

Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

[p5pRT]

From @iabyn

On Sun\, Apr 17\, 2016 at 08​:03​:11AM +0200\, demerphq wrote​:

Note that any distribution could achieve the same result without changing ANY code by simply always distributing Log​::Agent with their Perl builds. This is a backwards compatible approach too. Any existing system can ameliorate their risk by simply installing the missing optional dependencies all the time. Yes it results in distribution bloat\, but it is also safe and easy.

I think that overall we're not taking this issue seriously enough.

A quick scan through the .pm files in the core turns up 280 instances of this match​:

  /(eval\s*\{[^}]*require\s*\S+)/msg

i.e. where we're looking for something like 'eval { ....; require .... }'. While some of these will be false positives\, many of them represent attempts to load optional modules not included in the core\, e.g.

  dist/Test/lib/Test.pm​:

  if eval { require Algorithm​::Diff; Algorithm​::Diff->VERSION(1.15); 1; };

Even the ones which optionally load core modules will be vulnerable in an OS distribution which includes perl but doesn't include some core modules by default (many Linux distros do this).

A similar search on grep.cpan.me shows 'over 1000' affected distributions.

Exploiting this is as easy as writing a bunch of files in a common writable directory\, e.g. /tmp/Algorithm/Diff.pm\, then sit back and wait a while\, then you've got access to any account where that user has run a relevant perl script while chdir'ed to that writable directory. If they ran it while root\, then the whole system is immediately compromised.

Once could also envisage a CGI-type script that allows files to be uploaded to a temporary staging directory\, and where the script chdir's to that directory\, thus giving a remote exploit.

The big problem we have is that 5.24.0-RC1 is due to be released in 2 day's time\, and the question is what\, if anything\, we do before 5.24.0 is released.

One possible compromise for 5.24.0 would be be exclude '.' in @​INC only if the script is running as root (and possibly something analogous for non-UNIXY OSes).

Conventional wisdom is that things like perl shouldn't be built while logged in as root; only the final 'make install' step should be run as root.

Blead 'make install' can be made to work while running as root (and thus with no '.') with a simple fix to pod/buildtoc​:

  - require 'Porting/pod_lib.pl';   + require './Porting/pod_lib.pl';

I don't know whether installing CPAN modules while running as root would break\, however.

So it might be that the minimum we do for 5.24.0 is hack perl.c so that '.' isn't pushed when running as root\, plus the pod/buildtoc fix.

Then we could extend the proposed Configure/-Accflags build option to be multivalued rather than binary\, to handle 3 options​:

  1) never push '.'   2) push '.' only if not root   3) always push '.'

Then we could discuss whether that new build option should be included with 5.24.0 or leave till 5.25.0. if the former\, then we would make (2) the default in 5.24.0 and (1) the default in 5.26.0.

Further\, with a 'belt and braces' approach to security\, I think it would still be worth fixing up modules that do an optional require\, to make them localise @​INC without the '.'​: especially dual-lived modules\, which will then auto-fix older perls. Whether we should pull these fixes into core for 5.24.0 too\, I don't know.

Finally\, I think it's unreasonable to expect writers of scripts to know that they should always sanitise @​INC at the top of their scripts.

-- More than any other time in history\, mankind faces a crossroads. One path leads to despair and utter hopelessness. The other\, to total extinction. Let us pray we have the wisdom to choose correctly.   -- Woody Allen

[p5pRT]

From @Leont

On Mon\, Apr 18\, 2016 at 4​:13 PM\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

I think that overall we're not taking this issue seriously enough.

A quick scan through the .pm files in the core turns up 280 instances of this match​:

/\(eval\\s\*\\\{\[^\}\]\*require\\s\*\\S\+\)/msg

i.e. where we're looking for something like 'eval { ....; require .... }'. While some of these will be false positives\, many of them represent attempts to load optional modules not included in the core\, e.g.

dist/Test/lib/Test\.pm​:

 if eval \{ require Algorithm​::Diff; Algorithm​::Diff\->VERSION\(1\.15\); 1;

};

Even the ones which optionally load core modules will be vulnerable in an OS distribution which includes perl but doesn't include some core modules by default (many Linux distros do this).

A similar search on grep.cpan.me shows 'over 1000' affected distributions.

Exploiting this is as easy as writing a bunch of files in a common writable directory\, e.g. /tmp/Algorithm/Diff.pm\, then sit back and wait a while\, then you've got access to any account where that user has run a relevant perl script while chdir'ed to that writable directory. If they ran it while root\, then the whole system is immediately compromised.

Once could also envisage a CGI-type script that allows files to be uploaded to a temporary staging directory\, and where the script chdir's to that directory\, thus giving a remote exploit.

The big problem we have is that 5.24.0-RC1 is due to be released in 2 day's time\, and the question is what\, if anything\, we do before 5.24.0 is released.

One possible compromise for 5.24.0 would be be exclude '.' in @​INC only if the script is running as root (and possibly something analogous for non-UNIXY OSes).

Conventional wisdom is that things like perl shouldn't be built while logged in as root; only the final 'make install' step should be run as root.

Blead 'make install' can be made to work while running as root (and thus with no '.') with a simple fix to pod/buildtoc​:

\-  require 'Porting/pod\_lib\.pl';
\+  require '\./Porting/pod\_lib\.pl';

I don't know whether installing CPAN modules while running as root would break\, however.

So it might be that the minimum we do for 5.24.0 is hack perl.c so that '.' isn't pushed when running as root\, plus the pod/buildtoc fix.

Then we could extend the proposed Configure/-Accflags build option to be multivalued rather than binary\, to handle 3 options​:

1\) never push '\.'
2\) push '\.' only if not root
3\) always push '\.'

Then we could discuss whether that new build option should be included with 5.24.0 or leave till 5.25.0. if the former\, then we would make (2) the default in 5.24.0 and (1) the default in 5.26.0.

Further\, with a 'belt and braces' approach to security\, I think it would still be worth fixing up modules that do an optional require\, to make them localise @​INC without the '.'​: especially dual-lived modules\, which will then auto-fix older perls. Whether we should pull these fixes into core for 5.24.0 too\, I don't know.

Finally\, I think it's unreasonable to expect writers of scripts to know that they should always sanitise @​INC at the top of their scripts.

Another option may be to not add '.' if it isn't owned by you or root\, or if it is world-writable. Both of these can easily be checked for (at least on Unix). This won't safe you if your program chdir()s\, but I suspect that category of programs is significantly smaller.

Leon

[p5pRT]

From @toddr

On Apr 18\, 2016\, at 10​:41 AM\, Leon Timmermans \fawaka@&#8203;gmail\.com wrote​:

On Mon\, Apr 18\, 2016 at 4​:13 PM\, Dave Mitchell \<davem@​iabyn.com \mailto&#8203;:davem@&#8203;iabyn\.com> wrote​: I think that overall we're not taking this issue seriously enough.

A quick scan through the .pm files in the core turns up 280 instances of this match​:

/\(eval\\s\*\\\{\[^\}\]\*require\\s\*\\S\+\)/msg

i.e. where we're looking for something like 'eval { ....; require .... }'. While some of these will be false positives\, many of them represent attempts to load optional modules not included in the core\, e.g.

dist/Test/lib/Test\.pm​:

 if eval \{ require Algorithm​::Diff; Algorithm​::Diff\->VERSION\(1\.15\); 1; \};

Even the ones which optionally load core modules will be vulnerable in an OS distribution which includes perl but doesn't include some core modules by default (many Linux distros do this).

A similar search on grep.cpan.me \<http​://grep.cpan.me/> shows 'over 1000' affected distributions.

Exploiting this is as easy as writing a bunch of files in a common writable directory\, e.g. /tmp/Algorithm/Diff.pm\, then sit back and wait a while\, then you've got access to any account where that user has run a relevant perl script while chdir'ed to that writable directory. If they ran it while root\, then the whole system is immediately compromised.

Once could also envisage a CGI-type script that allows files to be uploaded to a temporary staging directory\, and where the script chdir's to that directory\, thus giving a remote exploit.

The big problem we have is that 5.24.0-RC1 is due to be released in 2 day's time\, and the question is what\, if anything\, we do before 5.24.0 is released.

One possible compromise for 5.24.0 would be be exclude '.' in @​INC only if the script is running as root (and possibly something analogous for non-UNIXY OSes).

Conventional wisdom is that things like perl shouldn't be built while logged in as root; only the final 'make install' step should be run as root.

Blead 'make install' can be made to work while running as root (and thus with no '.') with a simple fix to pod/buildtoc​:

\-  require 'Porting/pod\_lib\.pl \<http&#8203;://pod\_lib\.pl/>';
\+  require '\./Porting/pod\_lib\.pl \<http&#8203;://pod\_lib\.pl/>';

I don't know whether installing CPAN modules while running as root would break\, however.

So it might be that the minimum we do for 5.24.0 is hack perl.c so that '.' isn't pushed when running as root\, plus the pod/buildtoc fix.

Then we could extend the proposed Configure/-Accflags build option to be multivalued rather than binary\, to handle 3 options​:

1\) never push '\.'
2\) push '\.' only if not root
3\) always push '\.'

Then we could discuss whether that new build option should be included with 5.24.0 or leave till 5.25.0. if the former\, then we would make (2) the default in 5.24.0 and (1) the default in 5.26.0.

Further\, with a 'belt and braces' approach to security\, I think it would still be worth fixing up modules that do an optional require\, to make them localise @​INC without the '.'​: especially dual-lived modules\, which will then auto-fix older perls. Whether we should pull these fixes into core for 5.24.0 too\, I don't know.

Finally\, I think it's unreasonable to expect writers of scripts to know that they should always sanitise @​INC at the top of their scripts.

Another option may be to not add '.' if it isn't owned by you or root\, or if it is world-writable. Both of these can easily be checked for (at least on Unix). This won't safe you if your program chdir()s\, but I suspect that category of programs is significantly smaller.

This isn't technically a root only problem. It's of course much worse when it's a root process but lots of critical processes don't run as root. I'm also not clear if the original report JD provided would not be addressed by this fix.

Todd

[p5pRT]

From @craigberry

On Mon\, Apr 18\, 2016 at 10​:41 AM\, Leon Timmermans \fawaka@&#8203;gmail\.com wrote​:

On Mon\, Apr 18\, 2016 at 4​:13 PM\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

One possible compromise for 5.24.0 would be be exclude '.' in @​INC only if the script is running as root (and possibly something analogous for non-UNIXY OSes).

Another option may be to not add '.' if it isn't owned by you or root\, or if it is world-writable. Both of these can easily be checked for (at least on Unix). This won't safe you if your program chdir()s\, but I suspect that category of programs is significantly smaller.

Is there a way to describe the privilege involved that doesn't require referring to the root user? Perhaps testing for whether the filesystem bits tell you that you do not have write access to the current working directory but a stat() tells you that you do? I believe geteuid always returns zero on Windows and never returns zero on VMS\, and don't modern unixen often define privileged access (ACLs and what-not) that do not require being root?

For the chdir problem\, perhaps pp_require could become involved as well as the initial loading of @​INC.

[p5pRT]

From @Leont

On Mon\, Apr 18\, 2016 at 6​:05 PM\, Craig A. Berry \craig\.a\.berry@&#8203;gmail\.com wrote​:

Is there a way to describe the privilege involved that doesn't require referring to the root user? Perhaps testing for whether the filesystem bits tell you that you do not have write access to the current working directory but a stat() tells you that you do?

What I suggested has nothing to do with "do I have permissions"\, but with "do I trust the owner".

I believe geteuid always returns zero on Windows and never returns zero on VMS\, and don't modern unixen often define privileged access (ACLs and what-not) that do not require being root?

Security logic is rarely portable between OSes; I wasn't not expecting this to work verbatim on non-unix.

For the chdir problem\, perhaps pp_require could become involved as well as the initial loading of @​INC.

I suspect that will quickly become something too complicated for the current infrastructure.

Leon

[p5pRT]

From ask@nutmegjobs.com

On Apr 18\, 2016\, at 08​:41\, Leon Timmermans \fawaka@&#8203;gmail\.com wrote​:

Another option may be to not add '.' if it isn't owned by you or root\, or if it is world-writable. Both of these can easily be checked for (at least on Unix). This won't safe you if your program chdir()s\, but I suspect that category of programs is significantly smaller.

Isn't that just extra complexity for no real gain? It'd really need to check if ./Module/Path/File.pm or any of the parent directories have "bad" permissions (not necessarily limited to world writable).

Ask

[p5pRT]

From @craigberry

On Mon\, Apr 18\, 2016 at 11​:27 AM\, Leon Timmermans \fawaka@&#8203;gmail\.com wrote​:

On Mon\, Apr 18\, 2016 at 6​:05 PM\, Craig A. Berry \craig\.a\.berry@&#8203;gmail\.com wrote​:

Is there a way to describe the privilege involved that doesn't require referring to the root user? Perhaps testing for whether the filesystem bits tell you that you do not have write access to the current working directory but a stat() tells you that you do?

What I suggested has nothing to do with "do I have permissions"\, but with "do I trust the owner".

Right\, sorry. I conflated the desire to prevent the problem from happening with the desire to limit the damage when it does happen. We probably have to attempt both\, but perhaps not all at once.

I believe geteuid always returns zero on Windows and never returns zero on VMS\, and don't modern unixen often define privileged access (ACLs and what-not) that do not require being root?

Security logic is rarely portable between OSes; I wasn't not expecting this to work verbatim on non-unix.

It should be possible to test ownership and world writability across platforms. At least more easily than testing for privilege. It might still be tricky on any platform to define who a trusted owner is.

For the chdir problem\, perhaps pp_require could become involved as well as the initial loading of @​INC.

I suspect that will quickly become something too complicated for the current infrastructure.

I think we're doomed to attempting multiple partial mitigations in the absence of any simple solution that won't cause breakage. I wouldn't rule out looking at pp_require at some point\, but I agree there are complications there and it's not a good place to start.

Back to Dave's point. If we want any mitigation at all in 5.24.0\, it should have been done and ready to go\, like a week ago. The only thing that seems simple enough to do quickly is his suggestion of preventing '.' in @​INC when root. However\, won't the very common idiom​:

$ sudo cpan -i Acme​::Foo

fail if Acme​::Foo is using Module​::Install or if its tests depend on '.' in @​INC to find things?

[p5pRT]

From @lightsey

On Sat\, 2016-04-16 at 19​:30 +0200\, Florian Weimer wrote​:

* John Lightsey​:

Filtering @​INC in Storable before it tries to load Log​::Agent and in Encode before it tries to load Encode​::ConfigLocal switches 90% of the "flawed" scripts on my system to just being "at risk". It's likely that changing a dozen or so common modules in this fashion would push that number up to 99% or higher. It doesn't guarantee that subsequent updates to other modules won't reintroduce a similar flawed behavior\, but it has a very large immediate effect through very small set of changes.

It's also an option that has reduced risk of breaking legitimate user/developer setups which may rely on @​INC containing .

So from a distribution point-of-view\, it is more work than a single patch to Perl itself\, but I think it is still preferable for us over removal of . from the default @​INC.

This may help a bit in evaluating any potential fixes...

The attached systemtap script will display a message whenever something on the system tries to load a Perl module using the '.' in @​INC. It shows the command line of the script\, the directory it was running in\, and the module it was trying to load. It's much simpler to test with globally than strace.

Install systemtap with​:

apt-get install linux-headers-`uname -r` linux-image-`uname -r`-dbg systemtap

Then start it running as root with​:

stap perl_inc.stap

And in another terminal run commands that directly or indirectly execute perl​:

cd /tmp/ perldoc perlrun apt-get update && apt-get upgrade apt-get install --reinstall exim4-config exim4-daemon-heavy

You should see output from systemtap like this​:

CMD​: "/usr/bin/perl /usr/bin/perldoc perlrun" DIR​: "/tmp" FILENAME​: "./Encode/ConfigLocal.pm" CMD​: "/usr/bin/perl /usr/bin/perldoc perlrun" DIR​: "/tmp" FILENAME​: "./Pod/Perldoc/Toterm.pm" CMD​: "/usr/bin/perl -w /usr/bin/apt-show-versions -i" DIR​: "/tmp" FILENAME​: "./Log/Agent.pm" CMD​: "/usr/bin/perl -w /usr/sbin/dpkg-preconfigure --apt" DIR​: "/tmp" FILENAME​: "./Encode/ConfigLocal.pm" ...

The modules that pop out the most on my system with simple perl -c tests are​:

Encode​::ConfigLocal (via Encode) Log​::Agent (via Storable) Locale​::Maketext​::Lexicon (via Local​::Maketext​::Simple) Mo​::builder and Mo​::default (via YAML​::Mo)

Other ones that show up repeatedly when actually running different Perl scripts​:

Pod​::Perldoc​::ToXXXX (via Pod​::Perldoc's find_good_formatter_class()) Net​::LocalConfig (via Net​::Config) Term​::ReadLine​::XXXX (via Term​::ReadLine) IO​::Uncompress​::XXXX (via IO​::Uncompress​::AnyUncompress)

[p5pRT]

From @lightsey

probe syscall.stat {   if (@​defined(filename) && filename =~ "^\"\./.*\\.pm\"$") {   tc = task_current()   pwd_dentry = @​cast(tc\, "task_struct")->fs->pwd->dentry   pwd_mnt = @​cast(tc\, "task_struct")->fs->pwd->mnt   pwd_path = task_dentry_path(tc\, pwd_dentry\, pwd_mnt)   printf("CMD​: \"%s\" DIR​: \"%s\" FILENAME​: %s\n"\, cmdline_str()\, pwd_path\, filename)   } }

[p5pRT]

From @steve-m-hay

There has been discussion in this ticket about which version(s) of perl to apply any fixes to\, and I'm wondering whether it's worth delaying the release of 5.22.2\, which is otherwise due to go ahead tomorrow (24th)?

Does anyone know if any fixes are sufficiently close for that to be sensible and whether there are any plans for 5.24.0 to be delayed? On 21 Apr 2016 06​:30\, "John Lightsey" \lightsey@&#8203;debian\.org wrote​:

On Sat\, 2016-04-16 at 19​:30 +0200\, Florian Weimer wrote​:

* John Lightsey​:

Filtering @​INC in Storable before it tries to load Log​::Agent and in Encode before it tries to load Encode​::ConfigLocal switches 90% of the "flawed" scripts on my system to just being "at risk". It's likely that changing a dozen or so common modules in this fashion would push that number up to 99% or higher. It doesn't guarantee that subsequent updates to other modules won't reintroduce a similar flawed behavior\, but it has a very large immediate effect through very small set of changes.

It's also an option that has reduced risk of breaking legitimate user/developer setups which may rely on @​INC containing .

So from a distribution point-of-view\, it is more work than a single patch to Perl itself\, but I think it is still preferable for us over removal of . from the default @​INC.

This may help a bit in evaluating any potential fixes...

The attached systemtap script will display a message whenever something on the system tries to load a Perl module using the '.' in @​INC. It shows the command line of the script\, the directory it was running in\, and the module it was trying to load. It's much simpler to test with globally than strace.

Install systemtap with​:

apt-get install linux-headers-`uname -r` linux-image-`uname -r`-dbg systemtap

Then start it running as root with​:

stap perl_inc.stap

And in another terminal run commands that directly or indirectly execute perl​:

cd /tmp/ perldoc perlrun apt-get update && apt-get upgrade apt-get install --reinstall exim4-config exim4-daemon-heavy

You should see output from systemtap like this​:

CMD​: "/usr/bin/perl /usr/bin/perldoc perlrun" DIR​: "/tmp" FILENAME​: "./Encode/ConfigLocal.pm" CMD​: "/usr/bin/perl /usr/bin/perldoc perlrun" DIR​: "/tmp" FILENAME​: "./Pod/Perldoc/Toterm.pm" CMD​: "/usr/bin/perl -w /usr/bin/apt-show-versions -i" DIR​: "/tmp" FILENAME​: "./Log/Agent.pm" CMD​: "/usr/bin/perl -w /usr/sbin/dpkg-preconfigure --apt" DIR​: "/tmp" FILENAME​: "./Encode/ConfigLocal.pm" ...

The modules that pop out the most on my system with simple perl -c tests are​:

Encode​::ConfigLocal (via Encode) Log​::Agent (via Storable) Locale​::Maketext​::Lexicon (via Local​::Maketext​::Simple) Mo​::builder and Mo​::default (via YAML​::Mo)

Other ones that show up repeatedly when actually running different Perl scripts​:

Pod​::Perldoc​::ToXXXX (via Pod​::Perldoc's find_good_formatter_class()) Net​::LocalConfig (via Net​::Config) Term​::ReadLine​::XXXX (via Term​::ReadLine) IO​::Uncompress​::XXXX (via IO​::Uncompress​::AnyUncompress)

[p5pRT]

From @iabyn

On Sun\, Apr 24\, 2016 at 01​:16​:33AM +0100\, Steve Hay via perl5-security-report wrote​:

There has been discussion in this ticket about which version(s) of perl to apply any fixes to\, and I'm wondering whether it's worth delaying the release of 5.22.2\, which is otherwise due to go ahead tomorrow (24th)?

Does anyone know if any fixes are sufficiently close for that to be sensible and whether there are any plans for 5.24.0 to be delayed?

I think we're quite a ways off yet having a clear plan about to what do\, so I would suggest releasing 5.22.2 now; we can always push out a 5.22.3 if necessary.

-- This is a great day for France!   -- Nixon at Charles De Gaulle's funeral

[p5pRT]

From @steve-m-hay

On 25 April 2016 at 09​:01\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

On Sun\, Apr 24\, 2016 at 01​:16​:33AM +0100\, Steve Hay via perl5-security-report wrote​:

There has been discussion in this ticket about which version(s) of perl to apply any fixes to\, and I'm wondering whether it's worth delaying the release of 5.22.2\, which is otherwise due to go ahead tomorrow (24th)?

Does anyone know if any fixes are sufficiently close for that to be sensible and whether there are any plans for 5.24.0 to be delayed?

I think we're quite a ways off yet having a clear plan about to what do\, so I would suggest releasing 5.22.2 now; we can always push out a 5.22.3 if necessary.

Ok\, thanks for confirmation. I will release 5.22.2 this week\, or Saturday at the latest. And I'll be happy to make 5.22.3 with the fix whenever required.

[p5pRT]

From @iabyn

I've been doing some more pondering about this "." in @​INC malarkey.

The way I see it is that the end-goal is for it be be usual for perl to be built with "." excluded. But this is a way off yet as it breaks everything. In the interim\, we'd like ideally to present a spectrum of build options for perl which vary the balance of the risks of insecurity verses breakage; people can choose how strict they want their perl to be built as\, and the default we select for perl can be make stricter over time.

Todd's current patch represents something at the stricter end of the spectrum​: it removes "." from @​INC completely\, but in recognition that​: * the perl module build process often relies on "."\, * the end-user can't easily fix the perl modules\, it makes MakeMaker\, cpan etc set an environment var which basically tells perl\, "hey\, we're doing a module install\, reinstate '.'".

I'd like to make two further proposals as to what to do at the other end of the spectrum. Here\, we include "."\, but don't honour it under certain limited circumstances - namely those circumstances which are likely to be exploitable.

Both these proposals assume that the "." added by the perl executable itself during startup is specially flagged in some way\, meaning that require et al can distinguish it from any other "." in @​INC that the script may have added itself. This means that its possible to explicitly override the behaviours I'm about to suggest if the script wants to.

First\, a mutation of my earlier idea of not including "." if running as root (which has various issues that people pointed out). Instead\, how about​: when require has located a file using the @​INC search path\, and the matching @​INC entry was the system's "."​: then before loading it\, check whether the file is owned by either root or the process's euid. If neither\, it croaks with some sort of "refuse to load insecure module" error. This should stop most cases of bad files under /tmp\, root running a script in another user's home directory etc. It shouldn't affect module builds\, assuming the same user runs 'make' as unzipped the distribution tarball. It will still load those files if the script does "use lib '.'"etc\, since that "." in @​INC is detected as being a different "." from the one perl itself added at startup.

I don't know what analogue methods could be used on win and VMS and regards uid and file ownership. I also don;t know whether ACLs on UNIXy platforms will make a difference.

My second proposal​: since the main issue is *optional* requires\, make it so that if require is is about to load a module using the system's "." in @​INC\, then if require is wrapped in an eval (as determined by scanning the current context stack)\, then croak. I have a PoC private branch which does just this. I made it so that it only dies for a require with a constant arg\, e.g. for

  require Foo​::Bar;   require "t/Foo.pm"

but not for

  require $foo;

With the const restriction\, my PoC branch was able to build and install Moose and its ~45 dependencies.

A third thing to do\, in the middle of the spectrum\, would be to issue a warning each time a module is loaded via the system's ".". This would help people fix up problematic scripts or modules in preparation for moving to a "."-less environment.

In summary the spectrum would be something like​:

1. current situation​: "." always added to @​INC apart from in taint or setuid   mode.

2. As (1)\, but require would refuse to honour the system "." entry in a few   specific cases\, and would croak instead (eval { require }\, wrong file   ownership). An explicit -I. or "use lib '.'" would always be honoured.

3. As (2)\,but warn when the system's "." succeeds;

4. Remove "." from @​INC completely\, but add it back in when we think we're   building a module (Todd's patch)

5. as (4)\, but remove the mechanism to re-add ".". This might be useful for e.g. a specialised secure linux distribution who take their own responsibility for fixing up and building any modules that come bundled with the OS.

I can envisage us releasing 5.24.0 with\, and patching older perls with\, some variant of (2). I don't yet know whether we should include both mechanisms (file ownership and eval detection) or whether one is superior and good enough - I'm swinging towards the ownership option right now\, as it seems very simple. Then post 5.24.0\, blead gets the full works\, selectable by suitable #defines or Configure options\, meaning that 5.26.0 can be built in varying levels of strictness. Maybe this could also be back-ported to maint releases?

Some thoughts about Todd's current branch.

1. Like others\, adding a new\, highly-specific environment variable\, PERL_USE_UNSAFE_INC\, makes me twitchy. How about instead​:

we add a new perl switch\, -J\, which is exactly the same as -I\, except that it *appends* to @​INC rather than prepending. This sounds reasonably general-purpose and useful in its own right.

Then the effect of setting PERL_USE_UNSAFE_INC in MakeMaker etc can be achieved more of less equivalently with :

  $ENV{PERL5OPT} .= ' -J.' if $] >= 5.024000

The main issue is when setting this in places like MakeMaker and the cpan executable\, whether some parts of the build process invoked from there would want to run a sub-process using a different perl (and version) than that currently running? If so the child could croak with "Unrecognized switch​: -J".

2) should perl also special-case Makefile.PL and Build.PL? That is to say\, if the name of the executable perl is about to execute looks like one of those two\, should it add back "."? Otherwise a lot of distributions will fail to build if done manually (rather than via cpan)\, e.g.   $ perl Makefile.PL since things like 'use inc​::Module​::Install' in the script will fail.

3) should the MakeMaker and cpan hacks only set the PERL_USE_UNSAFE_INC / PERL5OPT env var if $Config{inc_dot or whatever} has the right value? Thus a perl built with the fully-fascist option wouldn't be adding -J. to PERL5OPT or whatever.

4) I'm a bit puzzled why PERL_USE_UNSAFE_INC is set in the cpan executable rather than in CPAN.pm?

5) I'm also a bit puzzled why MakeMaker sets PERL_USE_UNSAFE_INC=1 in   MM_Unix.pm when it calls out to perl? Isn't this already   covered by the $ENV{PERL_USE_UNSAFE_INC} = 1 in MakeMaker.pm?

Some other general thoughts.

I think I'm in favour of also patching common dual-lived modules that we bundle with perl which have this issue (Storable\, Encode etc)\, since they may end up getting installed on older perls which don't have any of the workarounds\, thus strengthening (slightly) older installations. But perhaps only after 5.24.0?

But in those modules\, the localisation of @​INC to remove "." should be done in such a way that's it possible for the script to override this with an explicit "use lib '.'" or similar. So for example it might only remove the *last* "."\, e.g. something along the lines of​:

  {   my $seen = 0;   local @​INC =   reverse grep !$seen && $_ eq "." && ($seen=1)\, reverse @​INC;   eval { require Optional​::Module }   }

We should also do an audit of core modules and similarly fix up any that appear to be loading optional modules.

Both Tony and Leon seemed keen to hide the unsafe_inc Configure option "a little deeper"\, e.g. make it done with -Accflags=-D... rather than as a Configure option. I don't yet have a strong opinion either way\, but I would be interested to hear arguments for and against.

Should all scripts that come bundled with the perl core remove "." (if possible)? I think probably yes.

-- My Dad used to say 'always fight fire with fire'\, which is probably why he got thrown out of the fire brigade.

[p5pRT]

From zefram@fysh.org

Dave Mitchell wrote​:

       In the interim\, we'd like ideally to present a spectrum of

build options for perl which vary the balance of the risks of insecurity verses breakage;

Maybe.

Both these proposals assume that the "." added by the perl executable itself during startup is specially flagged in some way\,

It's recognisable by being at the very end of @​INC. Both lib.pm and -I prepend. It would be rather uncomfortable to add any flag other than this.

  when require has located a file using the @​INC search path\, and the

matching @​INC entry was the system's "."​: then before loading it\, check whether the file is owned by either root or the process's euid.

For this general approach\, this semantic is not sufficient. root might well have dangerous code in a file without intending for Perl to load it as an optional module. You need to verify the whole lookup from name to code\, i.e.\, you need to also check ownership of each directory in the chain\, starting from "." and going down to the directory containing the purported module file. It's also good to check permissions of each entity\, and this check alone would rule out loading anything from /tmp.

I'm ambivalent about whether this kind of semantic is a good idea. On one hand\, with my tightened version there are still ways to surprise a user. On the other hand\, tarballs for CPAN dists are liable to be extracted with ownerships from the author's machine (if installing as root)\, so that even the flimsiest ownership checks would prevent loading things bundled in the dist.

                           I also don;t know whether ACLs on UNIXy

platforms will make a difference.

One ought to check the ACL if one is checking permissions. This is not difficult.

My second proposal​: since the main issue is *optional* requires\, make it so that if require is is about to load a module using the system's "." in @​INC\, then if require is wrapped in an eval (as determined by scanning the current context stack)\, then croak.

This strikes me as too much magic. The semantics are surprising\, and break basic code composability.

A third thing to do\, in the middle of the spectrum\, would be to issue a warning each time a module is loaded via the system's ".". This would help people fix up problematic scripts or modules in preparation for moving to a "."-less environment.

That's the *only* thing this option is good for\, and it doesn't seem the kind of thing that one can decide at Perl build time. An environment variable would be a better way of controlling such a warning.

       whether some parts of the build process invoked from there

would want to run a sub-process using a different perl (and version) than that currently running?

No\, that doesn't happen.

2) should perl also special-case Makefile.PL and Build.PL?

No\, that would be way too magical.

But in those modules\, the localisation of @​INC to remove "." should be done in such a way that's it possible for the script to override this with an explicit "use lib '.'" or similar.

Recognising the automatic "." by it being at the end of @​INC satisfies this requirement.

only remove the *last* "."\,

That breaks "use lib '.'" if the automatic "." is not being added\, due to taint or configuration. The criterion has to be to only remove a "." from the very end of @​INC.

Should all scripts that come bundled with the perl core remove "." (if possible)? I think probably yes.

Yes. I think all Perl scripts should.

-zefram

[p5pRT]

From @craigberry

On Wed\, Apr 27\, 2016 at 10​:54 AM\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

First\, a mutation of my earlier idea of not including "." if running as root (which has various issues that people pointed out). Instead\, how about​: when require has located a file using the @​INC search path\, and the matching @​INC entry was the system's "."​: then before loading it\, check whether the file is owned by either root or the process's euid. If neither\, it croaks with some sort of "refuse to load insecure module" error. This should stop most cases of bad files under /tmp\, root running a script in another user's home directory etc. It shouldn't affect module builds\, assuming the same user runs 'make' as unzipped the distribution tarball. It will still load those files if the script does "use lib '.'"etc\, since that "." in @​INC is detected as being a different "." from the one perl itself added at startup.

There would also have to be a writability check\, wouldn't there? If I own a file but the file or the directory it's in have world (or maybe even group) write access\, can't someone rewrite it with malicious content while preserving my ownership?

I don't know what analogue methods could be used on win and VMS and regards uid and file ownership.

Ownership and permissions should map pretty easily\, but "am I root?" is harder. However\, the number of VMS and Windows systems that run vendor-suppled Perl scripts with elevated privileges as part of standard operations (e.g.\, installers) is going to be tiny to nonexistent compared to Unixy systems that do so. Therefore I think a solution that's not completely portable on first iteration is probably acceptable in the short run. Let's put a finger in the dike where the worst breach is likely to occur while we look for places to put the other nine fingers.

I also don;t know whether ACLs on UNIXy platforms will make a difference.

I think ACLs on any OS are going to be a challenge because the question is not "do I have access via ACL?" but "can all of the users and groups who have write access via ACL be trusted not to leave a bomb for me to step on?". Making sure no one at all has write access via ACL may be the safest and simplest thing (not that it's particularly easy to do cross-platform).

[p5pRT]

From @tonycoz

On Wed Apr 27 09​:05​:32 2016\, davem wrote​:

Both Tony and Leon seemed keen to hide the unsafe_inc Configure option "a little deeper"\, e.g. make it done with -Accflags=-D... rather than as a Configure option. I don't yet have a strong opinion either way\, but I would be interested to hear arguments for and against.

My attitude is if this is a security fix\, then lets fix it - don't provide an explicit build-time mechanism to put the '.' back in\, but make the builder require a little knowledge. I don't think perl needs more build variants. As with -DNO_TAINT_SUPPORT\, a perl built with such an option is not supported by p5p.

By simply removing it early in the 5.25 series we provide plenty[1] of time for downstream users to adjust their code to add '.' back in where needed.

The idea of making the '.' we add magic so it doesn't work in a limited set of recognizable security issues strikes me as a rabbit hole we don't want to jump into\, since there's always going to be another issue that we didn't pick up that's going to bite someone.

In terms of whether we provide an environment variable beyond PERL5LIB to add '.' back in...

The simple cases that occurred to me​:

- Module​::Build Makefile.PL scripts and similar\,

- scripts designed to be run from a particular working directory with supporting modules below them

struck me as *less* likely to fail with '.' in at the front of @​INC (as with PERL5LIB)\, which is why I dislike the PERL_USE_UNSAFE_INC=1 solution\, I didn't see a practical case where it matters (probably a failure of imagination).

If we're going to provide a mechanism to add '.' at the end of @​INC controlled by an environment variable\, something like PERL5PUSHLIB=. seems to me to be a more general solution and perhaps useful in some extra cases.

As for 5.24 and earlier... I don't think we can simply remove '.' from @​INC\, and it may end up meaning that any code that wants to work with 5.24 and earlier *and* avoid loading optional modules from the current directory will need to remove that final '.' itself.

This particular bug can bite Imager too\, since it can load file handling modules at runtime.

Tony

[1] we'd want to announce it on blogs.perl.org and possibly other places

[p5pRT]

From @toddr

On Apr 27\, 2016\, at 10​:54 AM\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

I've been doing some more pondering about this "." in @​INC malarkey.

2. As (1)\, but require would refuse to honour the system "." entry in a few specific cases\, and would croak instead (eval { require }\, wrong file ownership). An explicit -I. or "use lib '.'" would always be honoured.

I really like this idea. I don't think the use lib inside a eval is particularly likely so I wouldn't even special case this. This seems like a viable backport option. Once one enters the eval block\, ideally you wouldn't want to strip . from @​INC unless require is going to happen\, which means require needs to somehow know it's inside an eval. If that's easily done\, this could potentially be a small patch that could be easily backported to previous major versions of Perl.

Or to rephrase​: Don't try "." if it's at the end of @​INC and you're in an eval.

3. As (2)\,but warn when the system's "." succeeds;

Meh. This just emits unexpected noise on a legacy version of Perl. The noise could have unexpected side effects. I'm not sure that's desirable.

I can envisage us releasing 5.24.0 with\, and patching older perls with\, some variant of (2). I don't yet know whether we should include both mechanisms (file ownership and eval detection) or whether one is superior and good enough - I'm swinging towards the ownership option right now\, as it seems very simple. Then post 5.24.0\, blead gets the full works\, selectable by suitable #defines or Configure options\, meaning that 5.26.0 can be built in varying levels of strictness. Maybe this could also be back-ported to maint releases?

IMO\, the ownership check is very OS specific and you're going to spend quite a bit of time debugging it after you post the CVE. There's also the question of properly validating the entire directory tree. This isn't really something in Perl's wheel house and introducing it for a backport seems problematic. I would suggest holding this in the wings in case a problem comes up with the eval approach.

Some thoughts about Todd's current branch.

These are good ideas but I'd I'd prefer discussing this in the public ticket since they do not apply to 5.24 and below.

I think I'm in favour of also patching common dual-lived modules that we bundle with perl which have this issue (Storable\, Encode etc)\, since they may end up getting installed on older perls which don't have any of the workarounds\, thus strengthening (slightly) older installations. But perhaps only after 5.24.0?

+1. I think fixing both core modules and patching Perl (if it can be simplified enough) should be done together as part of the initial public disclosure. I think this needs to be done after 5.24.0 goes out.

But in those modules\, the localisation of @​INC to remove "." should be done in such a way that's it possible for the script to override this with an explicit "use lib '.'" or similar. So for example it might only remove the *last* "."\, e.g. something along the lines of​:

{ my $seen = 0; local @​INC = reverse grep !$seen && $_ eq "." && ($seen=1)\, reverse @​INC; eval { require Optional​::Module } }

or eval { local @​INC = @​INC; pop @​INC if($INC[$#INC] eq '.'); require Optional​::Module }

or​: If people really want to overcome the use lib "." problem\, they could do use lib "./" and overcome the above issue. Seriously\, though\, who the heck needs this behavior in production code?

We should also do an audit of core modules and similarly fix up any that appear to be loading optional modules.

+1

This sounds like a viable proposal to me about how you fix the reported problem for legacy versions of Perl.

To summarize​: 1. Patch eval blocks to skip "." at the end of @​INC in use/require/do. 2. Patch modules shipped with Perl that try to load optional modules. 3. Patch perl scripts installed during make install which depend on . in @​INC. 4. Update dual life modules on CPAN.

I realize there is action at a distance / magic going on in this fix. I would argue there always was. I see this as a trade off. Yes\, we're changing the magic. But the magic to load optional modules from . was probably rarely used in production. And the work around to anything this breaks is to fix it in the script by using the optional module prior to loading Storable/Encode/etc.

Todd

[p5pRT]

From @rjbs

* demerphq \demerphq@&#8203;gmail\.com [2016-04-16T09​:57​:07]

I worry that discussing the fix for 5.26 in a public thread is a good idea actually.

I think this thread might as well be in public\, but I leave it to the rest of the team to decide whether they agree with that. I am considering leaving the security list now\, or at least reading its mail a lot less readily.

One thing I wanted to make sure had been considered​: removing "." by default from @​INC will break every Module​::Install-based dist on the CPAN -- over 10% of the CPAN by my last count\, a month ago. Users can't upgrade their local Module​::Install to fix it\, it doesn't work that way.

-- rjbs

[p5pRT]

From @iabyn

On Mon\, May 09\, 2016 at 08​:36​:48AM -0400\, Ricardo Signes wrote​:

* demerphq \demerphq@&#8203;gmail\.com [2016-04-16T09​:57​:07]

I worry that discussing the fix for 5.26 in a public thread is a good idea actually.

I think this thread might as well be in public

Given that the nature of this bug is that (possibly many) UNIX-like admin scripts intended to be run by root can be used to give any non-root user super-user privileges if that script happens to be run while root is chdired to an unsafe directory such as /tmp (as was demonstrated with apt-get)\, and given that (as far as I'm aware) this is not yet generally publicly known\, I think we most definitely should not make this public until full fixes are available.

-- "Emacs isn't a bad OS once you get used to it. It just lacks a decent editor."