[Server] QgsWmsRenderContext::isValidWidthHeight(): rewrite sanity checks in a foolproof way

The check for ``width * depth + 31`` not overflowing INT_MAX was done
after computing it, which would be undefined behavior if that happened.
Nowadays compilers can use "impossible situations" in smart ways, and
could very well discard the check if it is done afterwards. Be on the
safe sife and check before, as done in
https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/image/qimage_p.h#n89
This commit is contained in:
Even Rouault 2024-03-07 03:47:18 +01:00 committed by Nyall Dawson
parent 98017509f7
commit 5d97f57947
2 changed files with 48 additions and 6 deletions

View File

@ -650,6 +650,9 @@ bool QgsWmsRenderContext::isValidWidthHeight() const
bool QgsWmsRenderContext::isValidWidthHeight( int width, int height ) const
{
if ( width <= 0 || height <= 0 )
return false;
//test if maxWidth / maxHeight are set in the project or as an env variable
//and WIDTH / HEIGHT parameter is in the range allowed range
//WIDTH
@ -692,14 +695,15 @@ bool QgsWmsRenderContext::isValidWidthHeight( int width, int height ) const
return false;
}
// Sanity check from internal QImage checks (see qimage.cpp)
// Sanity check from internal QImage checks
// (see QImageData::calculateImageParameters() in qimage_p.h)
// this is to report a meaningful error message in case of
// image creation failure and to differentiate it from out
// of memory conditions.
// depth for now it cannot be anything other than 32, but I don't like
// to hardcode it: I hope we will support other depths in the future.
uint depth = 32;
int depth = 32;
switch ( mParameters.format() )
{
case QgsWmsParameters::Format::JPG:
@ -708,12 +712,12 @@ bool QgsWmsRenderContext::isValidWidthHeight( int width, int height ) const
depth = 32;
}
if ( width > ( std::numeric_limits<int>::max() - 31 ) / depth )
return false;
const int bytes_per_line = ( ( width * depth + 31 ) >> 5 ) << 2; // bytes per scanline (must be multiple of 4)
if ( std::numeric_limits<int>::max() / depth < static_cast<uint>( width )
|| bytes_per_line <= 0
|| height <= 0
|| std::numeric_limits<int>::max() / static_cast<uint>( bytes_per_line ) < static_cast<uint>( height )
if ( std::numeric_limits<int>::max() / bytes_per_line < height
|| std::numeric_limits<int>::max() / sizeof( uchar * ) < static_cast<uint>( height ) )
{
return false;

View File

@ -336,6 +336,25 @@ class TestQgsServerWMSGetMap(QgsServerTestBase):
err = b"HEIGHT (\'FOO\') cannot be converted into int" in r
self.assertTrue(err)
# height should be > 0
qs = "?" + "&".join(["%s=%s" % i for i in list({
"MAP": urllib.parse.quote(self.projectPath),
"SERVICE": "WMS",
"VERSION": "1.1.1",
"REQUEST": "GetMap",
"LAYERS": "Country",
"STYLES": "",
"FORMAT": "image/png",
"BBOX": "-16817707,-4710778,5696513,14587125",
"HEIGHT": "-1",
"WIDTH": "1",
"CRS": "EPSG:3857"
}.items())])
r, h = self._result(self._execute_request(qs))
err = b"The requested map size is too large" in r
self.assertTrue(err)
# width should be an int
qs = "?" + "&".join(["%s=%s" % i for i in list({
"MAP": urllib.parse.quote(self.projectPath),
@ -355,6 +374,25 @@ class TestQgsServerWMSGetMap(QgsServerTestBase):
err = b"WIDTH (\'FOO\') cannot be converted into int" in r
self.assertTrue(err)
# width should be > 0
qs = "?" + "&".join(["%s=%s" % i for i in list({
"MAP": urllib.parse.quote(self.projectPath),
"SERVICE": "WMS",
"VERSION": "1.1.1",
"REQUEST": "GetMap",
"LAYERS": "Country",
"STYLES": "",
"FORMAT": "image/png",
"BBOX": "-16817707,-4710778,5696513,14587125",
"HEIGHT": "1",
"WIDTH": "-1",
"CRS": "EPSG:3857"
}.items())])
r, h = self._result(self._execute_request(qs))
err = b"The requested map size is too large" in r
self.assertTrue(err)
# bbox should be formatted like "double,double,double,double"
qs = "?" + "&".join(["%s=%s" % i for i in list({
"MAP": urllib.parse.quote(self.projectPath),