user/trojita: CVE patches (#137)

1 files changed, 104 insertions, 0 deletions

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?= <jkt@kde.org>
+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<Imap::Message::Envelope>();
+         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
+