From cc1e76aa268ff60dfda20de7d7b1153e59512268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Wed, 29 Jul 2015 21:57:38 +0200 Subject: [PATCH 1/1] Harden NETWM data reading It's basically input data and cannot be assumed to be sane (a malicious or just stupid client could write anything there) BUG: 350173 REVIEW: 124354 FIXED-IN: 5.13 (cherry picked from commit a0698881fb0e5a4799d7320561acae84bcd6509f) --- src/netwm.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/netwm.cpp b/src/netwm.cpp index 9d5236aeec60db16b63f609c6000b3a724785877..d99a925ad2b99d5e60ab1dafcb01400b4a6a4c93 100644 --- a/src/netwm.cpp +++ b/src/netwm.cpp @@ -335,7 +335,7 @@ static QByteArray get_string_reply(xcb_connection_t *c, if (reply->type == type && reply->format == 8 && reply->value_len > 0) { const char *data = (const char *) xcb_get_property_value(reply); - int len = xcb_get_property_value_length(reply); + int len = reply->value_len; if (data) { value = QByteArray(data, data[len - 1] ? len : len - 1); @@ -551,10 +551,18 @@ static void readIcon(xcb_connection_t *c, const xcb_get_property_cookie_t cookie uint32_t *data = (uint32_t *) xcb_get_property_value(reply); - for (unsigned int i = 0, j = 0; j < reply->value_len; i++) { + for (unsigned int i = 0, j = 0; j < reply->value_len - 2; i++) { uint32_t width = data[j++]; uint32_t height = data[j++]; uint32_t size = width * height * sizeof(uint32_t); + if (j + width * height > reply->value_len) { + fprintf(stderr, "Ill-encoded icon data; proposed size leads to out of bounds access. Skipping. (%d x %d)\n", width, height); + break; + } + if (width > 1024 || height > 1024) { + fprintf(stderr, "Warning: found huge icon. The icon data may be ill-encoded. (%d x %d)\n", width, height); + // do not break nor continue - the data may likely be junk, but causes no harm (yet) and might actually be just a huge icon, eg. when the icon system is abused to transfer wallpapers or such. + } icons[i].size.width = width; icons[i].size.height = height; @@ -4708,7 +4716,7 @@ void NETWinInfo::update(NET::Properties dirtyProperties, NET::Properties2 dirtyP const QVector values = get_array_reply(p->conn, cookies[c++], XCB_ATOM_CARDINAL); p->opaqueRegion.clear(); p->opaqueRegion.reserve(values.count() / 4); - for (int i = 0; i < values.count(); i += 4) { + for (int i = 0; i < values.count() - 3; i += 4) { NETRect rect; rect.pos.x = values.at(i); rect.pos.y = values.at(i + 1); -- 2.4.6