[PATCH 0/1] Cracklib security CVE-2016-6318

[PATCH 0/1] Cracklib security CVE-2016-6318

[Leo Famulari]

A stack overflow in Cracklib that could potentially lead to arbitrary
code execution was just disclosed:

http://seclists.org/oss-sec/2016/q3/290

"When an application compiled against the cracklib libary, such as
"passwd" is used to parse the GECOS field, it could cause the
application to crash or execute arbitary code with the permissions of
the user running such an application."

The message recommends this patch:
https://bugzilla.redhat.com/show_bug.cgi?id=1364944#c2

For us, cracklib is used by libpwquality, which is used in turn by
gnome-control-center.

Passwd is safe:
$ guix build --check shadow
[...]
shadow will be compiled with the following features:

	auditing support:		no
	CrackLib support:		no
	PAM support:			yes
	suid account management tools:	yes
	SELinux support:		no
	ACL support:			no
	Extended Attributes support:	no
	tcb support (incomplete):	no
	shadow group support:		yes
	S/Key support:			no
	SHA passwords encryption:	yes
	nscd support:			yes
	subordinate IDs support:	yes

Leo Famulari (1):
  gnu: cracklib: Fix CVE-2016-6318.

 gnu/local.mk                                      |  1 +
 gnu/packages/password-utils.scm                   |  2 +
 gnu/packages/patches/cracklib-CVE-2016-6318.patch | 95 +++++++++++++++++++++++
 3 files changed, 98 insertions(+)
 create mode 100644 gnu/packages/patches/cracklib-CVE-2016-6318.patch

-- 
2.9.3

[PATCH 1/1] gnu: cracklib: Fix CVE-2016-6318.

[Leo Famulari]

* gnu/packages/patches/cracklib-CVE-2016-6318.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/password-utils.scm (cracklib)[source]: Use the patch.
---
 gnu/local.mk                                      |  1 +
 gnu/packages/password-utils.scm                   |  2 +
 gnu/packages/patches/cracklib-CVE-2016-6318.patch | 95 +++++++++++++++++++++++
 3 files changed, 98 insertions(+)
 create mode 100644 gnu/packages/patches/cracklib-CVE-2016-6318.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 7416850..d890046 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -464,6 +464,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/cpio-gets-undeclared.patch		\
   %D%/packages/patches/cpio-CVE-2016-2037.patch			\
   %D%/packages/patches/cpufrequtils-fix-aclocal.patch		\
+  %D%/packages/patches/cracklib-CVE-2016-6318.patch		\
   %D%/packages/patches/crda-optional-gcrypt.patch		\
   %D%/packages/patches/crossmap-allow-system-pysam.patch	\
   %D%/packages/patches/csound-header-ordering.patch		\
diff --git a/gnu/packages/password-utils.scm b/gnu/packages/password-utils.scm
index 7a8bdcb..7288da6 100644
--- a/gnu/packages/password-utils.scm
+++ b/gnu/packages/password-utils.scm
@@ -29,6 +29,7 @@
   #:use-module (guix build-system gnu)
   #:use-module (guix download)
   #:use-module (guix packages)
+  #:use-module (gnu packages)
   #:use-module (gnu packages admin)
   #:use-module (gnu packages base)
   #:use-module (gnu packages compression)
@@ -159,6 +160,7 @@ and vice versa.")
               (uri (string-append "https://github.com/cracklib/cracklib/"
                                   "releases/download/" name "-" version "/"
                                   name "-" version ".tar.gz"))
+              (patches (search-patches "cracklib-CVE-2016-6318.patch"))
               (sha256
                (base32
                 "0hrkb0prf7n92w6rxgq0ilzkk6rkhpys2cfqkrbzswp27na7dkqp"))))
diff --git a/gnu/packages/patches/cracklib-CVE-2016-6318.patch b/gnu/packages/patches/cracklib-CVE-2016-6318.patch
new file mode 100644
index 0000000..4806eca
--- /dev/null
+++ b/gnu/packages/patches/cracklib-CVE-2016-6318.patch
@@ -0,0 +1,95 @@
+Fix CVE-2016-6318.
+
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-6318
+
+Patch copied from Red Hat:
+
+https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2016-6318
+https://bugzilla.redhat.com/attachment.cgi?id=1188599&action=diff
+
+It is not safe to pass words longer than STRINGSIZE further to cracklib
+so the longbuffer cannot be longer than STRINGSIZE.
+diff -up cracklib-2.9.0/lib/fascist.c.longgecos cracklib-2.9.0/lib/fascist.c
+--- cracklib-2.9.0/lib/fascist.c.longgecos	2014-02-06 16:03:59.000000000 +0100
++++ cracklib-2.9.0/lib/fascist.c	2016-08-08 12:05:40.279235815 +0200
+@@ -515,7 +515,7 @@ FascistGecosUser(char *password, const c
+     char gbuffer[STRINGSIZE];
+     char tbuffer[STRINGSIZE];
+     char *uwords[STRINGSIZE];
+-    char longbuffer[STRINGSIZE * 2];
++    char longbuffer[STRINGSIZE];
+ 
+     if (gecos == NULL)
+ 	gecos = "";
+@@ -596,38 +596,47 @@ FascistGecosUser(char *password, const c
+     {
+ 	for (i = 0; i < j; i++)
+ 	{
+-	    strcpy(longbuffer, uwords[i]);
+-	    strcat(longbuffer, uwords[j]);
+-
+-	    if (GTry(longbuffer, password))
++	    if (strlen(uwords[i]) + strlen(uwords[j]) < STRINGSIZE)
+ 	    {
+-		return _("it is derived from your password entry");
+-	    }
+-
+-	    strcpy(longbuffer, uwords[j]);
+-	    strcat(longbuffer, uwords[i]);
++		strcpy(longbuffer, uwords[i]);
++		strcat(longbuffer, uwords[j]);
+ 
+-	    if (GTry(longbuffer, password))
+-	    {
+-		return _("it's derived from your password entry");
++		if (GTry(longbuffer, password))
++		{
++		    return _("it is derived from your password entry");
++		}
++
++		strcpy(longbuffer, uwords[j]);
++		strcat(longbuffer, uwords[i]);
++
++		if (GTry(longbuffer, password))
++		{
++		   return _("it's derived from your password entry");
++		}
+ 	    }
+ 
+-	    longbuffer[0] = uwords[i][0];
+-	    longbuffer[1] = '\0';
+-	    strcat(longbuffer, uwords[j]);
+-
+-	    if (GTry(longbuffer, password))
++	    if (strlen(uwords[j]) < STRINGSIZE - 1)
+ 	    {
+-		return _("it is derivable from your password entry");
++		longbuffer[0] = uwords[i][0];
++		longbuffer[1] = '\0';
++		strcat(longbuffer, uwords[j]);
++
++		if (GTry(longbuffer, password))
++		{
++		    return _("it is derivable from your password entry");
++		}
+ 	    }
+ 
+-	    longbuffer[0] = uwords[j][0];
+-	    longbuffer[1] = '\0';
+-	    strcat(longbuffer, uwords[i]);
+-
+-	    if (GTry(longbuffer, password))
++	    if (strlen(uwords[i]) < STRINGSIZE - 1)
+ 	    {
+-		return _("it's derivable from your password entry");
++		longbuffer[0] = uwords[j][0];
++		longbuffer[1] = '\0';
++		strcat(longbuffer, uwords[i]);
++
++		if (GTry(longbuffer, password))
++		{
++		    return _("it's derivable from your password entry");
++		}
+ 	    }
+ 	}
+     }
-- 
2.9.3

Re: [PATCH 1/1] gnu: cracklib: Fix CVE-2016-6318.

[Eric Bavier]

On Tue, 16 Aug 2016 22:49:55 -0400
Leo Famulari <leo@famulari.name> wrote:

> * gnu/packages/patches/cracklib-CVE-2016-6318.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/password-utils.scm (cracklib)[source]: Use the patch.
> ---
>  gnu/local.mk                                      |  1 +
>  gnu/packages/password-utils.scm                   |  2 +
>  gnu/packages/patches/cracklib-CVE-2016-6318.patch | 95 +++++++++++++++++++++++
>  3 files changed, 98 insertions(+)
>  create mode 100644 gnu/packages/patches/cracklib-CVE-2016-6318.patch

LGTM! Thanks for getting the patch so quick.

From the bug report it looks like we could get some real benefit from
the hardening project thread you revived.

`~Eric

Re: [PATCH 1/1] gnu: cracklib: Fix CVE-2016-6318.

[Leo Famulari]

On Tue, Aug 16, 2016 at 11:29:11PM -0500, Eric Bavier wrote:
> On Tue, 16 Aug 2016 22:49:55 -0400
> Leo Famulari <leo@famulari.name> wrote:
> 
> > * gnu/packages/patches/cracklib-CVE-2016-6318.patch: New file.
> > * gnu/local.mk (dist_patch_DATA): Add it.
> > * gnu/packages/password-utils.scm (cracklib)[source]: Use the patch.
> > ---
> >  gnu/local.mk                                      |  1 +
> >  gnu/packages/password-utils.scm                   |  2 +
> >  gnu/packages/patches/cracklib-CVE-2016-6318.patch | 95 +++++++++++++++++++++++
> >  3 files changed, 98 insertions(+)
> >  create mode 100644 gnu/packages/patches/cracklib-CVE-2016-6318.patch
> 
> LGTM! Thanks for getting the patch so quick.

Thanks for the fast review! Pushed as 53dcbbec07c

> From the bug report it looks like we could get some real benefit from
> the hardening project thread you revived.

Yes, it does look like that!

Re: [PATCH 1/1] gnu: cracklib: Fix CVE-2016-6318.

[Leo Famulari]

On Wed, Aug 17, 2016 at 12:44:29AM -0400, Leo Famulari wrote:
> On Tue, Aug 16, 2016 at 11:29:11PM -0500, Eric Bavier wrote:
> > On Tue, 16 Aug 2016 22:49:55 -0400
> > Leo Famulari <leo@famulari.name> wrote:
> > 
> > > * gnu/packages/patches/cracklib-CVE-2016-6318.patch: New file.
> > > * gnu/local.mk (dist_patch_DATA): Add it.
> > > * gnu/packages/password-utils.scm (cracklib)[source]: Use the patch.
> > > ---
> > >  gnu/local.mk                                      |  1 +
> > >  gnu/packages/password-utils.scm                   |  2 +
> > >  gnu/packages/patches/cracklib-CVE-2016-6318.patch | 95 +++++++++++++++++++++++
> > >  3 files changed, 98 insertions(+)
> > >  create mode 100644 gnu/packages/patches/cracklib-CVE-2016-6318.patch
> > 
> > LGTM! Thanks for getting the patch so quick.
> 
> Thanks for the fast review! Pushed as 53dcbbec07c

It seems this story is not over. SuSE identified another buffer
overflow:
http://seclists.org/oss-sec/2016/q3/370

What do people think of the patch linked from that message?