unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Expat regression fix for master branch
@ 2016-09-12 21:35 Leo Famulari
  2016-09-13 21:54 ` Ludovic Courtès
  2016-09-25 23:18 ` Leo Famulari
  0 siblings, 2 replies; 6+ messages in thread
From: Leo Famulari @ 2016-09-12 21:35 UTC (permalink / raw)
  To: guix-devel


[-- Attachment #1.1: Type: text/plain, Size: 369 bytes --]

This patch applies an upstream patch for a regression caused by the fix 
for CVE-2016-0718.

Apparently, the bug only manifests when building with -DXML_UNICODE,
which I don't think our package does.

Bug report:
https://sourceforge.net/p/expat/bugs/539/

Source of patch:
https://sourceforge.net/p/expat/code_git/ci/af507cef2c93cb8d40062a0abe43a4f4e9158fb2

[-- Attachment #1.2: 0001-gnu-expat-Fix-regression-caused-by-fix-for-CVE-2016-.patch --]
[-- Type: text/plain, Size: 4008 bytes --]

From 14abba35d1b8c455ce597b5c0eed4bcf87499e6a Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Mon, 12 Sep 2016 16:54:45 -0400
Subject: [PATCH] gnu: expat: Fix regression caused by fix for CVE-2016-0718.

* gnu/packages/xml.scm (expat)[replacement]: New field.
(expat/fixed): New variable.
* gnu/packages/patches/expat-CVE-2016-0718-fix-regression.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
 gnu/local.mk                                       |  1 +
 .../expat-CVE-2016-0718-fix-regression.patch       | 35 ++++++++++++++++++++++
 gnu/packages/xml.scm                               | 12 ++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 gnu/packages/patches/expat-CVE-2016-0718-fix-regression.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 5714009..6e1c97c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -501,6 +501,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/expat-CVE-2012-6702-and-CVE-2016-5300.patch	\
   %D%/packages/patches/expat-CVE-2015-1283-refix.patch		\
   %D%/packages/patches/expat-CVE-2016-0718.patch		\
+  %D%/packages/patches/expat-CVE-2016-0718-fix-regression.patch	\
   %D%/packages/patches/fastcap-mulGlobal.patch			\
   %D%/packages/patches/fastcap-mulSetup.patch			\
   %D%/packages/patches/fasthenry-spAllocate.patch		\
diff --git a/gnu/packages/patches/expat-CVE-2016-0718-fix-regression.patch b/gnu/packages/patches/expat-CVE-2016-0718-fix-regression.patch
new file mode 100644
index 0000000..b489401
--- /dev/null
+++ b/gnu/packages/patches/expat-CVE-2016-0718-fix-regression.patch
@@ -0,0 +1,35 @@
+Fix regression caused by fix for CVE-2016-0718 when building with -DXML_UNICODE.
+
+Discussion:
+
+https://sourceforge.net/p/expat/bugs/539/
+
+Patch copied from upstream source repository:
+
+https://sourceforge.net/p/expat/code_git/ci/af507cef2c93cb8d40062a0abe43a4f4e9158fb2/
+
+From af507cef2c93cb8d40062a0abe43a4f4e9158fb2 Mon Sep 17 00:00:00 2001
+From: Sebastian Pipping <sebastian@pipping.org>
+Date: Sun, 17 Jul 2016 20:22:29 +0200
+Subject: [PATCH 1/2] Fix regression bug #539 (needs -DXML_UNICODE)
+
+Thanks to Andy Wang and Karl Waclawek!
+---
+ expat/lib/xmlparse.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
+index b308e67..0d5dd7b 100644
+--- a/lib/xmlparse.c
++++ b/lib/xmlparse.c
+@@ -2468,7 +2468,7 @@ doContent(XML_Parser parser,
+                        &fromPtr, rawNameEnd,
+                        (ICHAR **)&toPtr, (ICHAR *)tag->bufEnd - 1);
+             convLen = (int)(toPtr - (XML_Char *)tag->buf);
+-            if ((convert_res == XML_CONVERT_COMPLETED) || (convert_res == XML_CONVERT_INPUT_INCOMPLETE)) {
++            if ((fromPtr >= rawNameEnd) || (convert_res == XML_CONVERT_INPUT_INCOMPLETE)) {
+               tag->name.strLen = convLen;
+               break;
+             }
+-- 
+2.10.0
diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index 8e0fe01..9dcedee 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -50,6 +50,7 @@
 (define-public expat
   (package
     (name "expat")
+    (replacement expat/fixed)
     (version "2.1.1")
     (source (origin
              (method url-fetch)
@@ -70,6 +71,17 @@ stream-oriented parser in which an application registers handlers for
 things the parser might find in the XML document (like start tags).")
     (license license:expat)))
 
+(define expat/fixed
+  (package
+    (inherit expat)
+    (source (origin
+              (inherit (package-source expat))
+              (patches (search-patches
+                         "expat-CVE-2012-6702-and-CVE-2016-5300.patch"
+                         "expat-CVE-2015-1283-refix.patch"
+                         "expat-CVE-2016-0718.patch"
+                         "expat-CVE-2016-0718-fix-regression.patch"))))))
+
 (define-public libxml2
   (package
     (name "libxml2")
-- 
2.10.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Expat regression fix for master branch
  2016-09-12 21:35 Expat regression fix for master branch Leo Famulari
@ 2016-09-13 21:54 ` Ludovic Courtès
  2016-09-14 18:14   ` Leo Famulari
  2016-09-25 23:18 ` Leo Famulari
  1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2016-09-13 21:54 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

Hello,

Leo Famulari <leo@famulari.name> skribis:

> This patch applies an upstream patch for a regression caused by the fix 
> for CVE-2016-0718.
>
> Apparently, the bug only manifests when building with -DXML_UNICODE,
> which I don't think our package does.

I rebuilt it with --check and can confirm that XML_UNICODE is not
defined.

‘README’ mentions it:

--8<---------------cut here---------------start------------->8---
If you are interested in building Expat to provide document
information in UTF-16 encoding rather than the default UTF-8, follow
these instructions (after having run "make distclean"):

        1. For UTF-16 output as unsigned short (and version/error
           strings as char), run:

               ./configure CPPFLAGS=-DXML_UNICODE
--8<---------------cut here---------------end--------------->8---

It’s enough to have the patch in ‘core-updates’ only, but I could go
either way.  WDYT?

Ludo’.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Expat regression fix for master branch
  2016-09-13 21:54 ` Ludovic Courtès
@ 2016-09-14 18:14   ` Leo Famulari
  0 siblings, 0 replies; 6+ messages in thread
From: Leo Famulari @ 2016-09-14 18:14 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

On Tue, Sep 13, 2016 at 11:54:09PM +0200, Ludovic Courtès wrote:
> It’s enough to have the patch in ‘core-updates’ only, but I could go
> either way.  WDYT?

I could go either way, too. It depends on how much we want to support
use of Guix outside of our package tree.

At least we have made the issue public here on the mailing list, and
provided a patch for whoever needs it.

Does anyone else have an opinion about this?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Expat regression fix for master branch
  2016-09-12 21:35 Expat regression fix for master branch Leo Famulari
  2016-09-13 21:54 ` Ludovic Courtès
@ 2016-09-25 23:18 ` Leo Famulari
  2016-09-26  8:21   ` Efraim Flashner
  1 sibling, 1 reply; 6+ messages in thread
From: Leo Famulari @ 2016-09-25 23:18 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 648 bytes --]

On Mon, Sep 12, 2016 at 05:35:15PM -0400, Leo Famulari wrote:
> This patch applies an upstream patch for a regression caused by the fix 
> for CVE-2016-0718.
> 
> Apparently, the bug only manifests when building with -DXML_UNICODE,
> which I don't think our package does.

Sebastian Pipping (the Expat maintainer) contacted me to recommend that
we apply the patch on the master branch.

He says that the faulty code path can be reached even when XML_UNICODE
is not defined. Apparently, building with -DXML_UNICODE merely makes it
easier to reach the faulty code.

I think we should take Sebastian's advice. What does everyone think?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Expat regression fix for master branch
  2016-09-25 23:18 ` Leo Famulari
@ 2016-09-26  8:21   ` Efraim Flashner
  2016-09-27 17:50     ` Leo Famulari
  0 siblings, 1 reply; 6+ messages in thread
From: Efraim Flashner @ 2016-09-26  8:21 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On Sun, Sep 25, 2016 at 07:18:11PM -0400, Leo Famulari wrote:
> On Mon, Sep 12, 2016 at 05:35:15PM -0400, Leo Famulari wrote:
> > This patch applies an upstream patch for a regression caused by the fix 
> > for CVE-2016-0718.
> > 
> > Apparently, the bug only manifests when building with -DXML_UNICODE,
> > which I don't think our package does.
> 
> Sebastian Pipping (the Expat maintainer) contacted me to recommend that
> we apply the patch on the master branch.
> 
> He says that the faulty code path can be reached even when XML_UNICODE
> is not defined. Apparently, building with -DXML_UNICODE merely makes it
> easier to reach the faulty code.
> 
> I think we should take Sebastian's advice. What does everyone think?


Seems to me that as the Expat maintainer he would know best.


-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Expat regression fix for master branch
  2016-09-26  8:21   ` Efraim Flashner
@ 2016-09-27 17:50     ` Leo Famulari
  0 siblings, 0 replies; 6+ messages in thread
From: Leo Famulari @ 2016-09-27 17:50 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 978 bytes --]

On Mon, Sep 26, 2016 at 11:21:07AM +0300, Efraim Flashner wrote:
> On Sun, Sep 25, 2016 at 07:18:11PM -0400, Leo Famulari wrote:
> > On Mon, Sep 12, 2016 at 05:35:15PM -0400, Leo Famulari wrote:
> > > This patch applies an upstream patch for a regression caused by the fix 
> > > for CVE-2016-0718.
> > > 
> > > Apparently, the bug only manifests when building with -DXML_UNICODE,
> > > which I don't think our package does.
> > 
> > Sebastian Pipping (the Expat maintainer) contacted me to recommend that
> > we apply the patch on the master branch.
> > 
> > He says that the faulty code path can be reached even when XML_UNICODE
> > is not defined. Apparently, building with -DXML_UNICODE merely makes it
> > easier to reach the faulty code.
> > 
> > I think we should take Sebastian's advice. What does everyone think?
> 
> 
> Seems to me that as the Expat maintainer he would know best.

I pushed the commit as b9bc6e842066b066ebdf9eaf75d41753598d75b5

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-09-27 17:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-12 21:35 Expat regression fix for master branch Leo Famulari
2016-09-13 21:54 ` Ludovic Courtès
2016-09-14 18:14   ` Leo Famulari
2016-09-25 23:18 ` Leo Famulari
2016-09-26  8:21   ` Efraim Flashner
2016-09-27 17:50     ` Leo Famulari

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).