Add some sanity and bounds checking

Since QImage does sanity checking for overflows and stuff wrt.
dimensions and depth, check for QImage::isNull() as early as possible to
see if there's some funky business going on.

Also tried to add some checks wherever we wrote to "raw" memory.

Unit tests pass, and tested converting some files from
https://samples.ffmpeg.org/image-samples/ to pngs, and that seemed to
work.

Reviewed By: aacid

Differential Revision: https://phabricator.kde.org/D24367
This commit is contained in:
Martin T. H. Sandsmark
2019-10-02 17:39:59 +02:00
parent f5b26cc9f9
commit 8562ce18f1
6 changed files with 106 additions and 12 deletions

View File

@ -178,6 +178,10 @@ static bool LoadTGA(QDataStream &s, const TgaHeader &tga, QImage &img)
{
// Create image.
img = QImage(tga.width, tga.height, QImage::Format_RGB32);
if (img.isNull()) {
qWarning() << "Failed to allocate image, invalid dimensions?" << QSize(tga.width, tga.height);
return false;
}
TgaHeaderInfo info(tga);
@ -186,6 +190,10 @@ static bool LoadTGA(QDataStream &s, const TgaHeader &tga, QImage &img)
// However alpha exists only in the 32 bit format.
if ((tga.pixel_size == 32) && (tga.flags & 0xf)) {
img = QImage(tga.width, tga.height, QImage::Format_ARGB32);
if (img.isNull()) {
qWarning() << "Failed to allocate image, invalid dimensions?" << QSize(tga.width, tga.height);
return false;
}
if (numAlphaBits > 8) {
return false;
@ -229,9 +237,10 @@ static bool LoadTGA(QDataStream &s, const TgaHeader &tga, QImage &img)
if (info.rle) {
// Decode image.
char *dst = (char *)image;
char *imgEnd = dst + size;
qint64 num = size;
while (num > 0) {
while (num > 0 && valid) {
if (s.atEnd()) {
valid = false;
break;
@ -257,6 +266,12 @@ static bool LoadTGA(QDataStream &s, const TgaHeader &tga, QImage &img)
memset(&pixel[dataRead], 0, pixel_size - dataRead);
}
do {
if (dst + pixel_size > imgEnd) {
qWarning() << "Trying to write out of bounds!" << ptrdiff_t(dst) << (ptrdiff_t(imgEnd) - ptrdiff_t(pixel_size));
valid = false;
break;
}
memcpy(dst, pixel, pixel_size);
dst += pixel_size;
} while (--count);
@ -268,8 +283,17 @@ static bool LoadTGA(QDataStream &s, const TgaHeader &tga, QImage &img)
free(image);
return false;
}
if ((uint)dataRead < count) {
memset(&dst[dataRead], 0, count - dataRead);
const size_t toCopy = count - dataRead;
if (&dst[dataRead] + toCopy > imgEnd) {
qWarning() << "Trying to write out of bounds!" << ptrdiff_t(image) << ptrdiff_t(&dst[dataRead]);;
valid = false;
break;
}
memset(&dst[dataRead], 0, toCopy);
}
dst += count;
}