Patchwork cracklib: Fix buffer overflow

login
register
mail settings
Submitter Leo Famulari
Date Sept. 15, 2016, 3:36 p.m.
Message ID <20160915153646.GA31020@jasmine>
Download mbox | patch
Permalink /patch/15676/
State New
Headers show

Comments

Leo Famulari - Sept. 15, 2016, 3:36 p.m.
This patch cherry-picks an upstream commit to fix a buffer overflow in
cracklib. Please see the patch file for more information about the bug.
From 62f8f1763ba1766e92e8dc05686bd9353eaf2ad5 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Thu, 15 Sep 2016 11:34:49 -0400
Subject: [PATCH] gnu: cracklib: Fix buffer overflow.

* gnu/packages/patches/cracklib-fix-buffer-overflow.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/password-utils.scm (cracklib)[source]: Use it.
---
 gnu/local.mk                                       |  1 +
 gnu/packages/password-utils.scm                    |  3 +-
 .../patches/cracklib-fix-buffer-overflow.patch     | 39 ++++++++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/cracklib-fix-buffer-overflow.patch
Efraim Flashner - Sept. 20, 2016, 9:32 a.m.
On Thu, Sep 15, 2016 at 11:36:46AM -0400, Leo Famulari wrote:
> This patch cherry-picks an upstream commit to fix a buffer overflow in
> cracklib. Please see the patch file for more information about the bug.

> From 62f8f1763ba1766e92e8dc05686bd9353eaf2ad5 Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo@famulari.name>
> Date: Thu, 15 Sep 2016 11:34:49 -0400
> Subject: [PATCH] gnu: cracklib: Fix buffer overflow.
> 
> * gnu/packages/patches/cracklib-fix-buffer-overflow.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/password-utils.scm (cracklib)[source]: Use it.
> ---
>  gnu/local.mk                                       |  1 +
>  gnu/packages/password-utils.scm                    |  3 +-
>  .../patches/cracklib-fix-buffer-overflow.patch     | 39 ++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 1 deletion(-)
>  create mode 100644 gnu/packages/patches/cracklib-fix-buffer-overflow.patch
> 
> diff --git a/gnu/local.mk b/gnu/local.mk
> index a7006cb..ab052af 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -473,6 +473,7 @@ dist_patch_DATA =						\
>    %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/cracklib-fix-buffer-overflow.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 7288da6..40ed933 100644
> --- a/gnu/packages/password-utils.scm
> +++ b/gnu/packages/password-utils.scm
> @@ -160,7 +160,8 @@ 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"))
> +              (patches (search-patches "cracklib-CVE-2016-6318.patch"
> +                                       "cracklib-fix-buffer-overflow.patch"))
>                (sha256
>                 (base32
>                  "0hrkb0prf7n92w6rxgq0ilzkk6rkhpys2cfqkrbzswp27na7dkqp"))))
> diff --git a/gnu/packages/patches/cracklib-fix-buffer-overflow.patch b/gnu/packages/patches/cracklib-fix-buffer-overflow.patch
> new file mode 100644
> index 0000000..b1c990f
> --- /dev/null
> +++ b/gnu/packages/patches/cracklib-fix-buffer-overflow.patch
> @@ -0,0 +1,39 @@
> +Fix buffer overflow processing long words in Mangle().
> +
> +Patch adpated from upstream commit, omitting changes to 'NEWS':
> +
> +https://github.com/cracklib/cracklib/commit/33d7fa4585247cd2247a1ffa032ad245836c6edb
> +
> +From 33d7fa4585247cd2247a1ffa032ad245836c6edb Mon Sep 17 00:00:00 2001
> +From: Jan Dittberner <jan@dittberner.info>
> +Date: Thu, 25 Aug 2016 17:17:53 +0200
> +Subject: [PATCH] Fix a buffer overflow processing long words
> +
> +A buffer overflow processing long words has been discovered. This commit
> +applies the patch from
> +https://build.opensuse.org/package/view_file/Base:System/cracklib/0004-overflow-processing-long-words.patch
> +by Howard Guo.
> +
> +See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=835386 and
> +http://www.openwall.com/lists/oss-security/2016/08/23/8
> +---
> + src/NEWS        | 1 +
> + src/lib/rules.c | 5 ++---
> + 2 files changed, 3 insertions(+), 3 deletions(-)
> +
> +diff --git a/src/lib/rules.c b/src/lib/rules.c
> +index d193cc0..3a2aa46 100644
> +--- a/lib/rules.c
> ++++ b/lib/rules.c
> +@@ -434,9 +434,8 @@ Mangle(input, control)		/* returns a pointer to a controlled Mangle */
> + {
> +     int limit;
> +     register char *ptr;
> +-    static char area[STRINGSIZE];
> +-    char area2[STRINGSIZE];
> +-    area[0] = '\0';
> ++    static char area[STRINGSIZE * 2] = {0};
> ++    char area2[STRINGSIZE * 2] = {0};
> +     strcpy(area, input);
> + 
> +     for (ptr = control; *ptr; ptr++)
> -- 
> 2.10.0
> 

not having looked at the full source of lib/rules.c, is there a maximum
value to STRINGSIZE to make sure STRINGSIZE * 2 doesn't wrap around?
Leo Famulari - Sept. 20, 2016, 5:43 p.m.
On Tue, Sep 20, 2016 at 12:32:02PM +0300, Efraim Flashner wrote:
> > +diff --git a/src/lib/rules.c b/src/lib/rules.c
> > +index d193cc0..3a2aa46 100644
> > +--- a/lib/rules.c
> > ++++ b/lib/rules.c
> > +@@ -434,9 +434,8 @@ Mangle(input, control)		/* returns a pointer to a controlled Mangle */
> > + {
> > +     int limit;
> > +     register char *ptr;
> > +-    static char area[STRINGSIZE];
> > +-    char area2[STRINGSIZE];
> > +-    area[0] = '\0';
> > ++    static char area[STRINGSIZE * 2] = {0};
> > ++    char area2[STRINGSIZE * 2] = {0};
> > +     strcpy(area, input);
> > + 
> > +     for (ptr = control; *ptr; ptr++)
> > -- 
> > 2.10.0
> > 
> 
> not having looked at the full source of lib/rules.c, is there a maximum
> value to STRINGSIZE to make sure STRINGSIZE * 2 doesn't wrap around?

STRINGSIZE is defined in 'lib/packer.h' as 1024:

https://github.com/cracklib/cracklib/blob/cracklib-2.9.6/src/lib/packer.h#L11

I just looked at all the uses of STRINGSIZE in order to give a brief
overview of how it's used, but I'm not skilled enough to recognize every
case where it might be dangerous and overflow.

STRINGSIZE is used to declare many char arrays, an array of pointers,
and as an argument to fgets, snprintf, and strncpy. Also the object
macro TRUNCSTRINGSIZE is defined as (STRINGSIZE / 4).

Patch

diff --git a/gnu/local.mk b/gnu/local.mk
index a7006cb..ab052af 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -473,6 +473,7 @@  dist_patch_DATA =						\
   %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/cracklib-fix-buffer-overflow.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 7288da6..40ed933 100644
--- a/gnu/packages/password-utils.scm
+++ b/gnu/packages/password-utils.scm
@@ -160,7 +160,8 @@  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"))
+              (patches (search-patches "cracklib-CVE-2016-6318.patch"
+                                       "cracklib-fix-buffer-overflow.patch"))
               (sha256
                (base32
                 "0hrkb0prf7n92w6rxgq0ilzkk6rkhpys2cfqkrbzswp27na7dkqp"))))
diff --git a/gnu/packages/patches/cracklib-fix-buffer-overflow.patch b/gnu/packages/patches/cracklib-fix-buffer-overflow.patch
new file mode 100644
index 0000000..b1c990f
--- /dev/null
+++ b/gnu/packages/patches/cracklib-fix-buffer-overflow.patch
@@ -0,0 +1,39 @@ 
+Fix buffer overflow processing long words in Mangle().
+
+Patch adpated from upstream commit, omitting changes to 'NEWS':
+
+https://github.com/cracklib/cracklib/commit/33d7fa4585247cd2247a1ffa032ad245836c6edb
+
+From 33d7fa4585247cd2247a1ffa032ad245836c6edb Mon Sep 17 00:00:00 2001
+From: Jan Dittberner <jan@dittberner.info>
+Date: Thu, 25 Aug 2016 17:17:53 +0200
+Subject: [PATCH] Fix a buffer overflow processing long words
+
+A buffer overflow processing long words has been discovered. This commit
+applies the patch from
+https://build.opensuse.org/package/view_file/Base:System/cracklib/0004-overflow-processing-long-words.patch
+by Howard Guo.
+
+See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=835386 and
+http://www.openwall.com/lists/oss-security/2016/08/23/8
+---
+ src/NEWS        | 1 +
+ src/lib/rules.c | 5 ++---
+ 2 files changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/src/lib/rules.c b/src/lib/rules.c
+index d193cc0..3a2aa46 100644
+--- a/lib/rules.c
++++ b/lib/rules.c
+@@ -434,9 +434,8 @@ Mangle(input, control)		/* returns a pointer to a controlled Mangle */
+ {
+     int limit;
+     register char *ptr;
+-    static char area[STRINGSIZE];
+-    char area2[STRINGSIZE];
+-    area[0] = '\0';
++    static char area[STRINGSIZE * 2] = {0};
++    char area2[STRINGSIZE * 2] = {0};
+     strcpy(area, input);
+ 
+     for (ptr = control; *ptr; ptr++)