[bug#27619] [PATCH] gnu: ncurses: Fix CVE-2017-10684 and CVE-2017-10685.

[bug#27619] [PATCH] gnu: ncurses: Fix CVE-2017-10684 and CVE-2017-10685.

[Leo Famulari]

* gnu/packages/patches/ncurses-CVE-2017-10684-10685.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/ncurses.scm (ncurses)[replacement]: New field.
(ncurses/fixed): New variable.
---
 gnu/local.mk                                       |   1 +
 gnu/packages/ncurses.scm                           |  14 +-
 .../patches/ncurses-CVE-2017-10684-10685.patch     | 200 +++++++++++++++++++++
 3 files changed, 214 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/ncurses-CVE-2017-10684-10685.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 1bcd790f2..67d655212 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -844,6 +844,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/mupen64plus-ui-console-notice.patch	\
   %D%/packages/patches/musl-CVE-2016-8859.patch			\
   %D%/packages/patches/mutt-store-references.patch		\
+  %D%/packages/patches/ncurses-CVE-2017-10684-10685.patch	\
   %D%/packages/patches/net-tools-bitrot.patch			\
   %D%/packages/patches/netcdf-date-time.patch			\
   %D%/packages/patches/netcdf-tst_h_par.patch			\
diff --git a/gnu/packages/ncurses.scm b/gnu/packages/ncurses.scm
index 44a79e718..0b23baf12 100644
--- a/gnu/packages/ncurses.scm
+++ b/gnu/packages/ncurses.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2014, 2016 Mark H Weaver <mhw@netris.org>
-;;; Copyright © 2015 Leo Famulari <leo@famulari.name>
+;;; Copyright © 2015, 2017 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2016 ng0 <ng0@we.make.ritual.n0.is>
 ;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
@@ -37,6 +37,7 @@
 (define-public ncurses
   (package
     (name "ncurses")
+    (replacement ncurses/fixed)
     (version "6.0")
     (source (origin
               (method url-fetch)
@@ -188,6 +189,17 @@ ncursesw library provides wide character support.")
     (license x11)
     (home-page "https://www.gnu.org/software/ncurses/")))
 
+(define ncurses/fixed
+  (package
+    (inherit ncurses)
+    (source
+      (origin
+        (inherit (package-source ncurses))
+        (patches
+          (append
+            (origin-patches (package-source ncurses))
+            (search-patches "ncurses-CVE-2017-10684-10685.patch")))))))
+
 (define-public dialog
   (package
     (name "dialog")
diff --git a/gnu/packages/patches/ncurses-CVE-2017-10684-10685.patch b/gnu/packages/patches/ncurses-CVE-2017-10684-10685.patch
new file mode 100644
index 000000000..1f1b26801
--- /dev/null
+++ b/gnu/packages/patches/ncurses-CVE-2017-10684-10685.patch
@@ -0,0 +1,200 @@
+Fix CVE-2017-10684 and CVE-2017-10685:
+
+http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-10684
+http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-10685
+
+Bug reports included proof of concept reproducer inputs:
+
+https://bugzilla.redhat.com/show_bug.cgi?id=1464684
+https://bugzilla.redhat.com/show_bug.cgi?id=1464685
+https://bugzilla.redhat.com/show_bug.cgi?id=1464686
+https://bugzilla.redhat.com/show_bug.cgi?id=1464687
+https://bugzilla.redhat.com/show_bug.cgi?id=1464688
+https://bugzilla.redhat.com/show_bug.cgi?id=1464691
+https://bugzilla.redhat.com/show_bug.cgi?id=1464692
+
+Patches copied from ncurses patch release 20170701:
+
+ftp://invisible-island.net/ncurses/6.0/ncurses-6.0-20170701.patch.gz
+
+Excerpt from patch release announcement:
+
+ + add/improve checks in tic's parser to address invalid input
+   (Redhat #1464684, #1464685, #1464686, #1464691).
+   + alloc_entry.c, add a check for a null-pointer.
+   + parse_entry.c, add several checks for valid pointers as well as
+     one check to ensure that a single character on a line is not
+     treated as the 2-character termcap short-name.
+ + the fixes for Redhat #1464685 obscured a problem subsequently
+   reported in Redhat #1464687; the given test-case was no longer
+   reproducible.  Testing without the fixes for the earlier reports
+   showed a problem with buffer overflow in dump_entry.c, which is
+   addressed by reducing the use of a fixed-size buffer.
+
+https://lists.gnu.org/archive/html/bug-ncurses/2017-07/msg00001.html
+
+--- ncurses-6.0-20170624+/ncurses/tinfo/alloc_entry.c	2017-04-09 23:33:51.000000000 +0000
++++ ncurses-6.0-20170701/ncurses/tinfo/alloc_entry.c	2017-06-27 23:48:55.000000000 +0000
+@@ -96,7 +96,11 @@
+ {
+     char *result = 0;
+     size_t old_next_free = next_free;
+-    size_t len = strlen(string) + 1;
++    size_t len;
++
++    if (string == 0)
++	return _nc_save_str("");
++    len = strlen(string) + 1;
+ 
+     if (len == 1 && next_free != 0) {
+ 	/*
+--- ncurses-6.0-20170624+/ncurses/tinfo/parse_entry.c	2017-06-24 22:59:46.000000000 +0000
++++ ncurses-6.0-20170701/ncurses/tinfo/parse_entry.c	2017-06-28 00:53:12.000000000 +0000
+@@ -236,13 +236,14 @@
+      * implemented it.  Note that the resulting terminal type was never the
+      * 2-character name, but was instead the first alias after that.
+      */
++#define ok_TC2(s) (isgraph(UChar(s)) && (s) != '|')
+     ptr = _nc_curr_token.tk_name;
+     if (_nc_syntax == SYN_TERMCAP
+ #if NCURSES_XNAMES
+ 	&& !_nc_user_definable
+ #endif
+ 	) {
+-	if (ptr[2] == '|') {
++	if (ok_TC2(ptr[0]) && ok_TC2(ptr[1]) && (ptr[2] == '|')) {
+ 	    ptr += 3;
+ 	    _nc_curr_token.tk_name[2] = '\0';
+ 	}
+@@ -284,9 +285,11 @@
+ 	if (is_use || is_tc) {
+ 	    entryp->uses[entryp->nuses].name = _nc_save_str(_nc_curr_token.tk_valstring);
+ 	    entryp->uses[entryp->nuses].line = _nc_curr_line;
+-	    entryp->nuses++;
+-	    if (entryp->nuses > 1 && is_tc) {
+-		BAD_TC_USAGE
++	    if (VALID_STRING(entryp->uses[entryp->nuses].name)) {
++		entryp->nuses++;
++		if (entryp->nuses > 1 && is_tc) {
++		    BAD_TC_USAGE
++		}
+ 	    }
+ 	} else {
+ 	    /* normal token lookup */
+@@ -588,7 +591,7 @@
+ static void
+ append_acs(string_desc * dst, int code, char *src)
+ {
+-    if (src != 0 && strlen(src) == 1) {
++    if (VALID_STRING(src) && strlen(src) == 1) {
+ 	append_acs0(dst, code, *src);
+     }
+ }
+@@ -849,15 +852,14 @@
+ 	    }
+ 
+ 	    if (tp->Strings[to_ptr->nte_index]) {
++		const char *s = tp->Strings[from_ptr->nte_index];
++		const char *t = tp->Strings[to_ptr->nte_index];
+ 		/* There's no point in warning about it if it's the same
+ 		 * string; that's just an inefficiency.
+ 		 */
+-		if (strcmp(
+-			      tp->Strings[from_ptr->nte_index],
+-			      tp->Strings[to_ptr->nte_index]) != 0)
++		if (VALID_STRING(s) && VALID_STRING(t) && strcmp(s, t) != 0)
+ 		    _nc_warning("%s (%s) already has an explicit value %s, ignoring ko",
+-				ap->to, ap->from,
+-				_nc_visbuf(tp->Strings[to_ptr->nte_index]));
++				ap->to, ap->from, t);
+ 		continue;
+ 	    }
+ 
+--- ncurses-6.0-20170624+/progs/dump_entry.c	2017-06-23 22:47:43.000000000 +0000
++++ ncurses-6.0-20170701/progs/dump_entry.c	2017-07-01 11:27:29.000000000 +0000
+@@ -841,9 +841,10 @@
+     PredIdx num_strings = 0;
+     bool outcount = 0;
+ 
+-#define WRAP_CONCAT	\
+-	wrap_concat(buffer); \
+-	outcount = TRUE
++#define WRAP_CONCAT1(s)		wrap_concat(s); outcount = TRUE
++#define WRAP_CONCAT2(a,b)	wrap_concat(a); WRAP_CONCAT1(b)
++#define WRAP_CONCAT3(a,b,c)	wrap_concat(a); WRAP_CONCAT2(b,c)
++#define WRAP_CONCAT		WRAP_CONCAT1(buffer)
+ 
+     len = 12;			/* terminfo file-header */
+ 
+@@ -1007,9 +1008,9 @@
+ 		    set_attributes = save_sgr;
+ 
+ 		    trimmed_sgr0 = _nc_trim_sgr0(tterm);
+-		    if (strcmp(capability, trimmed_sgr0))
++		    if (strcmp(capability, trimmed_sgr0)) {
+ 			capability = trimmed_sgr0;
+-		    else {
++		    } else {
+ 			if (trimmed_sgr0 != exit_attribute_mode)
+ 			    free(trimmed_sgr0);
+ 		    }
+@@ -1046,13 +1047,21 @@
+ 			_nc_SPRINTF(buffer, _nc_SLIMIT(sizeof(buffer))
+ 				    "%s=!!! %s WILL NOT CONVERT !!!",
+ 				    name, srccap);
++			WRAP_CONCAT;
+ 		    } else if (suppress_untranslatable) {
+ 			continue;
+ 		    } else {
+ 			char *s = srccap, *d = buffer;
+-			_nc_SPRINTF(d, _nc_SLIMIT(sizeof(buffer)) "..%s=", name);
+-			d += strlen(d);
++			WRAP_CONCAT3("..", name, "=");
+ 			while ((*d = *s++) != 0) {
++			    if ((d - buffer - 1) >= (int) sizeof(buffer)) {
++				fprintf(stderr,
++					"%s: value for %s is too long\n",
++					_nc_progname,
++					name);
++				*d = '\0';
++				break;
++			    }
+ 			    if (*d == ':') {
+ 				*d++ = '\\';
+ 				*d = ':';
+@@ -1061,13 +1070,12 @@
+ 			    }
+ 			    d++;
+ 			}
++			WRAP_CONCAT;
+ 		    }
+ 		} else {
+-		    _nc_SPRINTF(buffer, _nc_SLIMIT(sizeof(buffer))
+-				"%s=%s", name, cv);
++		    WRAP_CONCAT3(name, "=", cv);
+ 		}
+ 		len += (int) strlen(capability) + 1;
+-		WRAP_CONCAT;
+ 	    } else {
+ 		char *src = _nc_tic_expand(capability,
+ 					   outform == F_TERMINFO, numbers);
+@@ -1083,8 +1091,7 @@
+ 		    strcpy_DYN(&tmpbuf, src);
+ 		}
+ 		len += (int) strlen(capability) + 1;
+-		wrap_concat(tmpbuf.text);
+-		outcount = TRUE;
++		WRAP_CONCAT1(tmpbuf.text);
+ 	    }
+ 	}
+ 	/* e.g., trimmed_sgr0 */
+@@ -1526,7 +1533,8 @@
+ 		}
+ 		if (len > critlen) {
+ 		    (void) fprintf(stderr,
+-				   "warning: %s entry is %d bytes long\n",
++				   "%s: %s entry is %d bytes long\n",
++				   _nc_progname,
+ 				   _nc_first_name(tterm->term_names),
+ 				   len);
+ 		    SHOW_WHY("# WARNING: this entry, %d bytes long, may core-dump %s libraries!\n",
-- 
2.13.2

[bug#27619] ncurses patch releases

[Leo Famulari]

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

According to this message on bug-ncurses, the fixes could be incomplete,
although I doubt that person is using the exact same subset of the
upstream patch as the one I am proposing:

https://lists.gnu.org/archive/html/bug-ncurses/2017-07/msg00008.html

On the general subject of ncurses bugs, the ncurses author issues patch
releases for ncurses frequently:

ftp://invisible-island.net/ncurses/6.0/

I didn't know that and I haven't read the changelogs to see if there are
other very important fixes for us to use.

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

[bug#27619] [PATCH] gnu: ncurses: Fix CVE-2017-10684 and CVE-2017-10685.

[Ludovic Courtès]

Leo Famulari <leo@famulari.name> skribis:

> * gnu/packages/patches/ncurses-CVE-2017-10684-10685.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/ncurses.scm (ncurses)[replacement]: New field.
> (ncurses/fixed): New variable.

LGTM, thank you!

Ludo’.

[bug#27619] ncurses patch releases

[Ludovic Courtès]

Leo Famulari <leo@famulari.name> skribis:

> According to this message on bug-ncurses, the fixes could be incomplete,
> although I doubt that person is using the exact same subset of the
> upstream patch as the one I am proposing:
>
> https://lists.gnu.org/archive/html/bug-ncurses/2017-07/msg00008.html
>
> On the general subject of ncurses bugs, the ncurses author issues patch
> releases for ncurses frequently:
>
> ftp://invisible-island.net/ncurses/6.0/
>
> I didn't know that and I haven't read the changelogs to see if there are
> other very important fixes for us to use.

Indeed, it might be best to regularly upgrade from there.

BTW, what should we do in ‘core-updates’?  I would suggest at least
applying the patch you sent, and maybe upgrading to one of the releases
above, though I haven’t checked what fixes they contain.

Thanks,
Ludo’.

bug#27619: ncurses patch releases

[Leo Famulari]

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

On Mon, Jul 10, 2017 at 12:30:54PM +0200, Ludovic Courtès wrote:
> Leo Famulari <leo@famulari.name> skribis:
> 
> > According to this message on bug-ncurses, the fixes could be incomplete,
> > although I doubt that person is using the exact same subset of the
> > upstream patch as the one I am proposing:
> >
> > https://lists.gnu.org/archive/html/bug-ncurses/2017-07/msg00008.html
> >
> > On the general subject of ncurses bugs, the ncurses author issues patch
> > releases for ncurses frequently:
> >
> > ftp://invisible-island.net/ncurses/6.0/
> >
> > I didn't know that and I haven't read the changelogs to see if there are
> > other very important fixes for us to use.
> 
> Indeed, it might be best to regularly upgrade from there.
> 
> BTW, what should we do in ‘core-updates’?  I would suggest at least
> applying the patch you sent, and maybe upgrading to one of the releases
> above, though I haven’t checked what fixes they contain.

Since we are still making relatively "heavy" changes (groff and
ghostscript), I'll apply these patches without a graft on core-updates.
Then we should freeze it, for real :)

I looked into applying the upstream patch releases. It's not trivial,
because some of them are "just patches" and some of them are shell
scripts. So, we'll need to do write some special code to build ncurses.
I'd rather continue with core-updates and do this later.

Also, I'd like for us to download these patch releases when needed
instead of including them with Guix.

It will mean that ncurses will almost always be grafted...

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

[bug#27619] ncurses patch releases

[Ludovic Courtès]

Leo Famulari <leo@famulari.name> skribis:

> On Mon, Jul 10, 2017 at 12:30:54PM +0200, Ludovic Courtès wrote:
>> Leo Famulari <leo@famulari.name> skribis:
>> 
>> > According to this message on bug-ncurses, the fixes could be incomplete,
>> > although I doubt that person is using the exact same subset of the
>> > upstream patch as the one I am proposing:
>> >
>> > https://lists.gnu.org/archive/html/bug-ncurses/2017-07/msg00008.html
>> >
>> > On the general subject of ncurses bugs, the ncurses author issues patch
>> > releases for ncurses frequently:
>> >
>> > ftp://invisible-island.net/ncurses/6.0/
>> >
>> > I didn't know that and I haven't read the changelogs to see if there are
>> > other very important fixes for us to use.
>> 
>> Indeed, it might be best to regularly upgrade from there.
>> 
>> BTW, what should we do in ‘core-updates’?  I would suggest at least
>> applying the patch you sent, and maybe upgrading to one of the releases
>> above, though I haven’t checked what fixes they contain.
>
> Since we are still making relatively "heavy" changes (groff and
> ghostscript), I'll apply these patches without a graft on core-updates.
> Then we should freeze it, for real :)

Sounds good!

> I looked into applying the upstream patch releases. It's not trivial,
> because some of them are "just patches" and some of them are shell
> scripts. So, we'll need to do write some special code to build ncurses.
> I'd rather continue with core-updates and do this later.

OK.

> Also, I'd like for us to download these patch releases when needed
> instead of including them with Guix.
>
> It will mean that ncurses will almost always be grafted...

Yeah.

Thanks,
Ludo’.