1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
|
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
|