tags 533361 patch security
thanks
Upstream auhor of xcftools here.
Thanks for this bug report.
There is indeed a possible buffer overflow when the converted part of
the image extends above or to the left of the canvas. It was caused by
me foolishly assuming that C's modulus operator bevhaves sanely on
negative numbers.
A patch that ought to fix this is attached.
Jörgen, can you check whether this does fix your problem?
If so, I'll release a fixed version as 1.0.5 asap.
I don't THINK we need to go into a full-tilt security panic for this
one. Yes, the stack gets overwritten, but only if the -C or -O options
are used to shift the origin. Therefore, in order to mount an attack
one not only has to trick the victim into converting an appropriately
crafted image, but also to use one of these "advanced" options.
I'll tag it "security" nonetheless, so that others get a chance
to weigh in.
--
Henning Makholm "We will discuss your youth another time."
Index: flatten.c
===================================================================
RCS file: /home/makcvs/repository/source/xcftools/flatten.c,v
retrieving revision 1.27
diff -u -r1.27 flatten.c
--- flatten.c 22 Feb 2006 00:01:04 -0000 1.27
+++ flatten.c 18 Jun 2009 23:11:42 -0000
@@ -619,14 +619,14 @@
fillTile(&toptile,0);
for( where.t = spec->dim.c.t; where.t < spec->dim.c.b; where.t=where.b ) {
- where.b = (where.t+TILE_HEIGHT) - where.t % TILE_HEIGHT ;
+ where.b = TILE_TOP(where.t)+TILE_HEIGHT ;
if( where.b > spec->dim.c.b ) where.b = spec->dim.c.b ;
nrows = where.b - where.t ;
for( y = 0; y < nrows ; y++ )
rows[y] = xcfmalloc(4*(spec->dim.c.r-spec->dim.c.l));
for( where.l = spec->dim.c.l; where.l < spec->dim.c.r; where.l=where.r ) {
- where.r = (where.l+TILE_WIDTH) - where.l % TILE_WIDTH ;
+ where.r = TILE_LEFT(where.l)+TILE_WIDTH ;
if( where.r > spec->dim.c.r ) where.r = spec->dim.c.r ;
ncols = where.r - where.l ;
Index: xcftools.h
===================================================================
RCS file: /home/makcvs/repository/source/xcftools/xcftools.h,v
retrieving revision 1.17
diff -u -r1.17 xcftools.h
--- xcftools.h 13 Feb 2006 03:04:03 -0000 1.17
+++ xcftools.h 18 Jun 2009 23:11:42 -0000
@@ -146,6 +146,13 @@
/* These are hardcoded in the Gimp sources: */
#define TILE_WIDTH 64
#define TILE_HEIGHT 64
+/* These definitions of TILE_LEFT and TILE_TOP work correctly for negative
+ * numbers, but on the other hand depend on TILE_WIDTH and TILE_HEIGHT
+ * being powers of 2. That's okay, because the tile size cannot change
+ * anyway.
+ */
+#define TILE_LEFT(x) ((x) & -TILE_WIDTH)
+#define TILE_TOP(y) ((y) & -TILE_HEIGHT)
struct tileDimensions {
struct rect c ;
