From 2702008072fedaff402d752cf38cef7d1336f721 Mon Sep 17 00:00:00 2001 From: Nathan Date: Tue, 27 Oct 2020 21:45:33 +0000 Subject: user/trojita: CVE patches (#137) --- user/trojita/APKBUILD | 19 +++++-- user/trojita/CVE-2019-10734.patch | 104 ++++++++++++++++++++++++++++++++++++++ user/trojita/CVE-2020-15047.patch | 88 ++++++++++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 user/trojita/CVE-2019-10734.patch create mode 100644 user/trojita/CVE-2020-15047.patch diff --git a/user/trojita/APKBUILD b/user/trojita/APKBUILD index c55e5453c..3dd6a5ab6 100644 --- a/user/trojita/APKBUILD +++ b/user/trojita/APKBUILD @@ -2,7 +2,7 @@ # Maintainer: Kiyoshi Aman pkgname=trojita pkgver=0.7 -pkgrel=0 +pkgrel=1 pkgdesc="Qt-based IMAP email client" url="http://trojita.flaska.net/" arch="all" @@ -15,7 +15,15 @@ makedepends="cmake extra-cmake-modules zlib-dev qt5-qtbase-dev qt5-qtwebkit-dev qtkeychain-dev" source="https://sourceforge.net/projects/trojita/files/src/trojita-$pkgver.tar.xz use-qgpgme.patch - fix-gpg.patch" + fix-gpg.patch + CVE-2019-10734.patch + CVE-2020-15047.patch + " + +# secfixes: +# 0.7-r1: +# - CVE-2019-10734 +# - CVE-2020-15047 build() { if [ "$CBUILD" != "$CHOST" ]; then @@ -34,7 +42,8 @@ build() { } check() { - CTEST_OUTPUT_ON_FAILURE=TRUE ctest + # test_Html_formatting: requires X11 + CTEST_OUTPUT_ON_FAILURE=TRUE ctest -E test_Html_formatting } package() { @@ -43,4 +52,6 @@ package() { sha512sums="fe4d9316f97d913619f27d24a5023c3d8dd4a6b9fb058651be12c67188f394aa8cbb60c7593e5eb28fc12fc883b76deeeb5f4f631edd255fdec4c5862c9a91c8 trojita-0.7.tar.xz 740c2410d7236d722482f05dd1d2c681e35543620823cb7c1396710081f9de4f6ae530c5b5442ecf5d08a8e552f0697f3a35bf51e07a3b4336dec7021b665706 use-qgpgme.patch -9d0fbf7c0b0f6975990a7705f9d43043e5807401cee179d7a07f9514856332d6bb1aa8448e84d0083003c34a3bb181080b973e8c1f77d1b5a8930d07d57702da fix-gpg.patch" +9d0fbf7c0b0f6975990a7705f9d43043e5807401cee179d7a07f9514856332d6bb1aa8448e84d0083003c34a3bb181080b973e8c1f77d1b5a8930d07d57702da fix-gpg.patch +db96a566924b5d7b80787ab624af3726d5dd3459653192436a377d6482ab73801a7dcca1df1b1d937cf0d0798b827e04f8ef2c1124f91dc9da3e8036ef61e28a CVE-2019-10734.patch +2477612aca1e558fa3ba2b434a701cc255c573ac7e2001e7b5921c9b991f7c95720f53b70b49824e36bafb53ab53477950cb8d436e637fda4d59c7ec5883ce5f CVE-2020-15047.patch" diff --git a/user/trojita/CVE-2019-10734.patch b/user/trojita/CVE-2019-10734.patch new file mode 100644 index 000000000..d52edb042 --- /dev/null +++ b/user/trojita/CVE-2019-10734.patch @@ -0,0 +1,104 @@ +From 8db7f450d52539b4c72ee968384911b6813ad1e7 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= +Date: Thu, 25 Jun 2020 21:39:34 +0200 +Subject: [PATCH] Prevent a possible decryption oracle attack +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Thanks to Jens Mueller (Ruhr-Uni Bochum and FH Münster) for reporting +this. The gist is that an attacker can embed arbitrary ciphertext into +their messages. Trojita decrypts that, and when we hit reply, the +original *cleartext* gets quoted and put into a reply for the attacker +to see. + +Fix this by not quoting any plaintext which originated in an encrypted +message. That's pretty draconian, but hey, it works and we never came up +with any better patch. Also, given that Trojita does not encrypt +outgoing messages yet, this is probably also a conservative thing to do. + +Change-Id: I84c45b9e707eb7c99eb7183c6ef59ef41cd62c43 +CVE: CVE-2019-10734 +BUG: 404697 +--- + src/Cryptography/GpgMe++.cpp | 2 ++ + src/Gui/MessageView.cpp | 9 ++++++++- + src/Gui/PartWidget.cpp | 8 ++++++++ + src/Imap/Model/ItemRoles.h | 2 +- + 4 files changed, 19 insertions(+), 2 deletions(-) + +diff --git a/src/Cryptography/GpgMe++.cpp b/src/Cryptography/GpgMe++.cpp +index e012f603..716b8aff 100644 +--- a/src/Cryptography/GpgMe++.cpp ++++ b/src/Cryptography/GpgMe++.cpp +@@ -267,6 +267,8 @@ QVariant GpgMePart::data(int role) const + switch (role) { + case Imap::Mailbox::RolePartSignatureVerifySupported: + return m_wasSigned; ++ case RolePartDecryptionSupported: ++ return m_isAllegedlyEncrypted; + case RolePartCryptoNotFinishedYet: + return m_waitingForData || + (m_crypto.valid() && +diff --git a/src/Gui/MessageView.cpp b/src/Gui/MessageView.cpp +index 7d649308..c95e0878 100644 +--- a/src/Gui/MessageView.cpp ++++ b/src/Gui/MessageView.cpp +@@ -354,7 +354,6 @@ bool MessageView::eventFilter(QObject *object, QEvent *event) + QString MessageView::quoteText() const + { + if (auto w = bodyWidget()) { +- QStringList quote = Composer::quoteText(w->quoteMe().split(QLatin1Char('\n'))); + const Imap::Message::Envelope &e = message.data(Imap::Mailbox::RoleMessageEnvelope).value(); + QString sender; + if (!e.from.isEmpty()) +@@ -362,6 +361,14 @@ QString MessageView::quoteText() const + if (e.from.isEmpty()) + sender = tr("you"); + ++ if (messageModel->index(0, 0) /* fake message root */.child(0, 0) /* first MIME part */.data(Imap::Mailbox::RolePartDecryptionSupported).toBool()) { ++ // This is just an UX improvement shortcut: real filtering for CVE-2019-10734 is in ++ // MultipartSignedEncryptedWidget::quoteMe(). ++ // That is required because the encrypted part might not be the root part of the message. ++ return tr("On %1, %2 sent an encrypted message:\n> ...\n\n").arg(e.date.toLocalTime().toString(Qt::SystemLocaleLongDate), sender); ++ } ++ ++ QStringList quote = Composer::quoteText(w->quoteMe().split(QLatin1Char('\n'))); + // One extra newline at the end of the quoted text to separate the response + quote << QString(); + +diff --git a/src/Gui/PartWidget.cpp b/src/Gui/PartWidget.cpp +index bb27604d..96eff338 100644 +--- a/src/Gui/PartWidget.cpp ++++ b/src/Gui/PartWidget.cpp +@@ -378,6 +378,14 @@ void MultipartSignedEncryptedWidget::updateStatusIndicator() + + QString MultipartSignedEncryptedWidget::quoteMe() const + { ++ if (m_partIndex.data(Imap::Mailbox::RolePartDecryptionSupported).toBool()) { ++ // See CVE-2019-10734, the point is not to leak cleartext from encrypted content. Even when Trojita starts supporting ++ // encryption of outgoing mail, we will have to check whether the encrypted cleartext is from the same sender, whether ++ // it matches the list of recipients (which is dynamic and can be set later on), etc etc. ++ // TL;DR, this is a can of worms. ++ return tr("[Encrypted message]"); ++ } ++ + return quoteMeHelper(children()); + } + +diff --git a/src/Imap/Model/ItemRoles.h b/src/Imap/Model/ItemRoles.h +index 4588d4d0..00adb3bb 100644 +--- a/src/Imap/Model/ItemRoles.h ++++ b/src/Imap/Model/ItemRoles.h +@@ -193,7 +193,7 @@ enum { + RolePartSignatureVerifySupported, + /** @short Is the format of this particular multipart/encrypted supported and recognized? + +- See RolePartSignatureVerifySupported, this is an equivalent. ++ If true, this message part represents content of an encrypted message that Trojita can attempt to decrypt. + */ + RolePartDecryptionSupported, + /** @short Is there any point in waiting longer? +-- +GitLab + diff --git a/user/trojita/CVE-2020-15047.patch b/user/trojita/CVE-2020-15047.patch new file mode 100644 index 000000000..6199c01fd --- /dev/null +++ b/user/trojita/CVE-2020-15047.patch @@ -0,0 +1,88 @@ +From 77ddd5d44f2bf4155d0c9b6f7d05f01713b32d5d Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= +Date: Thu, 25 Jun 2020 11:30:51 +0200 +Subject: [PATCH] SMTP: Do not ignore TLS errors + +This fixes a CVE-2020-15047 (category: CWE-295). Since commit 0083eea5ed +which added initial, experimental support for SMTP message submission, +we have apparently never implemented proper SSL/TLS error handling, and +the code has ever since just kept silently ignoring any certificate +verification errors. As a result, Trojita was susceptible to a MITM +attack when sending e-mails. The information leaked include user's +authentication details, including the password, and the content of sent +messages. + +Sorry for this :(. + +Now, this patch re-enabes proper TLS error handling. It was not possible +to directly re-use our code for TLS key pinning which we are using for +IMAP connections. In the Qt TLS code, the decision to accept or not +accept a TLS connection is a blocking one, so the IMAP code relies upon +the protocol state machine (i.e., another layer) for deciding whether to +use or not to use the just-established TLS connection. Implementing an +equivalent code in the SMTP library would be nice, but this hot-fix has +a priority. As a result, SMTP connections to hosts with, e.g., +self-signed TLS certs, are no longer possible. Let's hope that this is +not a practical problem with Lets Encrypt anymore. + +Thanks to Damian Poddebniak for reporting this bug. + +Change-Id: Icd6bbb2b0fb3e45159fc9699ebd07ab84262fe37 +CVE: CVE-2020-15047 +BUG: 423453 +--- + src/MSA/SMTP.cpp | 11 +++++++++-- + src/MSA/SMTP.h | 1 + + 2 files changed, 10 insertions(+), 2 deletions(-) + +diff --git a/src/MSA/SMTP.cpp b/src/MSA/SMTP.cpp +index 3a054516..ac1eefc5 100644 +--- a/src/MSA/SMTP.cpp ++++ b/src/MSA/SMTP.cpp +@@ -21,6 +21,7 @@ + along with this program. If not, see . + */ + #include "SMTP.h" ++#include "UiUtils/Formatting.h" + + namespace MSA + { +@@ -32,8 +33,8 @@ SMTP::SMTP(QObject *parent, const QString &host, quint16 port, bool encryptedCon + user(user), failed(false), isWaitingForPassword(false), sendingMode(MODE_SMTP_INVALID) + { + qwwSmtp = new QwwSmtpClient(this); +- // FIXME: handle SSL errors properly +- connect(qwwSmtp, &QwwSmtpClient::sslErrors, qwwSmtp, &QwwSmtpClient::ignoreSslErrors); ++ // FIXME: handle SSL errors in the same way as we handle IMAP TLS errors, with key pinning, etc. ++ connect(qwwSmtp, &QwwSmtpClient::sslErrors, this, &SMTP::handleSslErrors); + connect(qwwSmtp, &QwwSmtpClient::connected, this, &AbstractMSA::sending); + connect(qwwSmtp, &QwwSmtpClient::done, this, &SMTP::handleDone); + connect(qwwSmtp, &QwwSmtpClient::socketError, this, &SMTP::handleError); +@@ -78,6 +79,12 @@ void SMTP::handleError(QAbstractSocket::SocketError err, const QString &msg) + emit error(msg); + } + ++void SMTP::handleSslErrors(const QList& errors) ++{ ++ auto msg = UiUtils::Formatting::sslErrorsToHtml(errors); ++ emit error(tr("

Cannot send message due to an SSL/TLS error

\n%1").arg(msg)); ++} ++ + void SMTP::setPassword(const QString &password) + { + pass = password; +diff --git a/src/MSA/SMTP.h b/src/MSA/SMTP.h +index 453407d3..913bb873 100644 +--- a/src/MSA/SMTP.h ++++ b/src/MSA/SMTP.h +@@ -43,6 +43,7 @@ public slots: + virtual void setPassword(const QString &password); + void handleDone(bool ok); + void handleError(QAbstractSocket::SocketError err, const QString &msg); ++ void handleSslErrors(const QList& errors); + private: + QwwSmtpClient *qwwSmtp; + QString host; +-- +GitLab + -- cgit v1.2.3-70-g09d2