Re: Bug#878901: dh-make-perl: FTBFS with dpkg >= 1.19: "Insecure dependency in eval while running with -T switch"

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Bug#878901: dh-make-perl: FTBFS with dpkg >= 1.19: "Insecure dependency in eval while running with -T switch"

Niko Tyni-3
On Tue, Oct 17, 2017 at 05:44:26PM +0200, gregor herrmann wrote:
> Package: dh-make-perl
> Version: 0.95
> Severity: serious
> Tags: buster sid
> Justification: fails to build from source

> As first seen on ci.debian.net, dh-make-perl's test suite fails with
> libdpkg-perl 1.19.0 and 1.19.0.1:
>
> Insecure dependency in eval while running with -T switch at /usr/share/perl5/Dpkg/Vendor.pm line 164.

> The -T seems to come from t/debian-version.t itself; no idea yet why
> it is a problem now and why it's used here in the first place.

It looks like Dpkg::Vendor::get_vendor_info() contents have become
tainted, probably due to changes in Dpkg::Control::HashCore. It used to
dig the values out with regexp captures but now uses split.

 https://anonscm.debian.org/cgit/dpkg/dpkg.git/commit/?h=sid&id=9e5e03e9a6ddf74bb22ffc5ea8794a14a592d6b6

A test case is

  perl -T -MDpkg::Vendor=get_vendor_info -MScalar::Util=tainted -e 'die if tainted get_vendor_info()->{Vendor}'

which dies on libdpkg-perl 1.19.0.1 but not 1.18.24.

I don't know if the earlier untainting was accidental or intended.
Copying the dpkg maintainers.

Hope this helps a bit,
--
Niko

Reply | Threaded
Open this post in threaded view
|

Re: Bug#878901: dh-make-perl: FTBFS with dpkg >= 1.19: "Insecure dependency in eval while running with -T switch"

Guillem Jover
Hi!

On Tue, 2017-10-17 at 19:48:07 +0300, Niko Tyni wrote:

> On Tue, Oct 17, 2017 at 05:44:26PM +0200, gregor herrmann wrote:
> > Package: dh-make-perl
> > Version: 0.95
> > Severity: serious
> > Tags: buster sid
> > Justification: fails to build from source
>
> > As first seen on ci.debian.net, dh-make-perl's test suite fails with
> > libdpkg-perl 1.19.0 and 1.19.0.1:
> >
> > Insecure dependency in eval while running with -T switch at /usr/share/perl5/Dpkg/Vendor.pm line 164.
>
> > The -T seems to come from t/debian-version.t itself; no idea yet why
> > it is a problem now and why it's used here in the first place.

> It looks like Dpkg::Vendor::get_vendor_info() contents have become
> tainted, probably due to changes in Dpkg::Control::HashCore. It used to
> dig the values out with regexp captures but now uses split.
>
>  https://anonscm.debian.org/cgit/dpkg/dpkg.git/commit/?h=sid&id=9e5e03e9a6ddf74bb22ffc5ea8794a14a592d6b6
>
> A test case is
>
>   perl -T -MDpkg::Vendor=get_vendor_info -MScalar::Util=tainted -e 'die if tainted get_vendor_info()->{Vendor}'
>
> which dies on libdpkg-perl 1.19.0.1 but not 1.18.24.
>
> I don't know if the earlier untainting was accidental or intended.
> Copying the dpkg maintainers.

TBH, I was not aware that anyone was running Dpkg modules in taint
mode. And I don't think anyone has writen code for the modules with
that in mind. I'm not sure either how much of it is taint clean, for
example.

If people are really running this code in taint mode, I'm willing to
discuss which parts of the API would make sense to cover or not, and
what tradeoffs related to performance to take, etc.

Thanks,
Guillem

Reply | Threaded
Open this post in threaded view
|

Re: Bug#878901: dh-make-perl: FTBFS with dpkg >= 1.19: "Insecure dependency in eval while running with -T switch"

Damyan Ivanov-5
-=| Guillem Jover, 17.10.2017 22:16:31 +0200 |=-

> On Tue, 2017-10-17 at 19:48:07 +0300, Niko Tyni wrote:
> > It looks like Dpkg::Vendor::get_vendor_info() contents have become
> > tainted, probably due to changes in Dpkg::Control::HashCore. It used to
> > dig the values out with regexp captures but now uses split.
> >
> >  https://anonscm.debian.org/cgit/dpkg/dpkg.git/commit/?h=sid&id=9e5e03e9a6ddf74bb22ffc5ea8794a14a592d6b6
> >
> > A test case is
> >
> >   perl -T -MDpkg::Vendor=get_vendor_info -MScalar::Util=tainted -e 'die if tainted get_vendor_info()->{Vendor}'
> >
> > which dies on libdpkg-perl 1.19.0.1 but not 1.18.24.
> >
> > I don't know if the earlier untainting was accidental or intended.
> > Copying the dpkg maintainers.
>
> TBH, I was not aware that anyone was running Dpkg modules in taint
> mode. And I don't think anyone has writen code for the modules with
> that in mind. I'm not sure either how much of it is taint clean, for
> example.
>
> If people are really running this code in taint mode, I'm willing to
> discuss which parts of the API would make sense to cover or not, and
> what tradeoffs related to performance to take, etc.

I think that using taint mode wasn't justified in that one case, so
imposing that on Dpkg::* would not be necessary.

During discussion, Matt S. Trout suggested on IRC that the check for
a valid package name is better written as $input =~
/\A([A-Za-z]\w*(?:::\w+)*)\Z/. If no hierarchy is possible, then
/\A([A-Za-z]\w*/ would be enough.

(Perhaps this belongs to the place where $name is interpreted as
a module name, not when parsing generic label:value lines).

That may be considered nitpicking, especially without a view on the
big picture, but I'd rather mention it here in case it is useful.

-- dam

Reply | Threaded
Open this post in threaded view
|

Re: Bug#878901: dh-make-perl: FTBFS with dpkg >= 1.19: "Insecure dependency in eval while running with -T switch"

Damyan Ivanov-5
-=| Damyan Ivanov, 18.10.2017 20:20:16 +0000 |=-
> During discussion, Matt S. Trout suggested on IRC that the check for
> a valid package name is better written as $input =~
> /\A([A-Za-z]\w*(?:::\w+)*)\Z/. If no hierarchy is possible, then
> /\A([A-Za-z]\w*/ would be enough.

I forgot an additional suggestion from Matt for replacing a big string
eval with a much smaller one.

Here it is:

## old code
        eval qq{
            pop \@INC if \$INC[-1] eq '.';
            require Dpkg::Vendor::$name;
            \$obj = Dpkg::Vendor::$name->new();
        };
        unless ($@) {
            $OBJECT_CACHE{$vendor} = $obj;
            return $obj;
        }

## new code
        pop @INC if $INC[-1] eq '.';
        (my $path = my $class = "Dpkg::Vendor::${name}") =~ s/\::/\//g
        my $obj = eval { require "${path}.pm"; $class->new };
        return $OBJECT_CACHE{$vendor} = $obj if $obj;


Cheers,
    dam

Reply | Threaded
Open this post in threaded view
|

Re: Bug#878901: dh-make-perl: FTBFS with dpkg >= 1.19: "Insecure dependency in eval while running with -T switch"

Christoph Biedl-7
In reply to this post by Guillem Jover
Guillem Jover wrote...

> TBH, I was not aware that anyone was running Dpkg modules in taint
> mode.

Well, I do as well, in some private code. I can and probably will change
that, though.

> If people are really running this code in taint mode, I'm willing to
> discuss which parts of the API would make sense to cover or not, and
> what tradeoffs related to performance to take, etc.

Honestly, I cannot decide neither on this particular case nor in the
general. On the one hand, given the fact a author of a code library
never knows where and how people will actually use it, it's prudent to
play safe and write all libraries so they run in taint mode as well.
On the other hand, certainly a lot of existing Perl libraries do not
follow that principle anyway and you might consider that approach, while
desirable, not feasible. Also, there might be a readability tradeoff
which I consider even worse than performance. (I could benchmark the
cost of "use strict" and "use warnings" one day, I bet they're worse.)

It's one of the many things where I consider Perl beyond repair. The
language is fairly sloppy but today safeguards like taint mode should
be turned on by default to mitigate at least the worst issues that
exist. But nobody is willing to fix the massive breakage that would
happen then, so it's not going to happen.

ΒΆΒΆ

    Christoph

signature.asc (836 bytes) Download Attachment