Skip to content

Commit da5a171

Browse files
committed
[gdal] Fix an int overflow issue with raster block read
Fixes #19760 - Raster calculator crashes QGIS 3 .. and probably more, but this is one of the greediest piece of code because it reads the whole raster to memory.
1 parent 4d5edb2 commit da5a171

File tree

1 file changed

+27
-26
lines changed

1 file changed

+27
-26
lines changed

src/providers/gdal/qgsgdalprovider.cpp

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ QgsRasterBlock *QgsGdalProvider::block( int bandNo, const QgsRectangle &extent,
659659
return block;
660660
}
661661

662-
void QgsGdalProvider::readBlock( int bandNo, int xBlock, int yBlock, void *block )
662+
void QgsGdalProvider::readBlock( int bandNo, int xBlock, int yBlock, void *data )
663663
{
664664
QMutexLocker locker( mpMutex );
665665
if ( !initIfNeeded() )
@@ -677,25 +677,25 @@ void QgsGdalProvider::readBlock( int bandNo, int xBlock, int yBlock, void *block
677677
// We have to read with correct data type consistent with other readBlock functions
678678
int xOff = xBlock * mXBlockSize;
679679
int yOff = yBlock * mYBlockSize;
680-
gdalRasterIO( myGdalBand, GF_Read, xOff, yOff, mXBlockSize, mYBlockSize, block, mXBlockSize, mYBlockSize, ( GDALDataType ) mGdalDataType.at( bandNo - 1 ), 0, 0 );
680+
gdalRasterIO( myGdalBand, GF_Read, xOff, yOff, mXBlockSize, mYBlockSize, data, mXBlockSize, mYBlockSize, ( GDALDataType ) mGdalDataType.at( bandNo - 1 ), 0, 0 );
681681
}
682682

683-
void QgsGdalProvider::readBlock( int bandNo, QgsRectangle const &extent, int pixelWidth, int pixelHeight, void *block, QgsRasterBlockFeedback *feedback )
683+
void QgsGdalProvider::readBlock( int bandNo, QgsRectangle const &extent, int pixelWidth, int pixelHeight, void *data, QgsRasterBlockFeedback *feedback )
684684
{
685685
QMutexLocker locker( mpMutex );
686686
if ( !initIfNeeded() )
687687
return;
688688

689-
QgsDebugMsgLevel( "thePixelWidth = " + QString::number( pixelWidth ), 5 );
690-
QgsDebugMsgLevel( "thePixelHeight = " + QString::number( pixelHeight ), 5 );
691-
QgsDebugMsgLevel( "theExtent: " + extent.toString(), 5 );
689+
QgsDebugMsgLevel( "pixelWidth = " + QString::number( pixelWidth ), 5 );
690+
QgsDebugMsgLevel( "pixelHeight = " + QString::number( pixelHeight ), 5 );
691+
QgsDebugMsgLevel( "extent: " + extent.toString(), 5 );
692692

693693
for ( int i = 0; i < 6; i++ )
694694
{
695695
QgsDebugMsgLevel( QStringLiteral( "transform : %1" ).arg( mGeoTransform[i] ), 5 );
696696
}
697697

698-
int dataSize = dataTypeSize( bandNo );
698+
size_t dataSize = static_cast<size_t>( dataTypeSize( bandNo ) );
699699

700700
// moved to block()
701701
#if 0
@@ -713,14 +713,14 @@ void QgsGdalProvider::readBlock( int bandNo, QgsRectangle const &extent, int pi
713713
}
714714
#endif
715715

716-
QgsRectangle myRasterExtent = extent.intersect( mExtent );
717-
if ( myRasterExtent.isEmpty() )
716+
QgsRectangle rasterExtent = extent.intersect( mExtent );
717+
if ( rasterExtent.isEmpty() )
718718
{
719719
QgsDebugMsg( QStringLiteral( "draw request outside view extent." ) );
720720
return;
721721
}
722-
QgsDebugMsgLevel( "mExtent: " + mExtent.toString(), 5 );
723-
QgsDebugMsgLevel( "myRasterExtent: " + myRasterExtent.toString(), 5 );
722+
QgsDebugMsgLevel( "extent: " + mExtent.toString(), 5 );
723+
QgsDebugMsgLevel( "rasterExtent: " + rasterExtent.toString(), 5 );
724724

725725
double xRes = extent.width() / pixelWidth;
726726
double yRes = extent.height() / pixelHeight;
@@ -751,7 +751,7 @@ void QgsGdalProvider::readBlock( int bandNo, QgsRectangle const &extent, int pi
751751
right = std::round( ( myRasterExtent.xMaximum() - extent.xMinimum() ) / xRes ) - 1;
752752
}
753753
#endif
754-
QRect subRect = QgsRasterBlock::subRect( extent, pixelWidth, pixelHeight, myRasterExtent );
754+
QRect subRect = QgsRasterBlock::subRect( extent, pixelWidth, pixelHeight, rasterExtent );
755755
int top = subRect.top();
756756
int bottom = subRect.bottom();
757757
int left = subRect.left();
@@ -804,23 +804,23 @@ void QgsGdalProvider::readBlock( int bandNo, QgsRectangle const &extent, int pi
804804
// another resampling here which appeares to be quite fast
805805

806806
// Get necessary src extent aligned to src resolution
807-
if ( mExtent.xMinimum() < myRasterExtent.xMinimum() )
807+
if ( mExtent.xMinimum() < rasterExtent.xMinimum() )
808808
{
809-
srcLeft = static_cast<int>( std::floor( ( myRasterExtent.xMinimum() - mExtent.xMinimum() ) / srcXRes ) );
809+
srcLeft = static_cast<int>( std::floor( ( rasterExtent.xMinimum() - mExtent.xMinimum() ) / srcXRes ) );
810810
}
811-
if ( mExtent.xMaximum() > myRasterExtent.xMaximum() )
811+
if ( mExtent.xMaximum() > rasterExtent.xMaximum() )
812812
{
813-
srcRight = static_cast<int>( std::floor( ( myRasterExtent.xMaximum() - mExtent.xMinimum() ) / srcXRes ) );
813+
srcRight = static_cast<int>( std::floor( ( rasterExtent.xMaximum() - mExtent.xMinimum() ) / srcXRes ) );
814814
}
815815

816816
// GDAL states that mGeoTransform[3] is top, may it also be bottom and mGeoTransform[5] positive?
817-
if ( mExtent.yMaximum() > myRasterExtent.yMaximum() )
817+
if ( mExtent.yMaximum() > rasterExtent.yMaximum() )
818818
{
819-
srcTop = static_cast<int>( std::floor( -1. * ( mExtent.yMaximum() - myRasterExtent.yMaximum() ) / srcYRes ) );
819+
srcTop = static_cast<int>( std::floor( -1. * ( mExtent.yMaximum() - rasterExtent.yMaximum() ) / srcYRes ) );
820820
}
821-
if ( mExtent.yMinimum() < myRasterExtent.yMinimum() )
821+
if ( mExtent.yMinimum() < rasterExtent.yMinimum() )
822822
{
823-
srcBottom = static_cast<int>( std::floor( -1. * ( mExtent.yMaximum() - myRasterExtent.yMinimum() ) / srcYRes ) );
823+
srcBottom = static_cast<int>( std::floor( -1. * ( mExtent.yMaximum() - rasterExtent.yMinimum() ) / srcYRes ) );
824824
}
825825

826826
QgsDebugMsgLevel( QStringLiteral( "srcTop = %1 srcBottom = %2 srcLeft = %3 srcRight = %4" ).arg( srcTop ).arg( srcBottom ).arg( srcLeft ).arg( srcRight ), 5 );
@@ -847,19 +847,20 @@ void QgsGdalProvider::readBlock( int bandNo, QgsRectangle const &extent, int pi
847847
QgsDebugMsgLevel( QStringLiteral( "tmpXMin = %1 tmpYMax = %2 tmpWidth = %3 tmpHeight = %4" ).arg( tmpXMin ).arg( tmpYMax ).arg( tmpWidth ).arg( tmpHeight ), 5 );
848848

849849
// Allocate temporary block
850-
char *tmpBlock = ( char * )qgsMalloc( dataSize * tmpWidth * tmpHeight );
850+
size_t bufferSize = dataSize * static_cast<size_t>( tmpWidth ) * static_cast<size_t>( tmpHeight );
851+
char *tmpBlock = static_cast<char *>( qgsMalloc( bufferSize ) );
851852
if ( ! tmpBlock )
852853
{
853854
QgsDebugMsgLevel( QStringLiteral( "Couldn't allocate temporary buffer of %1 bytes" ).arg( dataSize * tmpWidth * tmpHeight ), 5 );
854855
return;
855856
}
856857
GDALRasterBandH gdalBand = getBand( bandNo );
857-
GDALDataType type = ( GDALDataType )mGdalDataType.at( bandNo - 1 );
858+
GDALDataType type = static_cast<GDALDataType>( mGdalDataType.at( bandNo - 1 ) );
858859
CPLErrorReset();
859860

860861
CPLErr err = gdalRasterIO( gdalBand, GF_Read,
861862
srcLeft, srcTop, srcWidth, srcHeight,
862-
( void * )tmpBlock,
863+
static_cast<void *>( tmpBlock ),
863864
tmpWidth, tmpHeight, type,
864865
0, 0, feedback );
865866

@@ -873,15 +874,15 @@ void QgsGdalProvider::readBlock( int bandNo, QgsRectangle const &extent, int pi
873874
double tmpXRes = srcWidth * srcXRes / tmpWidth;
874875
double tmpYRes = srcHeight * srcYRes / tmpHeight; // negative
875876

876-
double y = myRasterExtent.yMaximum() - 0.5 * yRes;
877+
double y = rasterExtent.yMaximum() - 0.5 * yRes;
877878
for ( int row = 0; row < height; row++ )
878879
{
879880
int tmpRow = static_cast<int>( std::floor( -1. * ( tmpYMax - y ) / tmpYRes ) );
880881

881882
char *srcRowBlock = tmpBlock + dataSize * tmpRow * tmpWidth;
882-
char *dstRowBlock = ( char * )block + dataSize * ( top + row ) * pixelWidth;
883+
char *dstRowBlock = ( char * )data + dataSize * ( top + row ) * pixelWidth;
883884

884-
double x = ( myRasterExtent.xMinimum() + 0.5 * xRes - tmpXMin ) / tmpXRes; // cell center
885+
double x = ( rasterExtent.xMinimum() + 0.5 * xRes - tmpXMin ) / tmpXRes; // cell center
885886
double increment = xRes / tmpXRes;
886887

887888
char *dst = dstRowBlock + dataSize * left;

0 commit comments

Comments
 (0)