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/CVE-2019-10734.patch | 104 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 user/trojita/CVE-2019-10734.patch (limited to 'user/trojita/CVE-2019-10734.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 + -- cgit v1.2.3-70-g09d2