atrodo / App-MechaCPAN

Mechanize the installation of CPAN things
Other
2 stars 2 forks source link

Should set PERL_USE_UNSAFE_INC when running configure, build, etc commands #3

Closed haarg closed 7 years ago

haarg commented 7 years ago

When running Makefile.PL, make, make test, make install, and analogous Build.PL steps, the PERL_USE_UNSAFE_INC should be set to 1, unless it has already been set externally.

Future versions of perl will not include . in @INC by default, which breaks many module installs. For example, anything using Module::Install expects use inc::Module::Install; to be able to find the file via the current directory. If the PERL_USE_UNSAFE_INC environment variable is set, perl will include . in @INC, allowing them to work. This change in perl is being made as part of fixing CVE-2016-1238.

toddr commented 7 years ago

Ideally you need to do the following before doing build/test/install

local $ENV{PERL_USE_UNSAFE_INC} = exists $ENV{PERL_USE_UNSAFE_INC} ? $ENV{PERL_USE_UNSAFE_INC} : 1;

Note the policy of other clients that use this has been to not alter the environment variable when it's already set.

atrodo commented 7 years ago

@toddr Is there a downside to always setting $ENV{PERL_USE_UNSAFE_INC}? Is there a compelling reason why someone would want $ENV{PERL_USE_UNSAFE_INC} to be off during module install?

toddr commented 7 years ago

We wanted the script executor to have the option of setting PERL_USE_UNSAFE_INC=whatever and have it not overridden by the script.

haarg commented 7 years ago

We eventually want to remove the need for PERL_USE_UNSAFE_INC by fixing the dists so they don't need it. Part of being able to fix them is being able to easily test them with the variable set false.

toddr commented 7 years ago

You also don't want to set this globally since anything other than build/test could open up an unexpected security issue.

atrodo commented 7 years ago

My natural inclination is to just set it, but the ability to test it not being set makes a lot of sense, so I'll go that route.

atrodo commented 7 years ago

I've added code that will set or honor PERL_USE_UNSAFE_INC. I plan on doing a release this evening if everything continues to look good by then.

haarg commented 7 years ago

PERL_USE_UNSAFE_INC also needs to be set during the configure phase, when running Makefile.PL or Build.PL.

atrodo commented 7 years ago

That is a good point, I'm not entirely sure what I was thinking. I made a new commit ( d7fecfd ) that moved the setting of that environment variable to the beginning of the Install module's process. I'm going to think it over a little bit more and if I don't think of any other gotcha's will release the new version.

atrodo commented 7 years ago

I've released App::MechaCPAN v0.15 with the change to default PERL_USE_UNSAFE_INC to 1 when configuring, building, testing and installing a module.

toddr commented 7 years ago

Thanks for your help!