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