API tools faq
paste
david_david

mumble-1.2.3-mga-CVE-2014-3755.patch

david_david
May 18th, 2014
284
0
Never
Add comment
Not a member of Pastebin yet? Sign Up, it unlocks many cool features!
text 17.51 KB | None | 0 0
raw download clone embed print report
  1. commit c7aecb2956f214cd83b7a862a4fcf15cc76c4450
  2. Author: Mikkel Krautz <[email protected]>
  3. Date: Tue May 13 20:23:05 2014 +0200
  4.  
  5. mumble: fix Mumble-SA-2014-005.
  6.  
  7. Qt's SVG image plugin is not safe to use with potentially
  8. unsafe SVGs, such as those downloaded over the network.
  9.  
  10. More specifically, it is possible to trigger local file
  11. access via Qt's SVG renderer using SVG features such as XML
  12. stylesheets (XSL) and SVG image tags, and potentially other
  13. features. This allows an attacker to have Qt read arbitrary
  14. files into the memory space of the Mumble process.
  15.  
  16. This makes it easy to perform a Denial of Service attack
  17. against a Mumble client. A client DoS can be accomplished
  18. by serving it an SVG file that refers to a filesystem path
  19. that is un-ending or is known to block under certain
  20. circumstances.
  21.  
  22. Having arbitrary files read into the Mumble process could
  23. potentially also be abused by an attacker to gain access
  24. to the content of the files, if combined with an (as of
  25. yet unknown) vulnerability that allows the attacker to
  26. read Mumble's memory.
  27.  
  28. To fix the issue, this change removes SVG as a supported
  29. image format for all externally received images. This
  30. includes things such as text messages, user comments,
  31. channel comments and user textures/avatars. It also removes
  32. the ability to transmit SVGs using any of the aforementioned
  33. channels.
  34.  
  35. This is accomplished by introducing a new class called
  36. RichTextImage. The RichTextImage class is used, via its
  37. isValidImage() method, to validate images before they are used
  38. in a rich text context in Mumble. In its current form, the
  39. isValidImage() method simply checks whether the image's format
  40. is part of the set of image formats that are deemed safe
  41. (PNG, JPG, GIF).
  42.  
  43. The LogDocument class, which is the QTextDocument that backs
  44. the Mumble log view, undergoes the following changes:
  45.  
  46. - LogDocument now restricts images loaded via QNetworkRequest
  47. and QNetworkReply to those that pass the
  48. RichTextImage::isValidImage() check.
  49. - Resources that use the data:// scheme are now loaded via
  50. QNetworkRequest and QNetworkReply, just like http:// and
  51. https:// URLs. This allows all resources to make use of
  52. LogDocument's new image format restrictions.
  53. - The functionality of the ValidDocument class, a subclass
  54. of LogDocument that was used to validate HTML snippets,
  55. is now part of LogDocument itself. The original
  56. ValidDocument class has been removed.
  57.  
  58. The RichTextEditor class is used to author text messages
  59. and user comments. The RichTextEditor is changed to use
  60. a LogDocument instead of a regular QTextDocument as its
  61. backing document. This allows the RichTextEditor to benefit
  62. from LogDocument's new image filtering functionality.
  63.  
  64. The static method Log::validHtml is used to validate
  65. HTML before using it in various contexts such as the Mumble
  66. log view and tooltips. This method is modified to use the
  67. LogDocument class instead of the ValidDocument class.
  68. A call to documentLayout() on the LogDocument is also
  69. added to the method. This ensures that image loading
  70. (and thus validation) is performed.
  71.  
  72. The MainWindow::openImageFile() method is re-worked to
  73. sniff and validate a selected image using
  74. RichTextImage::isValidImage to ensure that only valid
  75. rich text images can be selected, regardless of file
  76. extension.
  77.  
  78. The Overlay::verifyTexture() method is used to verify and
  79. set a ClientUser's texture and texture format, either by
  80. reading it from the local cache, or by reqesting a new
  81. texture from the server. This method is changed to only
  82. verify and set textures that pass the
  83. RichTextImage::isValidImage() check.
  84.  
  85. The ServerHandler::setUserTexture() method (also known as
  86. ServerHandler::setTexture() in some 1.2.x versions of Mumble)
  87. is changed to only allow settings textures that pass the
  88. RichTextImage::isValidImage() check.
  89.  
  90. Thanks to Tim Cooper for reporting this issue, proposing an
  91. initial patch and reviewing the final patch. This commit has
  92. also been reviewed and iterated upon by Stefan Hacker.
  93.  
  94. Rediffed by daviddavid (Mageia Team) for mumble-1.2.3.
  95.  
  96. --- mumble-1.2.3/src/mumble/Log.cpp.orig 2014-05-16 20:01:17.293295057 +0200
  97. +++ mumble-1.2.3/src/mumble/Log.cpp 2014-05-16 20:23:38.598915421 +0200
  98. @@ -33,6 +33,7 @@
  99. #include "TextToSpeech.h"
  100. #include "MainWindow.h"
  101. #include "Channel.h"
  102. +#include "RichTextEditor.h"
  103. #include "ServerHandler.h"
  104. #include "NetworkConfig.h"
  105. #include "Global.h"
  106. @@ -361,13 +362,23 @@
  107.  
  108. QString Log::validHtml(const QString &html, bool allowReplacement, QTextCursor *tc) {
  109. QDesktopWidget dw;
  110. - ValidDocument qtd(allowReplacement);
  111. + LogDocument qtd;
  112. bool valid = false;
  113.  
  114. + qtd.setAllowHTTPResources(allowReplacement);
  115. + qtd.setOnlyLoadDataURLs(true);
  116. +
  117. QRectF qr = dw.availableGeometry(dw.screenNumber(g.mw));
  118. qtd.setTextWidth(qr.width() / 2);
  119. qtd.setDefaultStyleSheet(qApp->styleSheet());
  120.  
  121. + // Call documentLayout on our LogDocument to ensure
  122. + // it has a layout backing it. With a layout set on
  123. + // the document, it will attempt to load all the
  124. + // resources it contains as soon as we call setHtml(),
  125. + // allowing our validation checks for things such as
  126. + // data URL images to run.
  127. + (void) qtd.documentLayout();
  128. qtd.setHtml(html);
  129. valid = qtd.isValid();
  130.  
  131. @@ -612,65 +623,86 @@
  132. tts->say(terse);
  133. }
  134.  
  135. -ValidDocument::ValidDocument(bool allowhttp, QObject *p) : QTextDocument(p) {
  136. - bValid = true;
  137. - qslValidImage << QLatin1String("data");
  138. - if (allowhttp) {
  139. - qslValidImage << QLatin1String("http");
  140. - qslValidImage << QLatin1String("https");
  141. - }
  142. -}
  143. -
  144. -QVariant ValidDocument::loadResource(int type, const QUrl &url) {
  145. - QVariant v = QLatin1String("PlaceHolder");
  146. - if ((type == QTextDocument::ImageResource) && qslValidImage.contains(url.scheme()))
  147. - return QTextDocument::loadResource(type, url);
  148. - bValid = false;
  149. - return v;
  150. -}
  151. -
  152. -bool ValidDocument::isValid() const {
  153. - return bValid;
  154. -}
  155. -
  156. -LogDocument::LogDocument(QObject *p) : QTextDocument(p) {
  157. +LogDocument::LogDocument(QObject *p)
  158. + : QTextDocument(p)
  159. + , m_valid(true)
  160. + , m_onlyLoadDataURLs(false)
  161. + , m_allowHTTPResources(true) {
  162. }
  163.  
  164. QVariant LogDocument::loadResource(int type, const QUrl &url) {
  165. - if (type != QTextDocument::ImageResource)
  166. + if (type != QTextDocument::ImageResource) {
  167. + m_valid = false;
  168. return QLatin1String("No external resources allowed.");
  169. - if (g.s.iMaxImageSize <= 0)
  170. - return QLatin1String("Image download disabled.");
  171.  
  172. - if (url.scheme() == QLatin1String("data")) {
  173. - QVariant v = QTextDocument::loadResource(type, url);
  174. - addResource(type, url, v);
  175. - return v;
  176. }
  177.  
  178. - qWarning() << "LogDocument::loadResource " << type << url.toString();
  179. + if (url.scheme() != QLatin1String("data") && g.s.iMaxImageSize <= 0) {
  180. + m_valid = false;
  181. + return QLatin1String("Image download disabled.");
  182. + }
  183.  
  184. QImage qi(1, 1, QImage::Format_Mono);
  185. addResource(type, url, qi);
  186.  
  187. - if (! url.isValid() || url.isRelative())
  188. + if (! url.isValid() || url.isRelative()) {
  189. + m_valid = false;
  190. return qi;
  191. + }
  192. +
  193. + QStringList allowedSchemes;
  194. + allowedSchemes << QLatin1String("data");
  195. + if (m_allowHTTPResources) {
  196. + allowedSchemes << QLatin1String("http");
  197. + allowedSchemes << QLatin1String("https");
  198. + }
  199.  
  200. - if ((url.scheme() != QLatin1String("http")) && (url.scheme() != QLatin1String("https")))
  201. + if (!allowedSchemes.contains(url.scheme())) {
  202. + m_valid = false;
  203. return qi;
  204. + }
  205. +
  206. + bool shouldLoad = true;
  207. + if (m_onlyLoadDataURLs && url.scheme() != QLatin1String("data")) {
  208. + shouldLoad = false;
  209. + }
  210. +
  211. + if (shouldLoad) {
  212. + QNetworkReply *rep = Network::get(url);
  213. + connect(rep, SIGNAL(metaDataChanged()), this, SLOT(receivedHead()));
  214. + connect(rep, SIGNAL(finished()), this, SLOT(finished()));
  215. +
  216. + // Handle data URLs immediately without a roundtrip to the event loop.
  217. + // We need this to perform proper validation for data URL images when
  218. + // a LogDocument is used inside Log::validHtml().
  219. + if (url.scheme() == QLatin1String("data")) {
  220. + QCoreApplication::sendPostedEvents(rep, 0);
  221. + }
  222. + }
  223.  
  224. - QNetworkReply *rep = Network::get(url);
  225. - connect(rep, SIGNAL(metaDataChanged()), this, SLOT(receivedHead()));
  226. - connect(rep, SIGNAL(finished()), this, SLOT(finished()));
  227. return qi;
  228. }
  229.  
  230. +void LogDocument::setAllowHTTPResources(bool allowHTTPResources) {
  231. + m_allowHTTPResources = allowHTTPResources;
  232. +}
  233. +
  234. +void LogDocument::setOnlyLoadDataURLs(bool onlyLoadDataURLs) {
  235. + m_onlyLoadDataURLs = onlyLoadDataURLs;
  236. +}
  237. +
  238. +bool LogDocument::isValid() {
  239. + return m_valid;
  240. +}
  241. +
  242. void LogDocument::receivedHead() {
  243. QNetworkReply *rep = qobject_cast<QNetworkReply *>(sender());
  244. - QVariant length = rep->header(QNetworkRequest::ContentLengthHeader);
  245. - if (length == QVariant::Invalid || length.toInt() > g.s.iMaxImageSize) {
  246. - qWarning() << "Image "<< rep->url().toString() <<" (" << length.toInt() << " byte) to big, request aborted. ";
  247. - rep->abort();
  248. + if (rep->url().scheme() != QLatin1String("data")) {
  249. + QVariant length = rep->header(QNetworkRequest::ContentLengthHeader);
  250. + if (length == QVariant::Invalid || length.toInt() > g.s.iMaxImageSize) {
  251. + m_valid = false;
  252. + rep->abort();
  253. + }
  254. }
  255. }
  256.  
  257. @@ -678,14 +710,42 @@
  258. QNetworkReply *rep = qobject_cast<QNetworkReply *>(sender());
  259.  
  260. if (rep->error() == QNetworkReply::NoError) {
  261. - QVariant qv = rep->readAll();
  262. + QByteArray ba = rep->readAll();
  263. + QByteArray fmt;
  264. QImage qi;
  265.  
  266. - if (qi.loadFromData(qv.toByteArray()) && qi.width() <= g.s.iMaxImageWidth && qi.height() <= g.s.iMaxImageHeight) {
  267. - addResource(QTextDocument::ImageResource, rep->request().url(), qi);
  268. - g.mw->qteLog->setDocument(this);
  269. - } else qWarning() << "Image "<< rep->url().toString() <<" (" << qi.width() << "x" << qi.height() <<") to large.";
  270. - } else qWarning() << "Image "<< rep->url().toString() << " download failed.";
  271. + // Sniff the format instead of relying on the MIME type.
  272. + // There are many misconfigured servers out there and
  273. + // Mumble has historically sniffed the received data
  274. + // instead of strictly requiring a correct Content-Type.
  275. + if (RichTextImage::isValidImage(ba, fmt)) {
  276. + if (qi.loadFromData(ba, fmt)) {
  277. + bool ok = true;
  278. + if (rep->url().scheme() != QLatin1String("data")) {
  279. + ok = (qi.width() <= g.s.iMaxImageWidth && qi.height() <= g.s.iMaxImageHeight);
  280. + }
  281. + if (ok) {
  282. + addResource(QTextDocument::ImageResource, rep->request().url(), qi);
  283. +
  284. + // Force a re-layout of the QTextEdit the next
  285. + // time we enter the event loop.
  286. + // We must not trigger a re-layout immediately.
  287. + // Doing so can trigger crashes deep inside Qt
  288. + // if the QTextDocument has just been set on the
  289. + // text edit widget.
  290. + QTextEdit *qte = qobject_cast<QTextEdit *>(parent());
  291. + if (qte != NULL) {
  292. + QEvent *e = new QEvent(QEvent::FontChange);
  293. + QApplication::postEvent(qte, e);
  294. + }
  295. + } else {
  296. + m_valid = false;
  297. + }
  298. + }
  299. + } else {
  300. + m_valid = false;
  301. + }
  302. + }
  303.  
  304. rep->deleteLater();
  305. }
  306. --- mumble-1.2.3/src/mumble/Log.h.orig 2014-05-16 20:25:03.483113876 +0200
  307. +++ mumble-1.2.3/src/mumble/Log.h 2014-05-16 20:27:03.924596720 +0200
  308. @@ -96,29 +96,23 @@
  309. void log(MsgType t, const QString &console, const QString &terse=QString(), bool console_only = false);
  310. };
  311.  
  312. -class ValidDocument : public QTextDocument {
  313. - private:
  314. - Q_OBJECT
  315. - Q_DISABLE_COPY(ValidDocument)
  316. - protected:
  317. - QStringList qslValidImage;
  318. - bool bValid;
  319. - QVariant loadResource(int, const QUrl &);
  320. - public:
  321. - ValidDocument(bool httpimages, QObject *p = NULL);
  322. - bool isValid() const;
  323. -};
  324. -
  325. class LogDocument : public QTextDocument {
  326. private:
  327. Q_OBJECT
  328. Q_DISABLE_COPY(LogDocument)
  329. public:
  330. LogDocument(QObject *p = NULL);
  331. - QVariant loadResource(int, const QUrl &);
  332. + virtual QVariant loadResource(int, const QUrl &);
  333. + void setAllowHTTPResources(bool allowHttpResources);
  334. + void setOnlyLoadDataURLs(bool onlyLoadDataURLs);
  335. + bool isValid();
  336. public slots:
  337. void receivedHead();
  338. void finished();
  339. + private:
  340. + bool m_allowHTTPResources;
  341. + bool m_valid;
  342. + bool m_onlyLoadDataURLs;
  343. };
  344.  
  345. #else
  346. --- mumble-1.2.3/src/mumble/MainWindow.cpp.orig 2014-05-16 20:28:57.462753125 +0200
  347. +++ mumble-1.2.3/src/mumble/MainWindow.cpp 2014-05-16 20:31:24.702030629 +0200
  348. @@ -41,6 +41,7 @@
  349. #include "UserEdit.h"
  350. #include "Tokens.h"
  351. #include "Connection.h"
  352. +#include "RichTextEditor.h"
  353. #include "ServerHandler.h"
  354. #include "About.h"
  355. #include "GlobalShortcut.h"
  356. @@ -2630,7 +2631,7 @@
  357. if (g.s.qsImagePath.isEmpty() || ! QDir::root().exists(g.s.qsImagePath))
  358. g.s.qsImagePath = QDesktopServices::storageLocation(QDesktopServices::PicturesLocation);
  359.  
  360. - QString fname = QFileDialog::getOpenFileName(this, tr("Choose image file"), g.s.qsImagePath, tr("Images (*.png *.jpg *.jpeg *.svg)"));
  361. + QString fname = QFileDialog::getOpenFileName(this, tr("Choose image file"), g.s.qsImagePath, tr("Images (*.png *.jpg *.jpeg)"));
  362.  
  363. if (fname.isNull())
  364. return retval;
  365. @@ -2650,7 +2651,17 @@
  366. QBuffer qb(&qba);
  367. qb.open(QIODevice::ReadOnly);
  368.  
  369. - QImageReader qir(&qb, fi.suffix().toUtf8());
  370. + QImageReader qir;
  371. + qir.setAutoDetectImageFormat(false);
  372. +
  373. + QByteArray fmt;
  374. + if (!RichTextImage::isValidImage(qba, fmt)) {
  375. + QMessageBox::warning(this, tr("Failed to load image"), tr("Image format not recognized."));
  376. + return retval;
  377. + }
  378. +
  379. + qir.setFormat(fmt);
  380. + qir.setDevice(&qb);
  381.  
  382. QImage img = qir.read();
  383. if (img.isNull()) {
  384. --- mumble-1.2.3/src/mumble/Overlay.cpp.orig 2014-05-16 20:33:30.234704214 +0200
  385. +++ mumble-1.2.3/src/mumble/Overlay.cpp 2014-05-16 20:35:21.846653431 +0200
  386. @@ -36,6 +36,7 @@
  387. #include "Message.h"
  388. #include "Database.h"
  389. #include "NetworkConfig.h"
  390. +#include "RichTextEditor.h"
  391. #include "ServerHandler.h"
  392. #include "MainWindow.h"
  393. #include "GlobalShortcut.h"
  394. @@ -268,15 +269,21 @@
  395. qb.open(QIODevice::ReadOnly);
  396.  
  397. QImageReader qir;
  398. - if (cp->qbaTexture.startsWith("<?xml"))
  399. - qir.setFormat("svg");
  400. - qir.setDevice(&qb);
  401. - if (! qir.canRead() || (qir.size().width() > 1024) || (qir.size().height() > 1024)) {
  402. - valid = false;
  403. + qir.setAutoDetectImageFormat(false);
  404. +
  405. + QByteArray fmt;
  406. + if (RichTextImage::isValidImage(cp->qbaTexture, fmt)) {
  407. + qir.setFormat(fmt);
  408. + qir.setDevice(&qb);
  409. + if (! qir.canRead() || (qir.size().width() > 1024) || (qir.size().height() > 1024)) {
  410. + valid = false;
  411. + } else {
  412. + cp->qbaTextureFormat = qir.format();
  413. + QImage qi = qir.read();
  414. + valid = ! qi.isNull();
  415. + }
  416. } else {
  417. - cp->qbaTextureFormat = qir.format();
  418. - QImage qi = qir.read();
  419. - valid = ! qi.isNull();
  420. + valid = false;
  421. }
  422. }
  423. if (! valid) {
  424. --- mumble-1.2.3/src/mumble/RichTextEditor.cpp.orig 2014-05-16 20:37:19.709520210 +0200
  425. +++ mumble-1.2.3/src/mumble/RichTextEditor.cpp 2014-05-16 20:40:44.161654041 +0200
  426. @@ -34,6 +34,8 @@
  427. #include "Log.h"
  428.  
  429. RichTextHtmlEdit::RichTextHtmlEdit(QWidget *p) : QTextEdit(p) {
  430. + m_document = new LogDocument(this);
  431. + setDocument(m_document);
  432. }
  433.  
  434. /* On nix, some programs send utf8, some send wchar_t. Some zeroterminate once, some twice, some not at all.
  435. @@ -620,3 +622,20 @@
  436. bChanged = false;
  437. return qptePlainText->toPlainText();
  438. }
  439. +
  440. +bool RichTextImage::isValidImage(const QByteArray &ba, QByteArray &fmt) {
  441. + QBuffer qb;
  442. + qb.setData(ba);
  443. + if (!qb.open(QIODevice::ReadOnly)) {
  444. + return false;
  445. + }
  446. +
  447. + QByteArray detectedFormat = QImageReader::imageFormat(&qb).toLower();
  448. + if (detectedFormat == QByteArray("png") || detectedFormat == QByteArray("jpg")
  449. + || detectedFormat == QByteArray("jpeg") || detectedFormat == QByteArray("gif")) {
  450. + fmt = detectedFormat;
  451. + return true;
  452. + }
  453. +
  454. + return false;
  455. +}
  456. --- mumble-1.2.3/src/mumble/RichTextEditor.h.orig 2014-05-16 20:42:06.892502952 +0200
  457. +++ mumble-1.2.3/src/mumble/RichTextEditor.h 2014-05-16 20:44:57.069155722 +0200
  458. @@ -33,6 +33,8 @@
  459.  
  460. #include "mumble_pch.hpp"
  461.  
  462. +class LogDocument;
  463. +
  464. class RichTextHtmlEdit : public QTextEdit {
  465. private:
  466. Q_OBJECT
  467. @@ -41,6 +43,8 @@
  468. void insertFromMimeData(const QMimeData *source);
  469. public:
  470. RichTextHtmlEdit(QWidget *p);
  471. + private:
  472. + LogDocument *m_document;
  473. };
  474.  
  475. #include "ui_RichTextEditor.h"
  476. @@ -89,4 +93,9 @@
  477. void onCurrentChanged(int);
  478. };
  479.  
  480. +class RichTextImage {
  481. + public:
  482. + static bool isValidImage(const QByteArray &buf, QByteArray &fmt);
  483. +};
  484. +
  485. #endif
  486. --- mumble-1.2.3/src/mumble/ServerHandler.cpp.orig 2014-05-16 20:46:30.060435030 +0200
  487. +++ mumble-1.2.3/src/mumble/ServerHandler.cpp 2014-05-16 20:48:03.723135129 +0200
  488. @@ -41,6 +41,7 @@
  489. #include "PacketDataStream.h"
  490. #include "NetworkConfig.h"
  491. #include "OSInfo.h"
  492. +#include "RichTextEditor.h"
  493. #include "SSL.h"
  494.  
  495. ServerHandlerMessageEvent::ServerHandlerMessageEvent(const QByteArray &msg, unsigned int mtype, bool flush) : QEvent(static_cast<QEvent::Type>(SERVERSEND_EVENT)) {
  496. @@ -603,8 +604,14 @@
  497. QBuffer qb(& raw);
  498. qb.open(QIODevice::ReadOnly);
  499. QImageReader qir;
  500. - if (qba.startsWith("<?xml"))
  501. - qir.setFormat("svg");
  502. + qir.setDecideFormatFromContent(false);
  503. +
  504. + QByteArray fmt;
  505. + if (!RichTextImage::isValidImage(qba, fmt)) {
  506. + return;
  507. + }
  508. +
  509. + qir.setFormat(fmt);
  510. qir.setDevice(&qb);
  511.  
  512. QSize sz = qir.size();
Add Comment
Please, Sign In to add comment
Public Pastes
We use cookies for various purposes including analytics. By continuing to use Pastebin, you agree to our use of cookies as described in the Cookies Policy.  OK, I Understand
Not a member of Pastebin yet?
Sign Up, it unlocks many cool features!