From 5b2f75388424995925a0e45654a0d509b290aaa0 Mon Sep 17 00:00:00 2001
From: Robert Loehning <robert.loehning@qt.io>
Date: Thu, 9 Jul 2020 13:33:34 +0200
Subject: [PATCH] Fix buffer overflow
Fixes: oss-fuzz-23988
Change-Id: I4efdbfc3c0a96917c0c8224642896088ade99f35
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit e80be8a43da78b9544f12fbac47e92c7f1f64366)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
src/gui/image/qxpmhandler.cpp | 2 +-
tests/auto/gui/image/qimagereader/images/oss-fuzz-23988.xpm | 1 +
tests/auto/gui/image/qimagereader/tst_qimagereader.cpp | 8 ++++++++
3 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 tests/auto/gui/image/qimagereader/images/oss-fuzz-23988.xpm
diff --git a/src/gui/image/qxpmhandler.cpp b/src/gui/image/qxpmhandler.cpp
index 17272ffe69b..417dab7ce3f 100644
--- a/src/gui/image/qxpmhandler.cpp
+++ b/src/gui/image/qxpmhandler.cpp
@@ -973,7 +973,7 @@ static bool read_xpm_body(
} else {
char b[16];
b[cpp] = '\0';
- for (x=0; x<w && d<end; x++) {
+ for (x=0; x<w && d+cpp<end; x++) {
memcpy(b, (char *)d, cpp);
*p++ = (uchar)colorMap[xpmHash(b)];
d += cpp;
diff --git a/tests/auto/gui/image/qimagereader/images/oss-fuzz-23988.xpm b/tests/auto/gui/image/qimagereader/images/oss-fuzz-23988.xpm
new file mode 100644
index 00000000000..7e6c1e4ca2e
--- /dev/null
+++ b/tests/auto/gui/image/qimagereader/images/oss-fuzz-23988.xpm
@@ -0,0 +1 @@
+/* XPM "20 8 1 7"" ÿÿ c ÿ" " ÿÿÿÿÿÿÿ "
\ No newline at end of file
diff --git a/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp b/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp
index 1eee2f273ef..0135e48c7df 100644
--- a/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp
+++ b/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp
@@ -167,6 +167,8 @@ private slots:
void devicePixelRatio_data();
void devicePixelRatio();
+ void xpmBufferOverflow();
+
private:
QString prefix;
QTemporaryDir m_temporaryDir;
@@ -2002,5 +2004,11 @@ void tst_QImageReader::devicePixelRatio()
QCOMPARE(img.devicePixelRatio(), dpr);
}
+void tst_QImageReader::xpmBufferOverflow()
+{
+ // Please note that the overflow only showed when Qt was configured with "-sanitize address".
+ QImageReader(":/images/oss-fuzz-23988.xpm").read();
+}
+
QTEST_MAIN(tst_QImageReader)
#include "tst_qimagereader.moc"
--
2.16.3
From 35ecd0b69d58bcc8113afc5e449aef841c73e26c Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@qt.io>
Date: Thu, 23 Jul 2020 11:48:48 +0200
Subject: [PATCH] Fix buffer overflow in XBM parser
Avoid parsing over the buffer limit, or interpreting non-hex
as hex.
This still leaves parsing of lines longer than 300 chars
unreliable
Change-Id: I1c57a7e530c4380f6f9040b2ec729ccd7dc7a5fb
Reviewed-by: Robert Loehning <robert.loehning@qt.io>
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>
(cherry picked from commit c562c1fc19629fb505acd0f6380604840b634211)
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
src/gui/image/qxbmhandler.cpp | 4 ++-
.../gui/image/qimagereader/tst_qimagereader.cpp | 37 ++++++++++++++++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/gui/image/qxbmhandler.cpp b/src/gui/image/qxbmhandler.cpp
index 7ba44049b48..8c4be4f0eda 100644
--- a/src/gui/image/qxbmhandler.cpp
+++ b/src/gui/image/qxbmhandler.cpp
@@ -158,7 +158,9 @@ static bool read_xbm_body(QIODevice *device, int w, int h, QImage *outImage)
w = (w+7)/8; // byte width
while (y < h) { // for all encoded bytes...
- if (p) { // p = "0x.."
+ if (p && p < (buf + readBytes - 3)) { // p = "0x.."
+ if (!isxdigit(p[2]) || !isxdigit(p[3]))
+ return false;
*b++ = hex2byte(p+2);
p += 2;
if (++x == w && ++y < h) {
diff --git a/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp b/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp
index 0135e48c7df..61b11a77794 100644
--- a/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp
+++ b/tests/auto/gui/image/qimagereader/tst_qimagereader.cpp
@@ -168,6 +168,7 @@ private slots:
void devicePixelRatio();
void xpmBufferOverflow();
+ void xbmBufferHandling();
private:
QString prefix;
@@ -2010,5 +2011,41 @@ void tst_QImageReader::xpmBufferOverflow()
QImageReader(":/images/oss-fuzz-23988.xpm").read();
}
+void tst_QImageReader::xbmBufferHandling()
+{
+ uint8_t original_buffer[256];
+ for (int i = 0; i < 256; ++i)
+ original_buffer[i] = i;
+
+ QImage image(original_buffer, 256, 8, QImage::Format_MonoLSB);
+ image.setColorTable({0xff000000, 0xffffffff});
+
+ QByteArray buffer;
+ {
+ QBuffer buf(&buffer);
+ QImageWriter writer(&buf, "xbm");
+ writer.write(image);
+ }
+
+ QCOMPARE(QImage::fromData(buffer, "xbm"), image);
+
+ auto i = buffer.indexOf(',');
+ buffer.insert(i + 1, " ");
+ QCOMPARE(QImage::fromData(buffer, "xbm"), image);
+ buffer.insert(i + 1, " ");
+ QCOMPARE(QImage::fromData(buffer, "xbm"), image);
+ buffer.insert(i + 1, " ");
+#if 0 // Lines longer than 300 chars not supported currently
+ QCOMPARE(QImage::fromData(buffer, "xbm"), image);
+#endif
+
+ i = buffer.lastIndexOf("\n ");
+ buffer.truncate(i + 1);
+ buffer.append(QByteArray(297, ' '));
+ buffer.append("0x");
+ // Only check we get no buffer overflow
+ QImage::fromData(buffer, "xbm");
+}
+
QTEST_MAIN(tst_QImageReader)
#include "tst_qimagereader.moc"
--
2.16.3
