* [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.
@ 2017-01-01 20:29 Leo Famulari
2017-01-03 16:49 ` Marius Bakke
0 siblings, 1 reply; 6+ messages in thread
From: Leo Famulari @ 2017-01-01 20:29 UTC (permalink / raw)
To: guix-devel
* gnu/packages/patches/unrtf-CVE-2016-10091.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/unrtf.scm (unrtf)[source]: Use it.
---
gnu/local.mk | 1 +
gnu/packages/patches/unrtf-CVE-2016-10091.patch | 224 ++++++++++++++++++++++++
gnu/packages/unrtf.scm | 2 +
3 files changed, 227 insertions(+)
create mode 100644 gnu/packages/patches/unrtf-CVE-2016-10091.patch
diff --git a/gnu/local.mk b/gnu/local.mk
index 6ab1c1c48..58878e8e4 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -878,6 +878,7 @@ dist_patch_DATA = \
%D%/packages/patches/tophat-build-with-later-seqan.patch \
%D%/packages/patches/totem-debug-format-fix.patch \
%D%/packages/patches/tuxpaint-stamps-path.patch \
+ %D%/packages/patches/unrtf-CVE-2016-10091.patch \
%D%/packages/patches/unzip-CVE-2014-8139.patch \
%D%/packages/patches/unzip-CVE-2014-8140.patch \
%D%/packages/patches/unzip-CVE-2014-8141.patch \
diff --git a/gnu/packages/patches/unrtf-CVE-2016-10091.patch b/gnu/packages/patches/unrtf-CVE-2016-10091.patch
new file mode 100644
index 000000000..0a58b40db
--- /dev/null
+++ b/gnu/packages/patches/unrtf-CVE-2016-10091.patch
@@ -0,0 +1,224 @@
+Fix CVE-2016-10091 (stack-based buffer overflows in cmd_* functions):
+
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10091
+https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=849705
+http://seclists.org/oss-sec/2016/q4/787
+
+Patch copied from Debian:
+
+https://anonscm.debian.org/cgit/collab-maint/unrtf.git/commit/?h=jessie&id=7500a48fb0fbad3ab963fb17560b2f90a8a485c8
+
+The Debian patch adapts this upstream commit so that it can be applied
+to the 0.21.9 release tarball:
+
+http://hg.savannah.gnu.org/hgweb/unrtf/rev/3b16893a6406
+
+From 7dd568ed8a6a5acb6c04f2b40f457d63a00435f3 Mon Sep 17 00:00:00 2001
+From: Willi Mann <willi@debian.org>
+Date: Sat, 31 Dec 2016 20:31:38 +0100
+Subject: [PATCH] Add patch from upstream to fix CVE-2016-10091 (buffer
+ overflow in various cmd_ functions)
+
+closes: 849705
+---
+ ...-instances-of-sprintf-with-snprintf-and-a.patch | 179 +++++++++++++++++++++
+ debian/patches/series | 1 +
+ 2 files changed, 180 insertions(+)
+ create mode 100644 debian/patches/0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch
+ create mode 100644 debian/patches/series
+
+diff --git a/debian/patches/0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch b/debian/patches/0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch
+new file mode 100644
+index 0000000..2f3825a
+--- /dev/null
++++ b/debian/patches/0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch
+@@ -0,0 +1,179 @@
++From: Jean-Francois Dockes <jf@dockes.org>
++Date: Sat, 31 Dec 2016 20:25:19 +0100
++Subject: Replace all instances of sprintf with snprintf and adjust size of
++ integer field in some cases
++
++This fixes CVE-2016-10091
++
++Bug-Debian: https://bugs.debian.org/849705
++---
++ src/attr.c | 4 ++--
++ src/convert.c | 28 ++++++++++++++--------------
++ src/output.c | 4 ++--
++ 3 files changed, 18 insertions(+), 18 deletions(-)
++
++diff --git a/src/attr.c b/src/attr.c
++index 02b5c81..e2951ea 100644
++--- a/src/attr.c
+++++ b/src/attr.c
++@@ -746,7 +746,7 @@ char *
++ assemble_string(char *string, int nr)
++ {
++
++- char *s, tmp[12];/* Number of characters that can be in int type (including '\0') - AF */
+++ char *s, tmp[20];
++ int i = 0, j = 0;
++
++ if (string == NULL)
++@@ -762,7 +762,7 @@ assemble_string(char *string, int nr)
++ }
++
++ if (string[i] != '\0') {
++- sprintf(tmp, "%d", nr);
+++ snprintf(tmp, 20, "%d", nr);
++ strcpy(&s[j], tmp);
++ j = j + strlen(tmp);
++ }
++diff --git a/src/convert.c b/src/convert.c
++index c76d7d6..8eacdcb 100644
++--- a/src/convert.c
+++++ b/src/convert.c
++@@ -472,7 +472,7 @@ static const int fcharsetparmtocp(int parm)
++ }
++
++ // Translate code page to encoding name hopefully suitable as iconv input
++-static char *cptoencoding(parm)
+++static char *cptoencoding(int parm)
++ {
++ // Note that CP0 is supposed to mean current system default, which does
++ // not make any sense as a stored value, we don't handle it.
++@@ -964,7 +964,7 @@ cmd_cf (Word *w, int align, char has_param, int num)
++ }
++ else
++ {
++- sprintf(str,"#%02x%02x%02x",
+++ snprintf(str, 40, "#%02x%02x%02x",
++ color_table[num].r,
++ color_table[num].g,
++ color_table[num].b);
++@@ -993,7 +993,7 @@ cmd_cb (Word *w, int align, char has_param, int num)
++ }
++ else
++ {
++- sprintf(str,"#%02x%02x%02x",
+++ snprintf(str, 40, "#%02x%02x%02x",
++ color_table[num].r,
++ color_table[num].g,
++ color_table[num].b);
++@@ -1018,7 +1018,7 @@ cmd_fs (Word *w, int align, char has_param, int points) {
++ /* Note, fs20 means 10pt */
++ points /= 2;
++
++- sprintf(str,"%d",points);
+++ snprintf(str, 20, "%d", points);
++ attr_push(ATTR_FONTSIZE,str);
++
++ return FALSE;
++@@ -1166,7 +1166,7 @@ cmd_f (Word *w, int align, char has_param, int num)
++ {
++ // TOBEDONE: WHAT'S THIS ???
++ name = my_malloc(12);
++- sprintf(name, "%d", num);
+++ snprintf(name, 12, "%d", num);
++ }
++
++ /* we are going to output entities, so should not output font */
++@@ -1218,7 +1218,7 @@ cmd_highlight (Word *w, int align, char has_param, int num)
++ }
++ else
++ {
++- sprintf(str,"#%02x%02x%02x",
+++ snprintf(str, 40, "#%02x%02x%02x",
++ color_table[num].r,
++ color_table[num].g,
++ color_table[num].b);
++@@ -1373,9 +1373,9 @@ cmd_ftech (Word *w, int align, char has_param, int param) {
++
++ static int
++ cmd_expand (Word *w, int align, char has_param, int param) {
++- char str[10];
+++ char str[20];
++ if (has_param) {
++- sprintf(str, "%d", param/4);
+++ snprintf(str, 20, "%d", param / 4);
++ if (!param)
++ attr_pop(ATTR_EXPAND);
++ else
++@@ -1394,7 +1394,7 @@ cmd_expand (Word *w, int align, char has_param, int param) {
++
++ static int
++ cmd_emboss (Word *w, int align, char has_param, int param) {
++- char str[10];
+++ char str[20];
++ if (has_param && !param)
++ #ifdef SUPPORT_UNNESTED
++ attr_find_pop(ATTR_EMBOSS);
++@@ -1403,7 +1403,7 @@ cmd_emboss (Word *w, int align, char has_param, int param) {
++ #endif
++ else
++ {
++- sprintf(str, "%d", param);
+++ snprintf(str, 20, "%d", param);
++ attr_push(ATTR_EMBOSS, str);
++ }
++ return FALSE;
++@@ -1419,12 +1419,12 @@ cmd_emboss (Word *w, int align, char has_param, int param) {
++
++ static int
++ cmd_engrave (Word *w, int align, char has_param, int param) {
++- char str[10];
+++ char str[20];
++ if (has_param && !param)
++ attr_pop(ATTR_ENGRAVE);
++ else
++ {
++- sprintf(str, "%d", param);
+++ snprintf(str, 20, "%d", param);
++ attr_push(ATTR_ENGRAVE, str);
++ }
++ return FALSE;
++@@ -1976,7 +1976,7 @@ static int cmd_u (Word *w, int align, char has_param, int param) {
++
++ short done=0;
++ long unicode_number = (long) param; /* On 16bit architectures int is too small to store unicode characters. - AF */
++- char tmp[12]; /* Number of characters that can be in int type (including '\0'). If int size is greater than 4 bytes change this value. - AF */
+++ char tmp[20]; /* Number of characters that can be in int type (including '\0'). If int size is greater than 4 bytes change this value. - AF */
++ const char *alias;
++ #define DEBUG 0
++ #if DEBUG
++@@ -2006,7 +2006,7 @@ static int cmd_u (Word *w, int align, char has_param, int param) {
++ /* RTF spec: Unicode values beyond 32767 are represented by negative numbers */
++ unicode_number += 65536;
++ }
++- sprintf(tmp, "%ld", unicode_number);
+++ snprintf(tmp, 20, "%ld", unicode_number);
++
++ if (safe_printf(1, op->unisymbol_print, tmp)) fprintf(stderr, TOO_MANY_ARGS, "unisymbol_print");
++ done++;
++diff --git a/src/output.c b/src/output.c
++index 86d8b5c..4cdbfa6 100644
++--- a/src/output.c
+++++ b/src/output.c
++@@ -320,7 +320,7 @@ op_begin_std_fontsize (OutputPersonality *op, int size)
++ if (!found_std_expr) {
++ if (op->fontsize_begin) {
++ char expr[16];
++- sprintf (expr, "%d", size);
+++ snprintf(expr, 16, "%d", size);
++ if (safe_printf (1, op->fontsize_begin, expr)) fprintf(stderr, TOO_MANY_ARGS, "fontsize_begin");
++ } else {
++ /* If we cannot write out a change for the exact
++@@ -440,7 +440,7 @@ op_end_std_fontsize (OutputPersonality *op, int size)
++ if (!found_std_expr) {
++ if (op->fontsize_end) {
++ char expr[16];
++- sprintf (expr, "%d", size);
+++ snprintf(expr, 16, "%d", size);
++ if (safe_printf(1, op->fontsize_end, expr)) fprintf(stderr, TOO_MANY_ARGS, "fontsize_end");
++ } else {
++ /* If we cannot write out a change for the exact
+diff --git a/debian/patches/series b/debian/patches/series
+new file mode 100644
+index 0000000..7868249
+--- /dev/null
++++ b/debian/patches/series
+@@ -0,0 +1 @@
++0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch
+--
+2.11.0
+
diff --git a/gnu/packages/unrtf.scm b/gnu/packages/unrtf.scm
index 162dec752..e11c9445c 100644
--- a/gnu/packages/unrtf.scm
+++ b/gnu/packages/unrtf.scm
@@ -23,6 +23,7 @@
#:use-module (guix download)
#:use-module (guix build-system gnu)
#:use-module (guix gexp)
+ #:use-module (gnu packages)
#:use-module (gnu packages autotools)
#:use-module (gnu packages m4)
#:use-module (gnu packages base))
@@ -35,6 +36,7 @@
(method url-fetch)
(uri (string-append "mirror://gnu/unrtf/unrtf-"
version ".tar.gz"))
+ (patches (search-patches "unrtf-CVE-2016-10091.patch"))
(sha256
(base32
"1pcdzf2h1prn393dkvg93v80vh38q0v817xnbwrlwxbdz4k7i8r2"))
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.
2017-01-01 20:29 [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091 Leo Famulari
@ 2017-01-03 16:49 ` Marius Bakke
2017-01-04 7:13 ` Leo Famulari
0 siblings, 1 reply; 6+ messages in thread
From: Marius Bakke @ 2017-01-03 16:49 UTC (permalink / raw)
To: Leo Famulari, guix-devel
[-- Attachment #1: Type: text/plain, Size: 1615 bytes --]
Leo Famulari <leo@famulari.name> writes:
> * gnu/packages/patches/unrtf-CVE-2016-10091.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/unrtf.scm (unrtf)[source]: Use it.
[...]
> diff --git a/gnu/packages/patches/unrtf-CVE-2016-10091.patch b/gnu/packages/patches/unrtf-CVE-2016-10091.patch
> new file mode 100644
> index 000000000..0a58b40db
> --- /dev/null
> +++ b/gnu/packages/patches/unrtf-CVE-2016-10091.patch
> @@ -0,0 +1,224 @@
> +Fix CVE-2016-10091 (stack-based buffer overflows in cmd_* functions):
> +
> +https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10091
> +https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=849705
> +http://seclists.org/oss-sec/2016/q4/787
> +
> +Patch copied from Debian:
> +
> +https://anonscm.debian.org/cgit/collab-maint/unrtf.git/commit/?h=jessie&id=7500a48fb0fbad3ab963fb17560b2f90a8a485c8
> +
> +The Debian patch adapts this upstream commit so that it can be applied
> +to the 0.21.9 release tarball:
> +
> +http://hg.savannah.gnu.org/hgweb/unrtf/rev/3b16893a6406
Isn't the Debian patch the same as this upstream commit? I can't spot
the difference with a cursory glance.
> +diff --git a/debian/patches/series b/debian/patches/series
> +new file mode 100644
> +index 0000000..7868249
> +--- /dev/null
> ++++ b/debian/patches/series
> +@@ -0,0 +1 @@
> ++0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch
This part we surely don't need ;-)
Unless the Debian patch fixes other issues than upstream revision
3b16893a6406 I would just pick and link to that, skipping the Debian
step. WDYT?
Thanks for taking care of this!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.
2017-01-03 16:49 ` Marius Bakke
@ 2017-01-04 7:13 ` Leo Famulari
2017-01-04 7:27 ` Leo Famulari
0 siblings, 1 reply; 6+ messages in thread
From: Leo Famulari @ 2017-01-04 7:13 UTC (permalink / raw)
To: Marius Bakke; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 3797 bytes --]
On Tue, Jan 03, 2017 at 05:49:29PM +0100, Marius Bakke wrote:
> Leo Famulari <leo@famulari.name> writes:
> > +Patch copied from Debian:
> > +
> > +https://anonscm.debian.org/cgit/collab-maint/unrtf.git/commit/?h=jessie&id=7500a48fb0fbad3ab963fb17560b2f90a8a485c8
> > +
> > +The Debian patch adapts this upstream commit so that it can be applied
> > +to the 0.21.9 release tarball:
> > +
> > +http://hg.savannah.gnu.org/hgweb/unrtf/rev/3b16893a6406
>
> Isn't the Debian patch the same as this upstream commit? I can't spot
> the difference with a cursory glance.
The upstream patch mostly fails to apply:
------
can't find file to patch at input line 12
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|
|# HG changeset patch
|# User Jean-Francois Dockes <jf@dockes.org>
|# Date 1483180555 -3600
|# Node ID 3b16893a6406b548ff9c3a5966b47e11773eb671
|# Parent f37dd4dea9c1b97e2563759b1164573d42d679dd
|Replace all instances of sprintf with snprintf and adjust size of integer field in some cases
|
|diff --git a/Windows/unrtf_w.c b/Windows/unrtf_w.c
|--- a/Windows/unrtf_w.c
|+++ b/Windows/unrtf_w.c
--------------------------
No file to patch. Skipping patch.
1 out of 1 hunk ignored
patching file src/attr.c
Hunk #1 succeeded at 746 (offset -218 lines).
Hunk #2 FAILED at 986.
1 out of 2 hunks FAILED -- saving rejects to file src/attr.c.rej
patching file src/convert.c
Hunk #1 succeeded at 472 with fuzz 2 (offset -83 lines).
Hunk #2 FAILED at 1383.
Hunk #3 FAILED at 1412.
Hunk #4 FAILED at 1441.
Hunk #5 FAILED at 1630.
Hunk #6 FAILED at 1687.
Hunk #7 FAILED at 1855.
Hunk #8 FAILED at 1882.
Hunk #9 succeeded at 1403 (offset -488 lines).
Hunk #10 FAILED at 1908.
Hunk #11 succeeded at 1976 with fuzz 2 (offset -585 lines).
Hunk #12 FAILED at 2590.
Hunk #13 FAILED at 4214.
10 out of 13 hunks FAILED -- saving rejects to file src/convert.c.rej
patching file src/output.c
Hunk #1 FAILED at 402.
Hunk #2 FAILED at 629.
2 out of 2 hunks FAILED -- saving rejects to file src/output.c.rej
source is under 'unrtf-0.21.9'
------
Based on my comparison of the respective patches from UnRTF and Debian,
I think there have been widespread stylistic changes in the upstream
code. For example, the upstream patch has this hunk in 'src/attr.c',
which fails to apply to the tarball:
@@ -986,7 +986,7 @@
if (string[i] != '\0')
{
- sprintf(tmp, "%d", nr);
+ snprintf(tmp, 20, "%d", nr);
strcpy(&s[j], tmp);
j = j + strlen(tmp);
}
Whereas Debian's hunk is like this:
@@ -762,7 +762,7 @@ assemble_string(char *string, int nr)
}
if (string[i] != '\0') {
- sprintf(tmp, "%d", nr);
+ snprintf(tmp, 20, "%d", nr);
strcpy(&s[j], tmp);
j = j + strlen(tmp);
}
The conditional's opening bracket has been moved. There are lots of
these issues in the upstream patch. It's not really a "bugfix patch" but
merely the upstream commit that happens to fix this bug.
> > +diff --git a/debian/patches/series b/debian/patches/series
> > +new file mode 100644
> > +index 0000000..7868249
> > +--- /dev/null
> > ++++ b/debian/patches/series
> > +@@ -0,0 +1 @@
> > ++0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch
>
> This part we surely don't need ;-)
Oops!
> Unless the Debian patch fixes other issues than upstream revision
> 3b16893a6406 I would just pick and link to that, skipping the Debian
> step. WDYT?
I think we should use Debian's patch.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.
2017-01-04 7:13 ` Leo Famulari
@ 2017-01-04 7:27 ` Leo Famulari
2017-01-04 7:50 ` mcrypt CVE-2012-{4409,4527} [was Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.] Leo Famulari
2017-01-04 15:09 ` [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091 Marius Bakke
0 siblings, 2 replies; 6+ messages in thread
From: Leo Famulari @ 2017-01-04 7:27 UTC (permalink / raw)
To: Marius Bakke; +Cc: guix-devel
[-- Attachment #1.1: Type: text/plain, Size: 732 bytes --]
On Wed, Jan 04, 2017 at 02:13:25AM -0500, Leo Famulari wrote:
> On Tue, Jan 03, 2017 at 05:49:29PM +0100, Marius Bakke wrote:
> > Leo Famulari <leo@famulari.name> writes:
> > > +diff --git a/debian/patches/series b/debian/patches/series
> > > +new file mode 100644
> > > +index 0000000..7868249
> > > +--- /dev/null
> > > ++++ b/debian/patches/series
> > > +@@ -0,0 +1 @@
> > > ++0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch
> >
> > This part we surely don't need ;-)
>
> Oops!
x2
Of course, the patch I sent on January 1 was completely broken.
The patch it included from Debian was meant to be applied to the Debian
package tree, not the UnRTF source code.
I've attached a revision!
[-- Attachment #1.2: 0001-gnu-unrtf-Fix-CVE-2016-10091.patch --]
[-- Type: text/plain, Size: 8802 bytes --]
From 0799beece00361cbaa2e1d9796532de0f5f55ff9 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Sun, 1 Jan 2017 15:12:48 -0500
Subject: [PATCH] gnu: unrtf: Fix CVE-2016-10091.
* gnu/packages/patches/unrtf-CVE-2016-10091.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/unrtf.scm (unrtf)[source]: Use it.
---
gnu/local.mk | 1 +
gnu/packages/patches/unrtf-CVE-2016-10091.patch | 189 ++++++++++++++++++++++++
gnu/packages/unrtf.scm | 2 +
3 files changed, 192 insertions(+)
create mode 100644 gnu/packages/patches/unrtf-CVE-2016-10091.patch
diff --git a/gnu/local.mk b/gnu/local.mk
index f45030f84..00edacad7 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -878,6 +878,7 @@ dist_patch_DATA = \
%D%/packages/patches/tophat-build-with-later-seqan.patch \
%D%/packages/patches/totem-debug-format-fix.patch \
%D%/packages/patches/tuxpaint-stamps-path.patch \
+ %D%/packages/patches/unrtf-CVE-2016-10091.patch \
%D%/packages/patches/unzip-CVE-2014-8139.patch \
%D%/packages/patches/unzip-CVE-2014-8140.patch \
%D%/packages/patches/unzip-CVE-2014-8141.patch \
diff --git a/gnu/packages/patches/unrtf-CVE-2016-10091.patch b/gnu/packages/patches/unrtf-CVE-2016-10091.patch
new file mode 100644
index 000000000..badd1b8ed
--- /dev/null
+++ b/gnu/packages/patches/unrtf-CVE-2016-10091.patch
@@ -0,0 +1,189 @@
+Fix CVE-2016-10091 (stack-based buffer overflows in cmd_* functions):
+
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10091
+https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=849705
+http://seclists.org/oss-sec/2016/q4/787
+
+Patch adapted from Debian:
+
+https://anonscm.debian.org/cgit/collab-maint/unrtf.git/commit/?h=jessie&id=7500a48fb0fbad3ab963fb17560b2f90a8a485c8
+
+The Debian patch adapts this upstream commit so that it can be applied
+to the 0.21.9 release tarball:
+
+http://hg.savannah.gnu.org/hgweb/unrtf/rev/3b16893a6406
+
+From 7dd568ed8a6a5acb6c04f2b40f457d63a00435f3 Mon Sep 17 00:00:00 2001
+From: Willi Mann <willi@debian.org>
+Date: Sat, 31 Dec 2016 20:31:38 +0100
+Subject: [PATCH] Add patch from upstream to fix CVE-2016-10091 (buffer
+ overflow in various cmd_ functions)
+
+diff --git a/src/attr.c b/src/attr.c
+index 02b5c81..e2951ea 100644
+--- a/src/attr.c
++++ b/src/attr.c
+@@ -746,7 +746,7 @@ char *
+ assemble_string(char *string, int nr)
+ {
+
+- char *s, tmp[12];/* Number of characters that can be in int type (including '\0') - AF */
++ char *s, tmp[20];
+ int i = 0, j = 0;
+
+ if (string == NULL)
+@@ -762,7 +762,7 @@ assemble_string(char *string, int nr)
+ }
+
+ if (string[i] != '\0') {
+- sprintf(tmp, "%d", nr);
++ snprintf(tmp, 20, "%d", nr);
+ strcpy(&s[j], tmp);
+ j = j + strlen(tmp);
+ }
+diff --git a/src/convert.c b/src/convert.c
+index c76d7d6..8eacdcb 100644
+--- a/src/convert.c
++++ b/src/convert.c
+@@ -472,7 +472,7 @@ static const int fcharsetparmtocp(int parm)
+ }
+
+ // Translate code page to encoding name hopefully suitable as iconv input
+-static char *cptoencoding(parm)
++static char *cptoencoding(int parm)
+ {
+ // Note that CP0 is supposed to mean current system default, which does
+ // not make any sense as a stored value, we don't handle it.
+@@ -964,7 +964,7 @@ cmd_cf (Word *w, int align, char has_param, int num)
+ }
+ else
+ {
+- sprintf(str,"#%02x%02x%02x",
++ snprintf(str, 40, "#%02x%02x%02x",
+ color_table[num].r,
+ color_table[num].g,
+ color_table[num].b);
+@@ -993,7 +993,7 @@ cmd_cb (Word *w, int align, char has_param, int num)
+ }
+ else
+ {
+- sprintf(str,"#%02x%02x%02x",
++ snprintf(str, 40, "#%02x%02x%02x",
+ color_table[num].r,
+ color_table[num].g,
+ color_table[num].b);
+@@ -1018,7 +1018,7 @@ cmd_fs (Word *w, int align, char has_param, int points) {
+ /* Note, fs20 means 10pt */
+ points /= 2;
+
+- sprintf(str,"%d",points);
++ snprintf(str, 20, "%d", points);
+ attr_push(ATTR_FONTSIZE,str);
+
+ return FALSE;
+@@ -1166,7 +1166,7 @@ cmd_f (Word *w, int align, char has_param, int num)
+ {
+ // TOBEDONE: WHAT'S THIS ???
+ name = my_malloc(12);
+- sprintf(name, "%d", num);
++ snprintf(name, 12, "%d", num);
+ }
+
+ /* we are going to output entities, so should not output font */
+@@ -1218,7 +1218,7 @@ cmd_highlight (Word *w, int align, char has_param, int num)
+ }
+ else
+ {
+- sprintf(str,"#%02x%02x%02x",
++ snprintf(str, 40, "#%02x%02x%02x",
+ color_table[num].r,
+ color_table[num].g,
+ color_table[num].b);
+@@ -1373,9 +1373,9 @@ cmd_ftech (Word *w, int align, char has_param, int param) {
+
+ static int
+ cmd_expand (Word *w, int align, char has_param, int param) {
+- char str[10];
++ char str[20];
+ if (has_param) {
+- sprintf(str, "%d", param/4);
++ snprintf(str, 20, "%d", param / 4);
+ if (!param)
+ attr_pop(ATTR_EXPAND);
+ else
+@@ -1394,7 +1394,7 @@ cmd_expand (Word *w, int align, char has_param, int param) {
+
+ static int
+ cmd_emboss (Word *w, int align, char has_param, int param) {
+- char str[10];
++ char str[20];
+ if (has_param && !param)
+ #ifdef SUPPORT_UNNESTED
+ attr_find_pop(ATTR_EMBOSS);
+@@ -1403,7 +1403,7 @@ cmd_emboss (Word *w, int align, char has_param, int param) {
+ #endif
+ else
+ {
+- sprintf(str, "%d", param);
++ snprintf(str, 20, "%d", param);
+ attr_push(ATTR_EMBOSS, str);
+ }
+ return FALSE;
+@@ -1419,12 +1419,12 @@ cmd_emboss (Word *w, int align, char has_param, int param) {
+
+ static int
+ cmd_engrave (Word *w, int align, char has_param, int param) {
+- char str[10];
++ char str[20];
+ if (has_param && !param)
+ attr_pop(ATTR_ENGRAVE);
+ else
+ {
+- sprintf(str, "%d", param);
++ snprintf(str, 20, "%d", param);
+ attr_push(ATTR_ENGRAVE, str);
+ }
+ return FALSE;
+@@ -1976,7 +1976,7 @@ static int cmd_u (Word *w, int align, char has_param, int param) {
+
+ short done=0;
+ long unicode_number = (long) param; /* On 16bit architectures int is too small to store unicode characters. - AF */
+- char tmp[12]; /* Number of characters that can be in int type (including '\0'). If int size is greater than 4 bytes change this value. - AF */
++ char tmp[20]; /* Number of characters that can be in int type (including '\0'). If int size is greater than 4 bytes change this value. - AF */
+ const char *alias;
+ #define DEBUG 0
+ #if DEBUG
+@@ -2006,7 +2006,7 @@ static int cmd_u (Word *w, int align, char has_param, int param) {
+ /* RTF spec: Unicode values beyond 32767 are represented by negative numbers */
+ unicode_number += 65536;
+ }
+- sprintf(tmp, "%ld", unicode_number);
++ snprintf(tmp, 20, "%ld", unicode_number);
+
+ if (safe_printf(1, op->unisymbol_print, tmp)) fprintf(stderr, TOO_MANY_ARGS, "unisymbol_print");
+ done++;
+diff --git a/src/output.c b/src/output.c
+index 86d8b5c..4cdbfa6 100644
+--- a/src/output.c
++++ b/src/output.c
+@@ -320,7 +320,7 @@ op_begin_std_fontsize (OutputPersonality *op, int size)
+ if (!found_std_expr) {
+ if (op->fontsize_begin) {
+ char expr[16];
+- sprintf (expr, "%d", size);
++ snprintf(expr, 16, "%d", size);
+ if (safe_printf (1, op->fontsize_begin, expr)) fprintf(stderr, TOO_MANY_ARGS, "fontsize_begin");
+ } else {
+ /* If we cannot write out a change for the exact
+@@ -440,7 +440,7 @@ op_end_std_fontsize (OutputPersonality *op, int size)
+ if (!found_std_expr) {
+ if (op->fontsize_end) {
+ char expr[16];
+- sprintf (expr, "%d", size);
++ snprintf(expr, 16, "%d", size);
+ if (safe_printf(1, op->fontsize_end, expr)) fprintf(stderr, TOO_MANY_ARGS, "fontsize_end");
+ } else {
+ /* If we cannot write out a change for the exact
+-
+.11.0
+
diff --git a/gnu/packages/unrtf.scm b/gnu/packages/unrtf.scm
index 162dec752..e11c9445c 100644
--- a/gnu/packages/unrtf.scm
+++ b/gnu/packages/unrtf.scm
@@ -23,6 +23,7 @@
#:use-module (guix download)
#:use-module (guix build-system gnu)
#:use-module (guix gexp)
+ #:use-module (gnu packages)
#:use-module (gnu packages autotools)
#:use-module (gnu packages m4)
#:use-module (gnu packages base))
@@ -35,6 +36,7 @@
(method url-fetch)
(uri (string-append "mirror://gnu/unrtf/unrtf-"
version ".tar.gz"))
+ (patches (search-patches "unrtf-CVE-2016-10091.patch"))
(sha256
(base32
"1pcdzf2h1prn393dkvg93v80vh38q0v817xnbwrlwxbdz4k7i8r2"))
--
2.11.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread* mcrypt CVE-2012-{4409,4527} [was Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.]
2017-01-04 7:27 ` Leo Famulari
@ 2017-01-04 7:50 ` Leo Famulari
2017-01-04 15:09 ` [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091 Marius Bakke
1 sibling, 0 replies; 6+ messages in thread
From: Leo Famulari @ 2017-01-04 7:50 UTC (permalink / raw)
To: Marius Bakke; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 365 bytes --]
On Wed, Jan 04, 2017 at 02:27:57AM -0500, Leo Famulari wrote:
> Of course, the patch I sent on January 1 was completely broken.
>
> The patch it included from Debian was meant to be applied to the Debian
> package tree, not the UnRTF source code.
I found another instance of this in mcrypt, which I just fixed in
324f4fc559b4cf9f7df0bc334ac8a0a4fa040c22.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.
2017-01-04 7:27 ` Leo Famulari
2017-01-04 7:50 ` mcrypt CVE-2012-{4409,4527} [was Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.] Leo Famulari
@ 2017-01-04 15:09 ` Marius Bakke
1 sibling, 0 replies; 6+ messages in thread
From: Marius Bakke @ 2017-01-04 15:09 UTC (permalink / raw)
To: Leo Famulari; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 927 bytes --]
Leo Famulari <leo@famulari.name> writes:
> On Wed, Jan 04, 2017 at 02:13:25AM -0500, Leo Famulari wrote:
>> On Tue, Jan 03, 2017 at 05:49:29PM +0100, Marius Bakke wrote:
>> > Leo Famulari <leo@famulari.name> writes:
>> > > +diff --git a/debian/patches/series b/debian/patches/series
>> > > +new file mode 100644
>> > > +index 0000000..7868249
>> > > +--- /dev/null
>> > > ++++ b/debian/patches/series
>> > > +@@ -0,0 +1 @@
>> > > ++0001-Replace-all-instances-of-sprintf-with-snprintf-and-a.patch
>> >
>> > This part we surely don't need ;-)
>>
>> Oops!
>
> x2
>
> Of course, the patch I sent on January 1 was completely broken.
>
> The patch it included from Debian was meant to be applied to the Debian
> package tree, not the UnRTF source code.
OK! Thanks for the update, LGTM!
Thanks for keeping up with this. I'm currently travelling, but should
return to normal within a few days/week :-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-04 15:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-01 20:29 [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091 Leo Famulari
2017-01-03 16:49 ` Marius Bakke
2017-01-04 7:13 ` Leo Famulari
2017-01-04 7:27 ` Leo Famulari
2017-01-04 7:50 ` mcrypt CVE-2012-{4409,4527} [was Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.] Leo Famulari
2017-01-04 15:09 ` [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091 Marius Bakke
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).