CVE-2014-2567: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: The OpenConnectionTask::handleStateHelper function in Imap/Tasks/OpenConnectionTask.cpp in Trojita before 0.4.1 allows man-in-the-middle attackers to trigger use of cleartext for saving a message into a (1) sent or (2) draft folder via a PREAUTH response that prevents later use of the STARTTLS command.
CWE description: The product exposes sensitive information to an actor that is not explicitly authorized to have access to that information.
Published: 2014-03-21T10:55Z
Repository: https://github.com/jktjkt/trojita
Language: C++
Files changed: src/Imap/Tasks/OpenConnectionTask.cpp, tests/Imap/test_Imap_Tasks_OpenConnection.cpp, tests/Imap/test_Imap_Tasks_OpenConnection.h
Commit date: 2014-03-20 20:44:04 +0100
Commit message: IMAP: refuse to work when STARTTLS is required but server sends PREAUTH

Oops, we cannot send STARTTLS when the connection is already authenticated.
This is serious enough to warrant an error; an attacker might be going after a
plaintext of a message we're going to APPEND, etc.

Thanks to Arnt Gulbrandsen on the imap-protocol ML for asking what happens when
we're configured to request STARTTLS and a PREAUTH is received, and to Michael M
Slusarz for starting that discussion.

Hope the error message is readable enough.

CVE: CVE-2014-2567

Diff: diff --git a/src/Imap/Tasks/OpenConnectionTask.cpp b/src/Imap/Tasks/OpenConnectionTask.cpp
index 2957550f..fc4e6be5 100644
--- a/src/Imap/Tasks/OpenConnectionTask.cpp
+++ b/src/Imap/Tasks/OpenConnectionTask.cpp
@@ -151,10 +151,21 @@ bool OpenConnectionTask::handleStateHelper(const Imap::Responses::State *const r
     case CONN_STATE_CONNECTED_PRETLS_PRECAPS:
         // We're connected now -- this is our initial state.
     {
         switch (resp->kind) {
         case PREAUTH:
+            if (model->m_startTls) {
+                // Oops, we cannot send STARTTLS when the connection is already authenticated.
+                // This is serious enough to warrant an error; an attacker might be going after a plaintext
+                // of a message we're going to APPEND, etc.
+                // Thanks to Arnt Gulbrandsen on the imap-protocol ML for asking what happens when we're configured
+                // to request STARTTLS and a PREAUTH is received, and to Michael M Slusarz for starting that discussion.
+                logout(tr("Configuration requires sending STARTTLS, but the IMAP server greets us with PREAUTH. "
+                          "Encryption cannot be established. If this configuration worked previously, someone "
+                          "is after your data and they are pretty smart."));
+                return true;
+            }
             // Cool, we're already authenticated. Now, let's see if we have to issue CAPABILITY or if we already know that
             if (model->accessParser(parser).capabilitiesFresh) {
                 // We're alsmost done here, apart from compression
                 if (TROJITA_COMPRESS_DEFLATE && model->accessParser(parser).capabilities.contains(QLatin1String("COMPRESS=DEFLATE"))) {
                     compressCmd = parser->compressDeflate();
diff --git a/tests/Imap/test_Imap_Tasks_OpenConnection.cpp b/tests/Imap/test_Imap_Tasks_OpenConnection.cpp
index e6aaf3d5..f186c68c 100644
--- a/tests/Imap/test_Imap_Tasks_OpenConnection.cpp
+++ b/tests/Imap/test_Imap_Tasks_OpenConnection.cpp
@@ -127,10 +127,23 @@ void ImapModelOpenConnectionTest::testPreauthWithCapability()
     QVERIFY(failedSpy->isEmpty());
     QVERIFY( authSpy->isEmpty() );
     QVERIFY(startTlsUpgradeSpy->isEmpty());
 }
 
+/** @short What happens when the server responds with PREAUTH and we want STARTTLS? */
+void ImapModelOpenConnectionTest::testPreauthWithStartTlsWanted()
+{
+    cleanup(); init(true); // yuck, but I can't come up with anything better...
+
+    cEmpty();
+    cServer("* PREAUTH hi there\r\n");
+    QCOMPARE(failedSpy->size(), 1);
+    QVERIFY(completedSpy->isEmpty());
+    QVERIFY(authSpy->isEmpty());
+    QVERIFY(startTlsUpgradeSpy->isEmpty());
+}
+
 /** @short Test for obtaining capability and logging in without any STARTTLS */
 void ImapModelOpenConnectionTest::testOk()
 {
     QCoreApplication::processEvents();
     QCoreApplication::processEvents();
diff --git a/tests/Imap/test_Imap_Tasks_OpenConnection.h b/tests/Imap/test_Imap_Tasks_OpenConnection.h
index 23107a3f..dcc77c0d 100644
--- a/tests/Imap/test_Imap_Tasks_OpenConnection.h
+++ b/tests/Imap/test_Imap_Tasks_OpenConnection.h
@@ -37,10 +37,11 @@ private slots:
     void cleanup();
     void initTestCase();
 
     void testPreauth();
     void testPreauthWithCapability();
+    void testPreauthWithStartTlsWanted();
 
     void testOk();
     void testOkWithCapability();
     void testOkMissingImap4rev1();
 

Fixed code:             if (model->m_startTls) {
                // Oops, we cannot send STARTTLS when the connection is already authenticated.
                // This is serious enough to warrant an error; an attacker might be going after a plaintext
                // of a message we're going to APPEND, etc.
                // Thanks to Arnt Gulbrandsen on the imap-protocol ML for asking what happens when we're configured
                // to request STARTTLS and a PREAUTH is received, and to Michael M Slusarz for starting that discussion.
                logout(tr("Configuration requires sending STARTTLS, but the IMAP server greets us with PREAUTH. "
                          "Encryption cannot be established. If this configuration worked previously, someone "
                          "is after your data and they are pretty smart."));
                return true;
            }
/** @short What happens when the server responds with PREAUTH and we want STARTTLS? */
void ImapModelOpenConnectionTest::testPreauthWithStartTlsWanted()
{
    cleanup(); init(true); // yuck, but I can't come up with anything better...

    cEmpty();
    cServer("* PREAUTH hi there\r\n");
    QCOMPARE(failedSpy->size(), 1);
    QVERIFY(completedSpy->isEmpty());
    QVERIFY(authSpy->isEmpty());
    QVERIFY(startTlsUpgradeSpy->isEmpty());
}

    void testPreauthWithStartTlsWanted();