Control: tag -1 + patch

On Sun, 17 Mar 2019 23:27:59 +0100, Moritz Mühlenhoff wrote:

> severity 842893 grave
> thanks
> 
> On Wed, Nov 02, 2016 at 07:07:13AM +0100, Salvatore Bonaccorso wrote:
> > Source: libxml-twig-perl
> > Version: 1:3.39-1
> > Severity: important
> > Tags: security upstream
> > Forwarded: https://rt.cpan.org/Public/Bug/Display.html?id=118097
> > 
> > Hi
> > 
> > XML::Twig's expand_external_ents fails to work as documented, see the
> > upstream bug report for details and as well reported to Red Hat at
> > https://bugzilla.redhat.com/show_bug.cgi?id=1379553 .
> > 
> > There is though still no upstream fix for it.
> 
> This is unfixed for long time and I don't think this should slip
> unfixed into another stable release. Especially given that the fix is
> simple; 3.50 introduced the no_xxe option, so all that needs to be
> done is to fix the misleading docs for expand_external_ents (and
> refer to use no_xxe instead there).

I had a look at this bug yesterday evening, and I'm attaching a
potential patch.

Moritz, I'm not completely sure I understand which changes to the
docs you imagined, but I've added the following now:

+B<WARNING>: setting expand_external_ents to 0 or -1 currently doesn't work
+as expected; cf. L<https://rt.cpan.org/Public/Bug/Display.html?id=118097>.
+To completelty turn off expanding external entities use C<no_xxe>.
+
+=item no_xxe
+
+If this argument is set to a true value, expanding of external entities is
+turned off.
+

Additionally (actually before changing the docs, in the hope to fix
the actual bug which I didn't manage to do), I added a test for both
expand_external_ents and no_xxe, based on the reproducer in the CPAN
RT ticket. Doesn't help alot now but might be beneficial for the
future. Not sure what the release team thinks about it but it doesn't
affect the binary package.

In general, if we go ahead with something like this, I'm not sure if
we should really close this bug; the issue is mitigated by using and
documenting no_xxe but the expand_external_ents option is still buggy.
[0]. 


Cheers,
gregor

[0] This is also the last comment at
https://bugzilla.redhat.com/show_bug.cgi?id=1379553

-- 
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   NP: Tracy Chapman: Why
diff -Nru libxml-twig-perl-3.50/debian/changelog libxml-twig-perl-3.50/debian/changelog
--- libxml-twig-perl-3.50/debian/changelog	2016-08-04 18:52:02.000000000 +0200
+++ libxml-twig-perl-3.50/debian/changelog	2019-03-30 17:36:55.000000000 +0100
@@ -1,3 +1,15 @@
+libxml-twig-perl (1:3.50-1.1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * Fix "CVE-2016-9180: expand_external_ents fails to work as
+    documented": add patch CVE-2016-9180.patch:
+    - update documentation about expand_external_ents and no_xxe
+    - add test for expand_external_ents and no_xxe
+    Additionally add build dependency on libtest-exception-perl.
+    (Closes: #842893)
+
+ -- gregor herrmann <gre...@debian.org>  Sat, 30 Mar 2019 17:36:55 +0100
+
 libxml-twig-perl (1:3.50-1) unstable; urgency=medium
 
   * New upstream release.
diff -Nru libxml-twig-perl-3.50/debian/control libxml-twig-perl-3.50/debian/control
--- libxml-twig-perl-3.50/debian/control	2016-08-04 18:52:02.000000000 +0200
+++ libxml-twig-perl-3.50/debian/control	2019-03-30 17:36:55.000000000 +0100
@@ -3,7 +3,7 @@
 Priority: optional
 Maintainer: Bart Martens <ba...@debian.org>
 Standards-Version: 3.9.8
-Build-Depends-Indep: perl (>= 5.6.0-16), libxml-parser-perl, libunicode-map8-perl, libunicode-string-perl, libtie-ixhash-perl, libxml-xpathengine-perl | libxml-xpath-perl, libtest-pod-perl, libtest-pod-coverage-perl (>= 1.00), libxml-handler-yawriter-perl, libxml-sax-machines-perl, libxml-simple-perl, libyaml-perl
+Build-Depends-Indep: perl (>= 5.6.0-16), libxml-parser-perl, libunicode-map8-perl, libunicode-string-perl, libtie-ixhash-perl, libxml-xpathengine-perl | libxml-xpath-perl, libtest-pod-perl, libtest-pod-coverage-perl (>= 1.00), libxml-handler-yawriter-perl, libxml-sax-machines-perl, libxml-simple-perl, libyaml-perl, libtest-exception-perl
 Build-Depends: debhelper (>= 8.0.0), expat
 Homepage: http://www.xmltwig.org/
 
diff -Nru libxml-twig-perl-3.50/debian/patches/CVE-2016-9180.patch libxml-twig-perl-3.50/debian/patches/CVE-2016-9180.patch
--- libxml-twig-perl-3.50/debian/patches/CVE-2016-9180.patch	1970-01-01 01:00:00.000000000 +0100
+++ libxml-twig-perl-3.50/debian/patches/CVE-2016-9180.patch	2019-03-30 17:36:55.000000000 +0100
@@ -0,0 +1,85 @@
+Description: Update documentation for XML::Twig.
+ Mention problems with expand_external_ents and add
+ information about new no_xxe argument.
+ .
+ Additionally add tests for both expand_external_ents and no_xxe.
+Origin: vendor
+Bug: https://rt.cpan.org/Public/Bug/Display.html?id=118097
+Bug-Debian: https://bugs.debian.org/842893
+Author: gregor herrmann <gre...@debian.org>
+Last-Update: 2019-03-30
+
+--- a/Twig_pm.slow
++++ b/Twig_pm.slow
+@@ -10454,6 +10454,15 @@
+ pubid => <pubid> }). Yes, this is a bit of a hack, but it's useful in some
+ cases.  
+ 
++B<WARNING>: setting expand_external_ents to 0 or -1 currently doesn't work
++as expected; cf. L<https://rt.cpan.org/Public/Bug/Display.html?id=118097>.
++To completelty turn off expanding external entities use C<no_xxe>.
++
++=item no_xxe
++
++If this argument is set to a true value, expanding of external entities is
++turned off.
++
+ =item load_DTD
+ 
+ If this argument is set to a true value, C<parse> or C<parsefile> on the twig
+--- /dev/null
++++ b/t/CVE-2016-9180.t
+@@ -0,0 +1,41 @@
++#!/usr/bin/perl
++
++use strict;
++use warnings;
++use Test::More;
++use Test::Exception;
++
++BEGIN { use_ok('XML::Twig'); }
++
++my $twig = XML::Twig->new( expand_external_ents => 1 );
++$twig->parsefile('t/CVE-2016-9180.xml');
++my $result = $twig->sprint;
++like( $result, qr/Boom/, 'external entity expanded (expand_external_ents 1)' );
++
++TODO: {
++    local $TODO = 'This test currently fails: https://rt.cpan.org/Public/Bug/Display.html?id=118097';
++
++$twig = XML::Twig->new( expand_external_ents => 0 );
++$twig->parsefile('t/CVE-2016-9180.xml');
++$result = $twig->sprint;
++unlike( $result, qr/Boom/,
++    'external entity not expanded (expand_external_ents 0)' );
++
++$twig = XML::Twig->new( expand_external_ents => -1 );
++$twig->parsefile('t/CVE-2016-9180.xml');
++$result = $twig->sprint;
++unlike( $result, qr/Boom/,
++    'external entity not expanded and no fail (expand_external_ents -1)' );
++
++}
++
++$twig = XML::Twig->new( no_xxe => 1 );
++throws_ok { $twig->parsefile('t/CVE-2016-9180.xml') } qr/cannot expand &xxe;/,
++    'external entity not expanded (no_xxe 1)';
++
++$twig = XML::Twig->new( no_xxe => 0 );
++$twig->parsefile('t/CVE-2016-9180.xml');
++$result = $twig->sprint;
++like( $result, qr/Boom/, 'external entity expanded (no_xxe 0)' );
++
++done_testing();
+--- /dev/null
++++ b/t/CVE-2016-9180.txt
+@@ -0,0 +1 @@
++Boom
+--- /dev/null
++++ b/t/CVE-2016-9180.xml
+@@ -0,0 +1,5 @@
++<?xml version="1.0"?>
++<!DOCTYPE foo [
++	<!ENTITY xxe PUBLIC "bar" "CVE-2016-9180.txt">
++]>
++<root>&xxe;</root>
diff -Nru libxml-twig-perl-3.50/debian/patches/series libxml-twig-perl-3.50/debian/patches/series
--- libxml-twig-perl-3.50/debian/patches/series	2014-01-06 08:46:06.000000000 +0100
+++ libxml-twig-perl-3.50/debian/patches/series	2019-03-30 17:36:55.000000000 +0100
@@ -1,3 +1,4 @@
 03_cosmetics.diff
 06_spelling.diff
 07_691028.diff
+CVE-2016-9180.patch

Attachment: signature.asc
Description: Digital Signature

Reply via email to