Bug#832441: devscripts: CVE-2016-1238 fix

[Dominic Hargreaves]

On Mon, Jul 25, 2016 at 08:41:03PM -0400, James McCoy wrote:
> On Mon, Jul 25, 2016 at 03:48:13PM +0100, Dominic Hargreaves wrote:
> > An update for this package has been released as part of our handling for
> > the issue described below. This fixes an instance of the dynamic module
> > loading vulnerability alluded to.
> > 
> > I attach the patch I applied for jessie; please could you review this
> > and apply something similar for sid?
> 
> Thanks for the notice.
> 
> > From ec54f8919620d6b064f0c61015af553570c2ee3a Mon Sep 17 00:00:00 2001
> > From: Dominic Hargreaves <dom at earth.li>
> > Date: Mon, 25 Jul 2016 10:06:19 +0100
> > Subject: [PATCH 1/2] Remove . from @INC when loading modules dynamically
> >  [CVE-2016-1238]
> > 
> > diff --git a/scripts/desktop2menu.pl b/scripts/desktop2menu.pl
> > index f97551d..92c99f8 100755
> > --- a/scripts/desktop2menu.pl
> > +++ b/scripts/desktop2menu.pl
> > @@ -64,6 +64,7 @@ use File::Basename;
> >  my $progname = basename($0);
> >  
> >  BEGIN {
> > +    pop @INC if $INC[-1] eq '.';
> >      # Load the File::DesktopEntry module safely
> >      eval { require File::DesktopEntry; };
> >      if ($@) {
> 
> I'm curious why only this script was changed.  From a quick search, it
> looks like there are at least a few more that should be changed, if I
> understand the problem properly.
> 
> $ ag --perl '\{ require'
> scripts/desktop2menu.pl:68:    eval { require File::DesktopEntry; };
> scripts/dcontrol.pl:28:    eval { require URI::Escape; };
> scripts/dcontrol.pl:37:    eval { require LWP::UserAgent; };
> scripts/plotchangelog.pl:35:    eval { require Date::Parse; import Date::Parse (); };
> scripts/dscverify.pl:36:    eval { require Digest::MD5; };
> scripts/rmadison.pl:27:    eval { require URI::Escape; };
> scripts/uscan.pl:40:    eval { require LWP::UserAgent; };
> scripts/uscan.pl:59:eval { require LWP::Protocol::https; };
> scripts/grep-excuses.pl:32:    eval { require Term::Size; };
> 
> There are likely a few more not caught by this search, too.

You're right that those should probably be fixed too. The ones we fixed
as part of the DSA were found through fuzzing - ie looking for instances
of accessing modules in cwd when the scripts were run. Since most of
those modules apart from File::DesktopEntry are rather standard, they
would have been loaded successfully from the system path instead.

The DSA fix was never going to be complete, and is only really intended
(at least from the Debian point of view) as a short term improvement
until we can get '.' removed from @INC by default. See also [1].
Upstream, the plan/hope is that this will be done for 5.26[2], but we
definitely want to have this by default in stretch if not jessie via the
sitecustomize.pl route.

At the point that the '.' removal becomes the default these sorts of
patches become mostly irrelevant, and at the point that it becomes
non-configurable except at compile time, they become completely
irrelevant.

Hope that makes things a bit clearer!

Cheers,
Dominic.

[1] <https://lists.debian.org/debian-release/2016/07/msg00456.html>
[2] <https://rt.perl.org/Ticket/Display.html?id=127810>