xcftools flattenIncrementally rows allocation code execution vulnerability

Summary

An exploitable integer overflow vulnerability exists in the flattenIncrementally function in the xcf2png and xcf2pnm binaries of xcftools 1.0.7. An integer overflow can occur while calculating the row’s allocation size, that could be exploited to corrupt memory and eventually execute arbitrary code. In order to trigger this vulnerability, a victim would need to open a specially crafted XCF file.

Tested Versions

xcftools 1.0.7

Product URLs

http://henning.makholm.net/xcftools/

CVSSv3 Score

8.8 - CVSS:3.0/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H

CWE

CWE-680: Integer Overflow to Buffer Overflow

Details

Xcftools is a set of tools for handling Gimp’s XCF files. It provides an xcfinfo tool for extracting information from an XCF file, and xcf2png and xcf2pnm for converting an XCF to PNG or PNM format.

When converting an XCF, the utilities first parse the file using the function getBasicXcfInfo. For each XCF layer, the function computeDimensions is called:

struct tileDimensions {
  struct rect c ;
  unsigned width, height ;
  unsigned tilesx, tilesy ;
  unsigned ntiles ;
};

struct rect {
  int t, b, l, r ;
};

void
computeDimensions(struct tileDimensions *d)
{
  d->c.r = d->c.l + d->width ;                            // [1]
  d->c.b = d->c.t + d->height ;
  d->tilesx = (d->width+TILE_WIDTH-1)/TILE_WIDTH ;
  d->tilesy = (d->height+TILE_HEIGHT-1)/TILE_HEIGHT ;
  d->ntiles = d->tilesx * d->tilesy ;
}

The function takes a structure containing some information about the layer: width, height, horizontal and vertical offset. These values are all gathered from the input file. The code calculates the bounds of the layer by filling the rect structure.

As we can see at [1], the “horizontal offset” (d->c.l) is used together with the layer width, to calculate the “right” field of the rectangle.

When the flattenIncrementally function is called, the size of each row is calculated based on the values r and l alone:

void
flattenIncrementally(struct FlattenSpec *spec,lineCallback callback)
{
  rgba *rows[TILE_HEIGHT] ;
  unsigned i, y, nrows, ncols ;
  struct rect where ;
  struct Tile *tile ;
  static struct Tile toptile ;

  toptile.count = TILE_HEIGHT * TILE_WIDTH ;
  fillTile(&toptile,0);

  for( where.t = spec->dim.c.t; where.t < spec->dim.c.b; where.t=where.b ) {
    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));               // [2]

    for( where.l = spec->dim.c.l; where.l < spec->dim.c.r; where.l=where.r ) {
      ...
      for( y = 0 ; y < nrows ; y++ )
        memcpy(rows[y] + (where.l - spec->dim.c.l),                       // [3]
               tile->pixels + y * ncols, ncols*4);
      ...
    }
    for( y = 0 ; y < nrows ; y++ )
      callback(spec->dim.width,rows[y]);                                  // [4]
  }
}

At [2], a row is allocated using the width multiplied by 4. This operation doesn’t take integer overflows into account, so any width larger than 0x3fffffff would result in an overflow.

Any logic writing to rows after this point, will risk corrupting the heap, leading to arbitrary code execution. There are at least two paths for corruption, at [3] and [4] (which would be triggered by any callback writing to a row, for example by optimistic_palette_callback which calls palettify_row).

Timeline

2019-07-31 - Initial contact
2019-08-07 - Plain text file sent
2019-10-02 - 60+ day follow up
2019-10-21 - 90 day notice
2019-11-21 - Public release