[bug#36424] expat-2.2.7 for CVE-2018-20843

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Jack Hill]

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

[bug#36424] gnu: expat: Replace with 2.2.7 [security fixes]

[Jack Hill]

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

From 6db23c61704686016a57fb9557240dd83a79bea6 Mon Sep 17 00:00:00 2001
From: Jack Hill <jackhill@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@mykolab.ch>
  ;;; Copyright © 2017 Stefan Reichör <stefan@xsteve.at>
  ;;; Copyright © 2018 Pierre Neidhardt <mail@ambrevar.xyz>
+;;; Copyright © 2019 Jack Hill <jackhill@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

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Marius Bakke]

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

Hi Jack,

Jack Hill <jackhill@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

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

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Jack Hill]

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

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Jack Hill]

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

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-gnu-expat-Add-additional-source-URI.patch, Size: 2966 bytes --]

From 4186a68b660c93b5800be8f126051da92749dc9a Mon Sep 17 00:00:00 2001
From: Jack Hill <jackhill@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(-)

diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index fc60758724..dab6597690 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -20,6 +20,7 @@
 ;;; Copyright © 2017 Petter <petter@mykolab.ch>
 ;;; Copyright © 2017 Stefan Reichör <stefan@xsteve.at>
 ;;; Copyright © 2018 Pierre Neidhardt <mail@ambrevar.xyz>
+;;; Copyright © 2019 Jack Hill <jackhill@jackhill.us>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -63,24 +64,30 @@
   #:use-module (gnu packages pkg-config))
 
 (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
 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)))
+        (license license:expat))))
 
 (define-public libebml
   (package
-- 
2.22.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: text/x-diff; name=0002-gnu-expat-fix-CVE-2018-20843.patch, Size: 3910 bytes --]

From 2f8268a0b549b9c08744d8bc05e2cf135e40be99 Mon Sep 17 00:00:00 2001
From: Jack Hill <jackhill@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		\
   %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
+@@ -6071,7 +6071,7 @@ setElementTypePrefix(XML_Parser parser, ELEMENT_TYPE *elementType)
+       else
+         poolDiscard(&dtd->pool);
+       elementType->prefix = prefix;
+-
++      break;
+     }
+   }
+   return 1;
diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index dab6597690..8c289c5cbe 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -67,6 +67,7 @@
   (let ((dot->underscore (lambda (c) (if (equal? #\. c) #\_ c))))
       (package
         (name "expat")
+        (replacement expat/fixed)
         (version "2.2.6")
         (source (origin
                   (method url-fetch)
@@ -89,6 +90,14 @@ 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-2018-20843.patch"))))))
+
 (define-public libebml
   (package
     (name "libebml")
-- 
2.22.0

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Jack Hill]

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

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Jack Hill]

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

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Jack Hill]

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

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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-gnu-expat-Add-additional-source-URI.patch, Size: 2189 bytes --]

From 0e1394e7e410ec192b6c883b567ce414864cdbb1 Mon Sep 17 00:00:00 2001
From: Jack Hill <jackhill@jackhill.us>
Date: Wed, 10 Jul 2019 16:03:19 -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 | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index fc60758724..b6a376a405 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -20,6 +20,7 @@
 ;;; Copyright © 2017 Petter <petter@mykolab.ch>
 ;;; Copyright © 2017 Stefan Reichör <stefan@xsteve.at>
 ;;; Copyright © 2018 Pierre Neidhardt <mail@ambrevar.xyz>
+;;; Copyright © 2018 Jack Hill <jackhill@jackhill.us>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -66,13 +67,18 @@
   (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"))))
+    (source (let ((dot->underscore (lambda (c) (if (equal? #\. c) #\_ c))))
+              (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")
-- 
2.22.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: text/x-diff; name=0002-gnu-expat-fix-CVE-2018-20843.patch, Size: 3072 bytes --]

From c79efd83ecaa0b541de050da035ef67d972ac458 Mon Sep 17 00:00:00 2001
From: Jack Hill <jackhill@jackhill.us>
Date: Wed, 10 Jul 2019 16:23:03 -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                                  |  1 +
 .../patches/expat-CVE-2018-20843.patch        | 21 +++++++++++++++++++
 gnu/packages/xml.scm                          |  9 ++++++++
 3 files changed, 31 insertions(+)
 create mode 100644 gnu/packages/patches/expat-CVE-2018-20843.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 9a70d73759..054aa93fd5 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -785,6 +785,7 @@ dist_patch_DATA =						\
   %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/expat-CVE-2018-20843.patch		\
   %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..216fbe9667
--- /dev/null
+++ b/gnu/packages/patches/expat-CVE-2018-20843.patch
@@ -0,0 +1,21 @@
+Fix extraction of namespace prefix from XML name.
+Fixes CVE-2018-20843
+
+This patch comes from upstream commit 11f8838bf99ea0a6f0b76f9760c43704d00c4ff6
+https://github.com/libexpat/libexpat/commit/11f8838bf99ea0a6f0b76f9760c43704d00c4ff6
+
+CVE is https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-20843
+
+diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
+index 30d55c5..737d7cd 100644
+--- a/lib/xmlparse.c
++++ b/lib/xmlparse.c
+@@ -6071,7 +6071,7 @@ setElementTypePrefix(XML_Parser parser, ELEMENT_TYPE *elementType)
+       else
+         poolDiscard(&dtd->pool);
+       elementType->prefix = prefix;
+-
++      break;
+     }
+   }
+   return 1;
diff --git a/gnu/packages/xml.scm b/gnu/packages/xml.scm
index b6a376a405..fbd0ff284b 100644
--- a/gnu/packages/xml.scm
+++ b/gnu/packages/xml.scm
@@ -66,6 +66,7 @@
 (define-public expat
   (package
     (name "expat")
+    (replacement expat/fixed)
     (version "2.2.6")
     (source (let ((dot->underscore (lambda (c) (if (equal? #\. c) #\_ c))))
               (origin
@@ -88,6 +89,14 @@ 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-2018-20843.patch"))))))
+
 (define-public libebml
   (package
     (name "libebml")
-- 
2.22.0

bug#36424: expat-2.2.7 for CVE-2018-20843

[Marius Bakke]

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

Jack Hill <jackhill@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.

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

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Jack Hill]

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

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

[bug#36424] expat-2.2.7 for CVE-2018-20843

[Marius Bakke]

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

Jack Hill <jackhill@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@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@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!  :-)

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