Segmentation fault in gdal_rasterize

[jcrepetto]

When gdal_rasterize is used with a raster data file which is not georeferenced, a segmentation fault occurs.

Steps to reproduce :

Download a bitmap file of the world (​http://veimages.gsfc.nasa.gov//2433/land_shallow_topo_2048.tif) and a shape file of the administrative limits (​http://www.pagcgeo.org/world_factbk.zip).

gdal_rasterize -b 1 -b 2 -b 3 -burn 255 -burn 0 -burn 0 -l world_factbk world_factbk.shp land_shallow_topo_2048.tif

GDAL: Auto register /usr/lib/gdalplugins/gdal_GRASS.so using GDALRegister_GRASS.
OGR: Auto register /usr/lib/gdalplugins/ogr_GRASS.so using RegisterOGRGRASS.
GRASS: OGRGRASSDataSource::Open
GRASS: OGRGRASSDataSource::~OGRGRASSDataSource()
OGR: OGROpen(world_factbk.shp/0x616720) succeeded as ESRI Shapefile.
GDAL: GDALOpen(land_shallow_topo_2048.tif) succeeds as GTiff.
0100 - done.
Shape: 27459 features read on layer 'world_factbk'.
Segmentation fault

Expected result

gdal_rasterize should exit with an error message such as "raster data not georeferenced".

I have not tested with a non-referenced vector data file, but it should also be checked.

[warmerdam]

Mateusz, could you try to address this for 1.4.3?

[Mateusz Łoskot]

A few notes after spending long time in front of debugger:

• not geo-referenced raster is the reason that default transformer is created using GDALCreateGenImgProjTransformer

• this transformer does not change geographical coordinates of vector geometries being burned (ie. -178 lon stays unchanged)

• negative coordinates are passed to chain of functions
  • gv_rasterize_new_one_shape() - this algorithm assumes positive value of coordinates as scanline boundaries (min X and max X)
    • negative coordinates are forwarded to gvBurnScanline()

• function gvBurnScanline(), gdalrasterize.cpp:97, if negative coordinates have been passed, value of nXEnd is also negative what causes errors in following calculation of number of pixels to be burned:

memset( pabyInsert, nBurnValue, nXEnd - nXStart + 1 );

• the incorrect calculation breaks the raster data completely

• obviously, omitting negative coordinates does not lead to correct results

In my opinion, reasonable solution would be to do following:

• track original scanline boundaries (original values of nXStart and nXEnd for current scanline)

• fall back with burning old pixel values (existing in original raster) in old place (old scanline boundaries being tracked, see above), instead of burning new value in wrong place

...but I'm not sure if this idea makes sense at all, so I'm looking forward your comments.

[Even Rouault]

Mateusz, I'm afraid this has nothing to do with the rasterization process itself. The issue here is that the TIF is a LZW TIF... And GDAL has some known problems when updating a LZW TIF (see #1758 for example). (Side note : after having tried to fix #1758, I'm wondering if it's even possible to update safely a compressed TIF at all...)

I've tried to gdal_translate the source tif into an uncompressed tif and done the rasterization on it. Contrary to the LZW case, the result on the uncompressed tif does not look as a TV not receiving signals. The result is incorrect of course as the raster has no georeferencing.

If you use the following TFW, the result will be good (on the uncompressed TIF):

0.175781
0
0
-0.175781
-179.9121095
89.9121095

As suggested by jcrepetto, I think we should at least issue a warning when raster has no georeferencing but vector yes, and the other way round.

And probably we should refuse to open LZW TIF in GA_Update mode.

[Mateusz Łoskot]

Even,

Great catch, thanks! I haven't been aware of that problem. However, I'm not sure about issuing warning.

Frank,

I suggested similar solutions yesterday:

1. Issue warning and skip negative values or reset to 0 just before burning them, in gvBurnScanline()
2. Throw error message and abort.

You weren't convinced to that idea.

Generally, if I'm correct, we can not do any thing to produce correct results, so we can only prevent GDLA from crashing. Then, the goal is to avoid CPLAssert(nEndX >= 0) caught in the gvBurnScanline() function, but in my opinion just removing this assert won't prevent from crashes (the 3rd argument for memset() might cause buffer overrun). So, we can skip (as said in point 1.) or reset to zero to at least not to corrupt memory when calling memset().

How about this explanation?

[Mateusz Łoskot]

The bug has been fixed in trunk (r12556) and branches/1.4 r(12557).

I'd like to thank to Even for his effort in investigating and patching this bug.