168 lines
6.9 KiB
Diff
Executable File
168 lines
6.9 KiB
Diff
Executable File
From a55f36211efe1bb0d6717c8545366120bd6dfd9f Mon Sep 17 00:00:00 2001
|
|
From: Thiago Macieira <thiago.macieira@intel.com>
|
|
Date: Mon, 21 Nov 2016 15:17:03 -0800
|
|
Subject: Fix the JPEG EXIF reader to deal with some broken/corrupt files
|
|
|
|
We parse the EXIF header in order to get the proper orientation, so
|
|
let's be a bit more careful in what we accept. This patch adds better
|
|
handling for reading past the end of the stream, plus it limits the
|
|
number of IFDs read (to avoid processing too much data) and deals with a
|
|
pathological case of the EXIF file format: EXIF (due to its TIFF
|
|
origins) permits the offset to the next IFD to be backwards in the file,
|
|
which means it could result in a loop or pointing to plain corrupt data.
|
|
We disallow any backwards pointers, since it seems that's what other
|
|
decoders do (libexif, for example).
|
|
|
|
Change-Id: Iaeecaffe26af4535b416fffd1489332db92e3888
|
|
(cherry picked from 5.6 commit 02150649f95b8f46f826e6e002be3fa0b6d009bc)
|
|
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
|
|
---
|
|
src/gui/image/qjpeghandler.cpp | 31 ++++++++++++++-------
|
|
.../jpeg_exif_invalid_data_back_pointers.jpg | Bin 0 -> 910 bytes
|
|
.../images/jpeg_exif_invalid_data_past_end.jpg | Bin 0 -> 910 bytes
|
|
.../jpeg_exif_invalid_data_too_many_ifds.jpg | Bin 0 -> 964 bytes
|
|
.../jpeg_exif_invalid_data_too_many_tags.jpg | Bin 0 -> 910 bytes
|
|
tests/auto/gui/image/qimage/tst_qimage.cpp | 17 +++++++++--
|
|
6 files changed, 35 insertions(+), 13 deletions(-)
|
|
create mode 100644 tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_back_pointers.jpg
|
|
create mode 100644 tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_past_end.jpg
|
|
create mode 100644 tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_too_many_ifds.jpg
|
|
create mode 100644 tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_too_many_tags.jpg
|
|
|
|
diff --git a/src/gui/image/qjpeghandler.cpp b/src/gui/image/qjpeghandler.cpp
|
|
index 52e8b39..f3ba303 100644
|
|
--- a/src/gui/image/qjpeghandler.cpp
|
|
+++ b/src/gui/image/qjpeghandler.cpp
|
|
@@ -786,6 +786,10 @@ static bool readExifHeader(QDataStream &stream)
|
|
*/
|
|
static int getExifOrientation(QByteArray &exifData)
|
|
{
|
|
+ // Current EXIF version (2.3) says there can be at most 5 IFDs,
|
|
+ // byte we allow for 10 so we're able to deal with future extensions.
|
|
+ const int maxIfdCount = 10;
|
|
+
|
|
QDataStream stream(&exifData, QIODevice::ReadOnly);
|
|
|
|
if (!readExifHeader(stream))
|
|
@@ -793,7 +797,8 @@ static int getExifOrientation(QByteArray &exifData)
|
|
|
|
quint16 val;
|
|
quint32 offset;
|
|
- const qint64 headerStart = stream.device()->pos();
|
|
+ const qint64 headerStart = 6; // the EXIF header has a constant size
|
|
+ Q_ASSERT(headerStart == stream.device()->pos());
|
|
|
|
// read byte order marker
|
|
stream >> val;
|
|
@@ -804,7 +809,7 @@ static int getExifOrientation(QByteArray &exifData)
|
|
else
|
|
return -1; // unknown byte order
|
|
|
|
- // read size
|
|
+ // confirm byte order
|
|
stream >> val;
|
|
if (val != 0x2a)
|
|
return -1;
|
|
@@ -812,18 +817,22 @@ static int getExifOrientation(QByteArray &exifData)
|
|
stream >> offset;
|
|
|
|
// read IFD
|
|
- while (!stream.atEnd()) {
|
|
+ for (int n = 0; n < maxIfdCount; ++n) {
|
|
quint16 numEntries;
|
|
|
|
- // skip offset bytes to get the next IFD
|
|
const qint64 bytesToSkip = offset - (stream.device()->pos() - headerStart);
|
|
-
|
|
- if (stream.skipRawData(bytesToSkip) != bytesToSkip)
|
|
+ if (bytesToSkip < 0 || (offset + headerStart >= exifData.size())) {
|
|
+ // disallow going backwards, though it's permitted in the spec
|
|
return -1;
|
|
+ } else if (bytesToSkip != 0) {
|
|
+ // seek to the IFD
|
|
+ if (!stream.device()->seek(offset + headerStart))
|
|
+ return -1;
|
|
+ }
|
|
|
|
stream >> numEntries;
|
|
|
|
- for (; numEntries > 0; --numEntries) {
|
|
+ for (; numEntries > 0 && stream.status() == QDataStream::Ok; --numEntries) {
|
|
quint16 tag;
|
|
quint16 type;
|
|
quint32 components;
|
|
@@ -847,12 +856,14 @@ static int getExifOrientation(QByteArray &exifData)
|
|
|
|
// read offset to next IFD
|
|
stream >> offset;
|
|
+ if (stream.status() != QDataStream::Ok)
|
|
+ return -1;
|
|
if (offset == 0) // this is the last IFD
|
|
- break;
|
|
+ return 0; // No Exif orientation was found
|
|
}
|
|
|
|
- // No Exif orientation was found
|
|
- return 0;
|
|
+ // too many IFDs
|
|
+ return -1;
|
|
}
|
|
|
|
static QImageIOHandler::Transformations exif2Qt(int exifOrientation)
|
|
diff --git a/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_back_pointers.jpg b/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_back_pointers.jpg
|
|
new file mode 100644
|
|
index 0000000..164d308
|
|
Binary files /dev/null and b/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_back_pointers.jpg differ
|
|
diff --git a/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_past_end.jpg b/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_past_end.jpg
|
|
new file mode 100644
|
|
index 0000000..7e2451e
|
|
Binary files /dev/null and b/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_past_end.jpg differ
|
|
diff --git a/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_too_many_ifds.jpg b/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_too_many_ifds.jpg
|
|
new file mode 100644
|
|
index 0000000..52c6a93
|
|
Binary files /dev/null and b/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_too_many_ifds.jpg differ
|
|
diff --git a/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_too_many_tags.jpg b/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_too_many_tags.jpg
|
|
new file mode 100644
|
|
index 0000000..6a080aa
|
|
Binary files /dev/null and b/tests/auto/gui/image/qimage/images/jpeg_exif_invalid_data_too_many_tags.jpg differ
|
|
diff --git a/tests/auto/gui/image/qimage/tst_qimage.cpp b/tests/auto/gui/image/qimage/tst_qimage.cpp
|
|
index 85d258d..26772d5 100644
|
|
--- a/tests/auto/gui/image/qimage/tst_qimage.cpp
|
|
+++ b/tests/auto/gui/image/qimage/tst_qimage.cpp
|
|
@@ -183,7 +183,8 @@ private slots:
|
|
void exifOrientation();
|
|
|
|
void exif_QTBUG45865();
|
|
- void exif_invalid_data_QTBUG46870();
|
|
+ void exifInvalidData_data();
|
|
+ void exifInvalidData();
|
|
|
|
void cleanupFunctions();
|
|
|
|
@@ -3028,10 +3029,20 @@ void tst_QImage::exif_QTBUG45865()
|
|
QCOMPARE(image.size(), QSize(5, 8));
|
|
}
|
|
|
|
-void tst_QImage::exif_invalid_data_QTBUG46870()
|
|
+void tst_QImage::exifInvalidData_data()
|
|
+{
|
|
+ QTest::addColumn<bool>("$never used");
|
|
+ QTest::newRow("QTBUG-46870");
|
|
+ QTest::newRow("back_pointers");
|
|
+ QTest::newRow("past_end");
|
|
+ QTest::newRow("too_many_ifds");
|
|
+ QTest::newRow("too_many_tags");
|
|
+}
|
|
+
|
|
+void tst_QImage::exifInvalidData()
|
|
{
|
|
QImage image;
|
|
- QVERIFY(image.load(m_prefix + "jpeg_exif_invalid_data_QTBUG-46870.jpg"));
|
|
+ QVERIFY(image.load(m_prefix + "jpeg_exif_invalid_data_" + QTest::currentDataTag() + ".jpg"));
|
|
QVERIFY(!image.isNull());
|
|
}
|
|
|
|
--
|
|
cgit v1.0-4-g1e03
|
|
|