GNU bug report logs - #36424
expat-2.2.7 for CVE-2018-20843

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 36424 in the body.
You can then email your comments to 36424 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jack Hill <jackhill <at> jackhill.us>
To: guix-patches <at> gnu.org
Subject: expat-2.2.7 for CVE-2018-20843
Date: Fri, 28 Jun 2019 15:56:42 -0400 (EDT)

Hi Guix,

Sebastian Pipping recently wrote to guix-devel@ about expat-2.2.7 which 
fixes CVE-2018-20843 [0]. I've prepared the forthcoming patch to add a 
replacement for expat with expat-2.2.7. I also changed the origin to use 
the GitHub hosted tarball as upstream is moving in that direction.

[0] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-20843

Best,
Jack

Message #8 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Jack Hill <jackhill <at> jackhill.us>
To: 36424 <at> debbugs.gnu.org
Subject: gnu: expat: Replace with 2.2.7 [security fixes]
Date: Fri, 28 Jun 2019 15:57:58 -0400 (EDT)

[Message part 1 (text/plain, inline)]

From 6db23c61704686016a57fb9557240dd83a79bea6 Mon Sep 17 00:00:00 2001
From: Jack Hill <jackhill <at> jackhill.us>
Date: Fri, 28 Jun 2019 15:47:35 -0400

This fixes CVE-2018-20843.

* gnu/packages/xml.scm (expat)[replacement]: New field.
(expat-2.2.7): New public variable.
---
 gnu/packages/xml.scm | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index fc60758724..1be2a58d2e 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -20,6 +20,7 @@
 ;;; Copyright © 2017 Petter <petter <at> mykolab.ch>
 ;;; Copyright © 2017 Stefan Reichör <stefan <at> xsteve.at>
 ;;; Copyright © 2018 Pierre Neidhardt <mail <at> ambrevar.xyz>
+;;; Copyright © 2019 Jack Hill <jackhill <at> jackhill.us>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -65,6 +66,7 @@
 (define-public expat
   (package
     (name "expat")
+    (replacement expat-2.2.7)
     (version "2.2.6")
     (source (origin
              (method url-fetch)
@@ -82,6 +84,21 @@ 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-public expat-2.2.7
+  (let ((dot->underscore (lambda (c) (if (equal? #\. c) #\_ c))))
+    (package
+      (inherit expat)
+      (version "2.2.7")
+      (source
+       (origin
+         (method url-fetch)
+         (uri (string-append "https://github.com/libexpat/libexpat/releases/download/R_"
+                             (string-map dot->underscore version)
+                             "/expat-" version ".tar.xz"))
+         (sha256
+          (base32
+           "1y5yax6bq8p9xk49zqkd62pxk8bq266wrgbrqgaxp3wsrw5g9qrh")))))))
+
 (define-public libebml
   (package
     (name "libebml")
-- 
2.22.0

Message #11 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Jack Hill <jackhill <at> jackhill.us>, 36424 <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Sun, 30 Jun 2019 12:12:22 +0200

[Message part 1 (text/plain, inline)]

Hi Jack,

Jack Hill <jackhill <at> jackhill.us> writes:

> Hi Guix,
>
> Sebastian Pipping recently wrote to guix-devel@ about expat-2.2.7 which 
> fixes CVE-2018-20843 [0]. I've prepared the forthcoming patch to add a 
> replacement for expat with expat-2.2.7. I also changed the origin to use 
> the GitHub hosted tarball as upstream is moving in that direction.
>
> [0] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-20843

Thank you very much for this patch!  It did not apply cleanly on my end,
perhaps it got mangled by your mail user agent?

I tried running `abidiff` (from libabigail) on the new and old Expat:

$ abidiff /gnu/store/79a7p4fjh564czghfzfm1yn8b3r42rbi-expat-2.2.6/lib/libexpat.so /gnu/store/khy5yzn5fgipsfvcchqyhkg56d68wd2k-expat-2.2.7/lib/libexpat.so
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
Function symbols changes summary: 15 Removed, 0 Added function symbols not referenced by debug info
Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info

15 Removed function symbols not referenced by debug info:

  XmlGetUtf16InternalEncoding
  XmlGetUtf16InternalEncodingNS
  XmlGetUtf8InternalEncoding
  XmlGetUtf8InternalEncodingNS
  XmlInitEncoding
  XmlInitEncodingNS
  XmlInitUnknownEncoding
  XmlInitUnknownEncodingNS
  XmlParseXmlDecl
  XmlParseXmlDeclNS
  XmlPrologStateInit
  XmlPrologStateInitExternalEntity
  XmlSizeOfUnknownEncoding
  XmlUtf16Encode
  XmlUtf8Encode

Apparently these symbols were never supposed to be exported:
<https://github.com/libexpat/libexpat/pull/197>.  However, there could
be packages "in the wild" that uses these symbols and would silently
break with the grafted Expat.

IIUC the fix for CVE-2018-20843 is this commit:
<https://github.com/libexpat/libexpat/commit/11f8838bf99ea0a6f0b76f9760c43704d00c4ff6>.

I think it's better to graft a variant with only this patch to be on the
safe side.  Can you try that?

Could you also submit a second patch that adds GitHub as an additional
download location for the regular Expat package?  :-)

Thanks in advance,
Marius

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Jack Hill <jackhill <at> jackhill.us>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36424 <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Tue, 2 Jul 2019 16:49:30 -0400 (EDT)

Marius,

Thanks for looking at this.

On Sun, 30 Jun 2019, Marius Bakke wrote:

> I tried running `abidiff` (from libabigail) on the new and old Expat:
>
> $ abidiff /gnu/store/79a7p4fjh564czghfzfm1yn8b3r42rbi-expat-2.2.6/lib/libexpat.so /gnu/store/khy5yzn5fgipsfvcchqyhkg56d68wd2k-expat-2.2.7/lib/libexpat.so
> Functions changes summary: 0 Removed, 0 Changed, 0 Added function
> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> Function symbols changes summary: 15 Removed, 0 Added function symbols not referenced by debug info
> Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info
>
> 15 Removed function symbols not referenced by debug info:
>
>  XmlGetUtf16InternalEncoding
>  XmlGetUtf16InternalEncodingNS
>  XmlGetUtf8InternalEncoding
>  XmlGetUtf8InternalEncodingNS
>  XmlInitEncoding
>  XmlInitEncodingNS
>  XmlInitUnknownEncoding
>  XmlInitUnknownEncodingNS
>  XmlParseXmlDecl
>  XmlParseXmlDeclNS
>  XmlPrologStateInit
>  XmlPrologStateInitExternalEntity
>  XmlSizeOfUnknownEncoding
>  XmlUtf16Encode
>  XmlUtf8Encode
>
> Apparently these symbols were never supposed to be exported:
> <https://github.com/libexpat/libexpat/pull/197>.  However, there could
> be packages "in the wild" that uses these symbols and would silently
> break with the grafted Expat.
>
> IIUC the fix for CVE-2018-20843 is this commit:
> <https://github.com/libexpat/libexpat/commit/11f8838bf99ea0a6f0b76f9760c43704d00c4ff6>.
>
> I think it's better to graft a variant with only this patch to be on the
> safe side.  Can you try that?

Good idea. I didn't think to check. Yes, I can try to do that.

> Could you also submit a second patch that adds GitHub as an additional
> download location for the regular Expat package?  :-)

I'll try that as well.

I'll also try to not let my mail client mangle them :)

Best,
Jack

Message #19 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Jack Hill <jackhill <at> jackhill.us>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36424 <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Thu, 4 Jul 2019 19:49:57 -0400 (EDT)

[Message part 1 (text/plain, inline)]

On Tue, 2 Jul 2019, Jack Hill wrote:

>> Apparently these symbols were never supposed to be exported:
>> <https://github.com/libexpat/libexpat/pull/197>.  However, there could
>> be packages "in the wild" that uses these symbols and would silently
>> break with the grafted Expat.
>> 
>> IIUC the fix for CVE-2018-20843 is this commit:
>> <https://github.com/libexpat/libexpat/commit/11f8838bf99ea0a6f0b76f9760c43704d00c4ff6>.
>> 
>> I think it's better to graft a variant with only this patch to be on the
>> safe side.  Can you try that?
>
> Good idea. I didn't think to check. Yes, I can try to do that.
>
>> Could you also submit a second patch that adds GitHub as an additional
>> download location for the regular Expat package?  :-)
>
> I'll try that as well.

I've prepared the two attached patches that I believe implement Marius's 
proposed solution.

Thanks,
Jack

[0001-gnu-expat-Add-additional-source-URI.patch (text/x-diff, attachment)]

[0002-gnu-expat-fix-CVE-2018-20843.patch (text/x-diff, attachment)]

Message #22 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Jack Hill <jackhill <at> jackhill.us>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36424 <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Thu, 4 Jul 2019 19:57:33 -0400 (EDT)

Woops, looks like I still mangled the patches (by adding carriage-returns), 
but hopefully in a way that they still apply without infecting the code 
with that problem.

I guess Alpine has let me down. At any rate, hopefully they're still 
useful and fix the problem. Let me know if you'd like me to try again.

Best,
Jack

Message #25 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Jack Hill <jackhill <at> jackhill.us>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36424 <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Thu, 4 Jul 2019 20:02:13 -0400 (EDT)

Also, sorry for the extra noise in gnu/local.mk. I had inserted my patch 
in the wrong place and alphabetized a number of lines using my 
en_us.UTF-8 locale to fix it. Let me know if I should re-submit without 
the extraneous changes.

Today hasn't been the best day for computer use for me I'm afraid.

Best,
Jack

Message #28 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Jack Hill <jackhill <at> jackhill.us>
Cc: 36424 <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Sat, 06 Jul 2019 00:53:52 +0200

[Message part 1 (text/plain, inline)]

Jack Hill <jackhill <at> jackhill.us> writes:

> On Tue, 2 Jul 2019, Jack Hill wrote:
>
>>> Apparently these symbols were never supposed to be exported:
>>> <https://github.com/libexpat/libexpat/pull/197>.  However, there could
>>> be packages "in the wild" that uses these symbols and would silently
>>> break with the grafted Expat.
>>> 
>>> IIUC the fix for CVE-2018-20843 is this commit:
>>> <https://github.com/libexpat/libexpat/commit/11f8838bf99ea0a6f0b76f9760c43704d00c4ff6>.
>>> 
>>> I think it's better to graft a variant with only this patch to be on the
>>> safe side.  Can you try that?
>>
>> Good idea. I didn't think to check. Yes, I can try to do that.
>>
>>> Could you also submit a second patch that adds GitHub as an additional
>>> download location for the regular Expat package?  :-)
>>
>> I'll try that as well.
>
> I've prepared the two attached patches that I believe implement Marius's 
> proposed solution.

Thanks!

One minor problem... the expat patch does not actually apply on our copy
of expat!  Can you look into it?

> From 4186a68b660c93b5800be8f126051da92749dc9a Mon Sep 17 00:00:00 2001
> From: Jack Hill <jackhill <at> jackhill.us>
> Date: Thu, 4 Jul 2019 17:00:27 -0400
> Subject: [PATCH 1/2] gnu: expat: Add additional source URI
>
> The expat sourceforge page announces that the project is in the process of
> moving to GitHub.
>
> * gnu/packages/xml.scm (expat)[source]: Add GitHub URI.
> ---
>  gnu/packages/xml.scm | 39 +++++++++++++++++++++++----------------
>  1 file changed, 23 insertions(+), 16 deletions(-)

[...]
  
>  (define-public expat
> -  (package
> -    (name "expat")
> -    (version "2.2.6")
> -    (source (origin
> -             (method url-fetch)
> -             (uri (string-append "mirror://sourceforge/expat/expat/"
> -                                 version "/expat-" version ".tar.bz2"))
> -             (sha256
> -              (base32
> -               "1wl1x93b5w457ddsdgj0lh7yjq4q6l7wfbgwhagkc8fm2qkkrd0p"))))
> -    (build-system gnu-build-system)
> -    (home-page "https://libexpat.github.io/")
> -    (synopsis "Stream-oriented XML parser library written in C")
> -    (description
> -     "Expat is an XML parser library written in C.  It is a
> +  (let ((dot->underscore (lambda (c) (if (equal? #\. c) #\_ c))))
> +      (package
> +        (name "expat")
> +        (version "2.2.6")
> +        (source (origin
> +                  (method url-fetch)
> +                  (uri (list (string-append
> +                              "mirror://sourceforge/expat/expat/"
> +                              version "/expat-" version ".tar.bz2")
> +                             (string-append
> +                              "https://github.com/libexpat/libexpat/releases/download/R_"
> +                              (string-map dot->underscore version)
> +                              "/expat-" version ".tar.bz2")))
> +                  (sha256
> +                   (base32
> +                    "1wl1x93b5w457ddsdgj0lh7yjq4q6l7wfbgwhagkc8fm2qkkrd0p"))))
> +        (build-system gnu-build-system)
> +        (home-page "https://libexpat.github.io/")
> +        (synopsis "Stream-oriented XML parser library written in C")
> +        (description
> +         "Expat is an XML parser library written in C.  It is a

Can you move this let binding inside the (source ...) field?  That way
we don't have to reindent the whole thing.

> From 2f8268a0b549b9c08744d8bc05e2cf135e40be99 Mon Sep 17 00:00:00 2001
> From: Jack Hill <jackhill <at> jackhill.us>
> Date: Thu, 4 Jul 2019 19:41:30 -0400
> Subject: [PATCH 2/2] gnu: expat: fix CVE-2018-20843.
>
> * gnu/packages/xml.scm (expat)[replacement]: New field.
> (expat/fixed): New variable.
> * gnu/packages/patches/expat-CVE-2018-20843.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add patch file.
> ---
>  gnu/local.mk                                    |  7 ++++---
>  gnu/packages/patches/expat-CVE-2018-20843.patch | 16 ++++++++++++++++
>  gnu/packages/xml.scm                            |  9 +++++++++
>  3 files changed, 29 insertions(+), 3 deletions(-)
>  create mode 100644 gnu/packages/patches/expat-CVE-2018-20843.patch
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 6e90d88689..bcf47d7378 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -764,20 +764,21 @@ dist_patch_DATA =						\
>    %D%/packages/patches/einstein-build.patch			\
>    %D%/packages/patches/emacs-exec-path.patch			\
>    %D%/packages/patches/emacs-fix-scheme-indent-function.patch	\
> -  %D%/packages/patches/emacs-json-reformat-fix-tests.patch	\
>    %D%/packages/patches/emacs-highlight-stages-add-gexp.patch	\
> +  %D%/packages/patches/emacs-json-reformat-fix-tests.patch	\
>    %D%/packages/patches/emacs-scheme-complete-scheme-r5rs-info.patch	\
>    %D%/packages/patches/emacs-source-date-epoch.patch		\
> -  %D%/packages/patches/emacs-unpackaged-req.patch		\
>    %D%/packages/patches/emacs-undohist-ignored.patch	\
> +  %D%/packages/patches/emacs-unpackaged-req.patch		\
>    %D%/packages/patches/emacs-wordnut-require-adaptive-wrap.patch	\
>    %D%/packages/patches/emacs-zones-called-interactively.patch	\
>    %D%/packages/patches/enlightenment-fix-setuid-path.patch	\
>    %D%/packages/patches/erlang-man-path.patch			\
>    %D%/packages/patches/eudev-rules-directory.patch		\
>    %D%/packages/patches/evilwm-lost-focus-bug.patch		\
> -  %D%/packages/patches/exiv2-CVE-2017-14860.patch		\
>    %D%/packages/patches/exiv2-CVE-2017-14859-14862-14864.patch	\
> +  %D%/packages/patches/exiv2-CVE-2017-14860.patch		\
> +  %D%/packages/patches/expat-CVE-2018-20843.patch		\

You addressed this in another email, and I do think we should try to
avoid needless moving around of these lines.  There are enough merge
conflicts on this file as-is, no need to introduce artificial ones.  :-)

>    %D%/packages/patches/extundelete-e2fsprogs-1.44.patch		\
>    %D%/packages/patches/fastcap-mulGlobal.patch			\
>    %D%/packages/patches/fastcap-mulSetup.patch			\
> diff --git a/gnu/packages/patches/expat-CVE-2018-20843.patch b/gnu/packages/patches/expat-CVE-2018-20843.patch
> new file mode 100644
> index 0000000000..dd64b91965
> --- /dev/null
> +++ b/gnu/packages/patches/expat-CVE-2018-20843.patch
> @@ -0,0 +1,16 @@
> +Fix extraction of namespace prefix from XML name.
> +Fixes CVE-2018-20843
> +
> +diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
> +index 30d55c5..737d7cd 100644
> +--- a/expat/lib/xmlparse.c
> ++++ b/expat/lib/xmlparse.c
         ^^^^^^
It looks like this has to be removed from the patch file.  Could you
also add a link to the upstream commit for reference?

It's also good practice to provide an URL to the MITRE CVE page:
<https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-20843>.

Thanks for working on this!  :-)

[signature.asc (application/pgp-signature, inline)]

Message #31 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Jack Hill <jackhill <at> jackhill.us>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36424 <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Wed, 10 Jul 2019 16:54:12 -0400 (EDT)

[Message part 1 (text/plain, inline)]

Please find updated patch files attached, that I think take into account 
Marius's suggestions (thanks Marius!)

Best,
Jack

P.S. I'm afraid, I'm still struggling with alpine inserting carriage returns 
in the attachments.

[0001-gnu-expat-Add-additional-source-URI.patch (text/x-diff, attachment)]

[0002-gnu-expat-fix-CVE-2018-20843.patch (text/x-diff, attachment)]

Message #36 received at 36424-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Jack Hill <jackhill <at> jackhill.us>
Cc: 36424-done <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Fri, 12 Jul 2019 01:00:32 +0200

[Message part 1 (text/plain, inline)]

Jack Hill <jackhill <at> jackhill.us> writes:

> Please find updated patch files attached, that I think take into account 
> Marius's suggestions (thanks Marius!)

Thank you!  I made a tiny tweak to use char=? instead of equal=? for the
character comparison.

Pushed as 5a836ce38c9c29e9c2bd306007347486b90c5064.

[signature.asc (application/pgp-signature, inline)]

Message #39 received at 36424 <at> debbugs.gnu.org (full text, mbox):

From: Jack Hill <jackhill <at> jackhill.us>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36424 <at> debbugs.gnu.org
Subject: Re: [bug#36424] expat-2.2.7 for CVE-2018-20843
Date: Thu, 11 Jul 2019 19:09:45 -0400 (EDT)

[Message part 1 (text/plain, inline)]

On Fri, 12 Jul 2019, Marius Bakke wrote:

> Thank you!  I made a tiny tweak to use char=? instead of equal=? for the
> character comparison.

Cool, now I know about char=? ☺

> Pushed as 5a836ce38c9c29e9c2bd306007347486b90c5064.

Thanks, and thanks for being patient with me working through the issues.

Best,
Jack

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.