Mailing List Archive: CVE-2021-36770: Encode.pm loads code from outside expected @INC

CVE-2021-36770: Encode.pm loads code from outside expected @INC

Aug 9, 2021, 5:27 AM

Post #1 of 10 (456 views)

Porters,

I have attached a fix for a bug in Encode, registered as CVE-2021-36770. This bug replaces the contents of @INC with a predictable integer, which is treated as a directory relative to the current working directory, long enough to execute one "require".

The vulnerability was introduced in Encode v3.05, here: dankogai/p5-encode@9c5f5a3 <https://github.com/dankogai/p5-encode/commit/9c5f5a307863b66d> It was shipped with perl v5.32 and v5.34.

A simple proof of concept:

dinah:~/tmp$ perl -MEncode -e0
dinah:~/tmp$ perl -E 'say scalar @INC'
4
dinah:~/tmp$ mkdir -p 4/Encode
dinah:~/tmp$ echo 'print "Something evil here!!\n"' > 4/Encode/ConfigLocal.pm
dinah:~/tmp$ perl -MEncode -e0
Something evil here!!

A new release of Encode should be available from the CPAN today, and will be swiftly integrated into perl5.git. I expect this fix will shortly be available from major distributors of perl. In the meantime, I have applied a patch to the repository.

This bug was reported to perlsec on June 26 by Dom Hargreaves on behalf of Debian, passing on a report from Paul Wise.

--
rjbs

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

Stromeko at nexgo

Aug 9, 2021, 2:03 PM

Post #2 of 10 (456 views)

Permalink

"Ricardo Signes" writes:
> diff --git a/Encode.pm b/Encode.pm
> index a56a999..9691382 100644
> --- a/Encode.pm
> +++ b/Encode.pm
> @@ -65,8 +65,8 @@ require Encode::Config;
> eval {
> local $SIG{__DIE__};
> local $SIG{__WARN__};
> - local @INC = @INC || ();
> - pop @INC if $INC[-1] eq '.';
> + local @INC = @INC;
> + pop @INC if @INC && $INC[-1] eq '.';
> require Encode::ConfigLocal;
> };

Hmm, the original change in 3.05 looks… planted; and since Encode.pm is
XS-ed it gets baked in pretty hard. Since that change precludes any
attempt to use Encode::ConfigLocal as intended, this also shows that the
distribution is missing a test for this functionality and that
practically nobody is using it or it should have been caught much
earlier. :-(

Going back to CVE-2016-1238, why is "." in @INC only removed when it's
at the end (which used to be where it was by default)? Shouldn't it be
sanitized regardless of position, or even more generally any
world-writable components of @INC?

Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

grinnz at gmail

Aug 9, 2021, 3:35 PM

Post #3 of 10 (456 views)

Permalink

On Mon, Aug 9, 2021 at 5:03 PM Achim Gratz <Stromeko@nexgo.de> wrote:

> "Ricardo Signes" writes:
> > diff --git a/Encode.pm b/Encode.pm
> > index a56a999..9691382 100644
> > --- a/Encode.pm
> > +++ b/Encode.pm
> > @@ -65,8 +65,8 @@ require Encode::Config;
> > eval {
> > local $SIG{__DIE__};
> > local $SIG{__WARN__};
> > - local @INC = @INC || ();
> > - pop @INC if $INC[-1] eq '.';
> > + local @INC = @INC;
> > + pop @INC if @INC && $INC[-1] eq '.';
> > require Encode::ConfigLocal;
> > };
>
> Hmm, the original change in 3.05 looks… planted; and since Encode.pm is
> XS-ed it gets baked in pretty hard. Since that change precludes any
> attempt to use Encode::ConfigLocal as intended, this also shows that the
> distribution is missing a test for this functionality and that
> practically nobody is using it or it should have been caught much
> earlier. :-(
>
> Going back to CVE-2016-1238, why is "." in @INC only removed when it's
> at the end (which used to be where it was by default)? Shouldn't it be
> sanitized regardless of position, or even more generally any
> world-writable components of @INC?
>

It's quite different to account for the default behavior of Perl up until
5.26, than to account for anyone's modification of @INC which may have a
good reason (and if you want to protect against that, you must remove any
relative path from @INC, not just '.').

-Dan

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

Stromeko at nexgo

Aug 9, 2021, 10:25 PM

Post #4 of 10 (456 views)

Permalink

Dan Book writes:
> It's quite different to account for the default behavior of Perl up
> until 5.26, than to account for anyone's modification of @INC which
> may have a good reason (and if you want to protect against that, you
> must remove any relative path from @INC, not just '.').

The attack vector doesn't depend on the path being relative.

Looking at enc2xs, I'm wondering if the search path could / should be
restricted to $INC{"Encode.pm"}, which would be safe by default and
explicitly set up to differ from the default by the user otherwise.
While it's possible to put a generated ConfigLocal someplace else after
the fact with the current implementation, enc2xs doesn't seem to support
that scenario consciously.

Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

grinnz at gmail

Aug 9, 2021, 11:40 PM

Post #5 of 10 (456 views)

Permalink

On Tue, Aug 10, 2021 at 1:25 AM ASSI <Stromeko@nexgo.de> wrote:

> Dan Book writes:
> > It's quite different to account for the default behavior of Perl up
> > until 5.26, than to account for anyone's modification of @INC which
> > may have a good reason (and if you want to protect against that, you
> > must remove any relative path from @INC, not just '.').
>
> The attack vector doesn't depend on the path being relative.
>

What do you mean by this? This is the entire reason that the current
working directory in @INC is a vulnerability. Other relative paths are also
treated as relative to the current working directory.

-Dan

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

Stromeko at nexgo

Aug 14, 2021, 1:15 AM

Post #6 of 10 (454 views)

Permalink

"Ricardo Signes" writes:
> I have attached a fix for a bug in Encode, registered as
> CVE-2021-36770. This bug replaces the contents of @INC with a
> predictable integer, which is treated as a directory relative to the
> current working directory, long enough to execute one "require".

I've decided to put a different fix in Cygwin's Perl:

--8<---------------cut here---------------start------------->8---
--- origsrc/perl-5.32.1/cpan/Encode/Encode.pm
+++ src/perl-5.32.1/cpan/Encode/Encode.pm
@@ -65,8 +65,7 @@
eval {
local $SIG{__DIE__};
local $SIG{__WARN__};
- local @INC = @INC || ();
- pop @INC if $INC[-1] eq '.';
+ local @INC = ( substr( $INC{"Encode.pm"}, 0, -length( "/Encode.pm" )) ); # where enc2xs would have installed it
require Encode::ConfigLocal;
};

--8<---------------cut here---------------end--------------->8---

If Encode.pm already got loaded from an unsafe directory this isn't
making anything worse than it already is, otherwise this prevents any
shenanigans with @INC, intended or not.

Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

grinnz at gmail

Aug 14, 2021, 10:18 AM

Post #7 of 10 (454 views)

Permalink

On Sat, Aug 14, 2021 at 4:15 AM Achim Gratz <Stromeko@nexgo.de> wrote:

> "Ricardo Signes" writes:
> > I have attached a fix for a bug in Encode, registered as
> > CVE-2021-36770. This bug replaces the contents of @INC with a
> > predictable integer, which is treated as a directory relative to the
> > current working directory, long enough to execute one "require".
>
> I've decided to put a different fix in Cygwin's Perl:
>
> --8<---------------cut here---------------start------------->8---
> --- origsrc/perl-5.32.1/cpan/Encode/Encode.pm
> +++ src/perl-5.32.1/cpan/Encode/Encode.pm
> @@ -65,8 +65,7 @@
> eval {
> local $SIG{__DIE__};
> local $SIG{__WARN__};
> - local @INC = @INC || ();
> - pop @INC if $INC[-1] eq '.';
> + local @INC = ( substr( $INC{"Encode.pm"}, 0, -length( "/Encode.pm" ))
> ); # where enc2xs would have installed it
> require Encode::ConfigLocal;
> };
>
>
> --8<---------------cut here---------------end--------------->8---
>

A less fragile version of this would be:

require File::Basename;
local @INC = File::Basename::dirname($INC{'Encode.pm'});

-Dan

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

grinnz at gmail

Aug 14, 2021, 10:20 AM

Post #8 of 10 (454 views)

Permalink

On Sat, Aug 14, 2021 at 1:18 PM Dan Book <grinnz@gmail.com> wrote:

> On Sat, Aug 14, 2021 at 4:15 AM Achim Gratz <Stromeko@nexgo.de> wrote:
>
>> "Ricardo Signes" writes:
>> > I have attached a fix for a bug in Encode, registered as
>> > CVE-2021-36770. This bug replaces the contents of @INC with a
>> > predictable integer, which is treated as a directory relative to the
>> > current working directory, long enough to execute one "require".
>>
>> I've decided to put a different fix in Cygwin's Perl:
>>
>> --8<---------------cut here---------------start------------->8---
>> --- origsrc/perl-5.32.1/cpan/Encode/Encode.pm
>> +++ src/perl-5.32.1/cpan/Encode/Encode.pm
>> @@ -65,8 +65,7 @@
>> eval {
>> local $SIG{__DIE__};
>> local $SIG{__WARN__};
>> - local @INC = @INC || ();
>> - pop @INC if $INC[-1] eq '.';
>> + local @INC = ( substr( $INC{"Encode.pm"}, 0, -length( "/Encode.pm"
>> )) ); # where enc2xs would have installed it
>> require Encode::ConfigLocal;
>> };
>>
>>
>> --8<---------------cut here---------------end--------------->8---
>>
>
> A less fragile version of this would be:
>
> require File::Basename;
> local @INC = File::Basename::dirname($INC{'Encode.pm'});
>

And probably should be using __FILE__ instead of the %INC entry since this
*is* Encode.pm.

-Dan

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

Stromeko at nexgo

Aug 14, 2021, 10:24 AM

Post #9 of 10 (454 views)

Permalink

Dan Book writes:
> A less fragile version of this would be:
>
> require File::Basename;
> local @INC = File::Basename::dirname($INC{'Encode.pm'});

Good idea, plus it would actually work, since this module is in the
perl_base package, so always available. I'll do that in the next
release.

Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

Re: CVE-2021-36770: Encode.pm loads code from outside expected @INC [ In reply to ]

fawaka at gmail

Aug 14, 2021, 10:50 AM

Post #10 of 10 (454 views)

Permalink

On Sat, Aug 14, 2021 at 7:21 PM Dan Book <grinnz@gmail.com> wrote:

> On Sat, Aug 14, 2021 at 1:18 PM Dan Book <grinnz@gmail.com> wrote:
>
>> On Sat, Aug 14, 2021 at 4:15 AM Achim Gratz <Stromeko@nexgo.de> wrote:
>>
>>> "Ricardo Signes" writes:
>>> > I have attached a fix for a bug in Encode, registered as
>>> > CVE-2021-36770. This bug replaces the contents of @INC with a
>>> > predictable integer, which is treated as a directory relative to the
>>> > current working directory, long enough to execute one "require".
>>>
>>> I've decided to put a different fix in Cygwin's Perl:
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> --- origsrc/perl-5.32.1/cpan/Encode/Encode.pm
>>> +++ src/perl-5.32.1/cpan/Encode/Encode.pm
>>> @@ -65,8 +65,7 @@
>>> eval {
>>> local $SIG{__DIE__};
>>> local $SIG{__WARN__};
>>> - local @INC = @INC || ();
>>> - pop @INC if $INC[-1] eq '.';
>>> + local @INC = ( substr( $INC{"Encode.pm"}, 0, -length( "/Encode.pm"
>>> )) ); # where enc2xs would have installed it
>>> require Encode::ConfigLocal;
>>> };
>>>
>>>
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>
>> A less fragile version of this would be:
>>
>> require File::Basename;
>> local @INC = File::Basename::dirname($INC{'Encode.pm'});
>>
>
> And probably should be using __FILE__ instead of the %INC entry since this
> *is* Encode.pm.
>

If you really want to go that way, you might as well give require the
actual path

Leon

Mailing List Archive

Mailing List Archive

Attached Files: