[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091.
|
From: |
Leo Famulari |
|
Subject: |
Re: [PATCH 1/1] gnu: unrtf: Fix CVE-2016-10091. |
|
Date: |
Wed, 4 Jan 2017 02:13:25 -0500 |
|
User-agent: |
Mutt/1.7.2 (2016-11-26) |
On Tue, Jan 03, 2017 at 05:49:29PM +0100, Marius Bakke wrote:
> Leo Famulari <address@hidden> 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 <address@hidden>
|# 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.
signature.asc
Description: PGP signature