Bug#801398: [Patch] Replace Dpkg::IPC::spawn with IPC::Run::run

[Sandro Mani]

On 10.10.2015 10:16, Osamu Aoki wrote:
> Hi,
>
> On Fri, Oct 09, 2015 at 07:36:21PM +0200, Salvatore Bonaccorso wrote:
>> Hi,
>>
>> On Fri, Oct 09, 2015 at 05:22:16PM +0200, Sandro Mani wrote:
>>> Some time back licensecheck grew a dependency on Dpkg::IPC [1], which on
>>> Fedora causes the "devscripts-minimal" package (which includes licensecheck)
>>> to pull in dpkg. I'd like to propose the patch below to reduce the
>>> dependency load:
>> [...]
>>
>> If this is changed, one needs to make sure that CVE-2015-5705 /
>> #794365 isn't reintroduced (argument injection vulnerability).
> I understand back-tick is problematic as CVE-2015-5705.  (hmmm ...
> something to keep in mind :-)  Avoiding it was a good idea.
>
> Both Dpkg::IPC and IPC::Run seem to offer similar functionality and
> similar security protection by using the list of strings instead of a
> long shell interpreted command string.  So this change itself looks like
> neutral for the concern raised by Salvatore.(In-depth evaluation may be
> a good idea.  Feed back on this aspect from Sandro is appreciated.)
>
> But before digging that deep for such security feature differences, I
> fail to understand the rationale of switching from Dpkg::IPC to IPC::Run
> for devscripts as presented.
>
> I do not know how OP wishes to use this code on Fedora but if we look at
> the devscripts package as a whole, the OP's claim "grew a dependency"
> does not make sense.  The use of Dpkg::IPC was meant to avoid growing
> dependency.
Ok I see that this is the case on debian. On Fedora, licensecheck ended 
up pulling in the entire dpkg through Dpkg::IPC (which in Fedora is 
packaged as dpkg-perl), and people complained:

https://bodhi.fedoraproject.org/updates/FEDORA-2015-e0237fcd94

On debian it might indeed be the other way round: dpkg is installed 
anyway, so not much changes by having licensecheck depend on Dpkg::IPC.

I'm proposing this patch upstream because of our policy to do so, but if 
it does not make sense for you, I'll just mark the patch as 
non-upstreamable and carry it downstream.


Thanks
Sandro