Opened 7 years ago

Closed 6 years ago

#4045 closed defect (fixed)

[VRT] [PATCH] VRTDerivedRasterBand with ComplexSource and nodata value yields potentially uninitialized buffer

Reported by: bclaywell Owned by: warmerdam
Priority: normal Milestone: 1.8.1
Component: GDAL_Raster Version: 1.8.0
Severity: normal Keywords:
Cc: klaricmn, antonio

Description

(This bug was observed in 1.8.0 but it looks like the affected code is still the same in trunk, so the traclinks below go to trunk.)

VRTComplexSource::RasterIO appears to expect that the buffer pointed to by its pData argument has already been initialized with zero or a nodata value by its caller, as the per-pixel loop simply skips to the next pixel if a nodata value is encountered, as seen here.

When VRTComplexSource::RasterIO is called by VRTSourcedRasterBand::IRasterIO, this is not a problem, as the buffer is correctly initialized with zeroes or nodata, starting here.

However, VRTDerivedRasterBand::IRasterIO does *not* initialize its buffers before calling out to the individual sources to populate those buffers; see here. The buffers are simply malloc'd and then immediately passed to VRTSource::RasterIO to be populated, with no initialization.

In other words, any pixel in the source image that equals the ComplexSource's nodata value will never be initialized and will instead contain whatever garbage happened to be at those locations in memory.

Attachments (1)

gdal-1.8.0-vrtderivedrasterband-buffer-fix.patch (2.1 KB) - added by bclaywell 7 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by klaricmn

Cc: klaricmn added

Changed 7 years ago by bclaywell

comment:2 Changed 7 years ago by bclaywell

Summary: [VRT] VRTDerivedRasterBand with ComplexSource and nodata value yields potentially uninitialized buffer[VRT] [PATCH] VRTDerivedRasterBand with ComplexSource and nodata value yields potentially uninitialized buffer

I've attached a patch that resolves the issue by initializing the source buffers to zero or nodata inside VRTDerivedRasterBand::IRasterIO. Let me know if that's not the philosophically "right" thing to do, i.e. if it would be better if the VRTSources initialized their own buffers or something like that.

comment:3 Changed 7 years ago by Even Rouault

r22324 /trunk/gdal/frmts/vrt/vrtderivedrasterband.cpp: Fix 'VRTDerivedRasterBand with ComplexSource? and nodata value yields potentially uninitialized buffer' (#4045)

comment:4 Changed 7 years ago by Even Rouault

Milestone: 1.9.0

comment:5 Changed 7 years ago by antonio

Cc: antonio added

comment:6 Changed 6 years ago by Even Rouault

Milestone: 1.9.01.8.1
Resolution: fixed
Status: newclosed

r22441 /branches/1.8/gdal/frmts/vrt/vrtderivedrasterband.cpp: Fix 'VRTDerivedRasterBand with ComplexSource? and nodata value yields potentially uninitialized buffer' (#4045)

Note: See TracTickets for help on using tickets.