test(email): cover crypto boundary hardening

- test rejection of oversized decrypted plaintext
- test bounded signature verification and exact region bytes
- test hostile MIME nesting, part counts, and boundaries
- add lifecycle and wiping tests for EMailSecret
- update network tests to use secure secret values

4 files changed, 443 insertions, 3 deletions

diff --git a/src/m_email/test/EMailCryptoBoundaryTest.cpp b/src/m_email/test/EMailCryptoBoundaryTest.cpp
index 8393ff2..2858e07 100644
--- a/src/m_email/test/EMailCryptoBoundaryTest.cpp
+++ b/src/m_email/test/EMailCryptoBoundaryTest.cpp
@@ -337,4 +337,136 @@ TEST_F(CryptoBoundaryTest, EncryptingPassesTheBodyOctetsWhole) {
       << "the plaintext was cut at the embedded NUL before encryption";
 }
 
+
+TEST_F(CryptoBoundaryTest, AnOversizedDecryptedPlaintextIsRefused) {
+  // The ceiling on the way in bounds the CIPHERTEXT. An OpenPGP compressed
+  // packet a few megabytes long expands to gigabytes, so the expansion has to
+  // be refused on its own terms rather than trusted because its input was
+  // small.
+  QByteArray eml;
+  eml += "From: Alice <[email protected]>\r\n";
+  eml += "To: Bob <[email protected]>\r\n";
+  eml += "MIME-Version: 1.0\r\n";
+  eml +=
+      "Content-Type: multipart/encrypted; "
+      "protocol=\"application/pgp-encrypted\"; boundary=\"encb\"\r\n";
+  eml += "\r\n--encb\r\n";
+  eml += "Content-Type: application/pgp-encrypted\r\n\r\nVersion: 1\r\n";
+  eml += "\r\n--encb\r\n";
+  eml += "Content-Type: application/octet-stream\r\n\r\n";
+  eml +=
+      "-----BEGIN PGP MESSAGE-----\r\nc3R1Yg==\r\n"
+      "-----END PGP MESSAGE-----\r\n";
+  eml += "\r\n--encb--\r\n";
+
+  // A small ciphertext that "decrypts" to more than the ceiling allows.
+  Get().decrypt_output = QByteArray(kMaxParseInputBytes + 1, 'x');
+
+  EMailMetaData meta;
+  QByteArray out;
+  gpgme_error_t err = 0;
+  QString capsule;
+  const auto ret = DecryptEMLData(0, eml, meta, out, err, capsule);
+
+  EXPECT_EQ(ret, kEML_FAILED);
+  EXPECT_LT(out.size(), kMaxParseInputBytes)
+      << "the oversized plaintext was carried forward anyway";
+}
+
+
+// --- bounded verification work ----------------------------------------------
+
+namespace {
+
+/// A message carrying @p count sibling multipart/signed regions.
+auto ManySignedRegions(int count) -> QByteArray {
+  QByteArray eml;
+  eml += "From: Alice <[email protected]>\r\n";
+  eml += "To: Bob <[email protected]>\r\n";
+  eml += "Subject: many\r\n";
+  eml += "MIME-Version: 1.0\r\n";
+  eml += "Content-Type: multipart/mixed; boundary=\"outer\"\r\n\r\n";
+
+  for (int i = 0; i < count; ++i) {
+    const auto b = QByteArray("sig") + QByteArray::number(i);
+    eml += "--outer\r\n";
+    eml += "Content-Type: multipart/signed; micalg=pgp-sha256; "
+           "protocol=\"application/pgp-signature\"; boundary=\"" + b +
+           "\"\r\n\r\n";
+    eml += "--" + b + "\r\n";
+    eml += "Content-Type: text/plain\r\n\r\npart\r\n";
+    eml += "--" + b + "\r\n";
+    eml += "Content-Type: application/pgp-signature\r\n\r\n";
+    eml += "-----BEGIN PGP SIGNATURE-----\r\naGVsbG8=\r\n"
+           "-----END PGP SIGNATURE-----\r\n";
+    eml += "--" + b + "--\r\n";
+  }
+  eml += "--outer--\r\n";
+  return eml;
+}
+
+}  // namespace
+
+TEST_F(CryptoBoundaryTest, VerificationStopsAtTheRegionBudget) {
+  // Every region costs a blocking GPG call on the GUI thread, with no timeout
+  // and no cancel. Without a cap, a message decides how long the application
+  // stops responding for.
+  const auto raw = ManySignedRegions(kMaxVerifiedRegions + 12);
+
+  EMailPart root;
+  QList<EMailSignatureRegion> regions;
+  vmime::shared_ptr<vmime::message> message;
+  ASSERT_TRUE(CheckIfEMLMessage(raw, message));
+  ASSERT_EQ(ParseMimeTree(message, raw, root, regions), 0);
+  ASSERT_GT(regions.size(), kMaxVerifiedRegions)
+      << "the fixture did not produce enough regions to test the cap";
+
+  QList<EMailSignatureResult> results;
+  const auto verified = VerifyEMLRegions(0, raw, root, regions, results);
+
+  EXPECT_LE(verified, kMaxVerifiedRegions);
+  EXPECT_LE(Get().verify.size(), kMaxVerifiedRegions)
+      << "more GPG calls were made than the budget allows";
+}
+
+TEST_F(CryptoBoundaryTest, AnOrdinaryMessageIsNotAffectedByTheBudget) {
+  // The cap must be well clear of anything real: the worst legitimate case in
+  // the corpus is a nested signature, i.e. two regions.
+  const auto raw = ManySignedRegions(2);
+
+  EMailPart root;
+  QList<EMailSignatureRegion> regions;
+  vmime::shared_ptr<vmime::message> message;
+  ASSERT_TRUE(CheckIfEMLMessage(raw, message));
+  ASSERT_EQ(ParseMimeTree(message, raw, root, regions), 0);
+
+  QList<EMailSignatureResult> results;
+  const auto verified = VerifyEMLRegions(0, raw, root, regions, results);
+
+  EXPECT_EQ(verified, regions.size()) << "a normal message was cut short";
+}
+
+TEST_F(CryptoBoundaryTest, EachRegionIsVerifiedOnItsOwnExactBytes) {
+  // The cap must not disturb which bytes go with which region.
+  const auto raw = ManySignedRegions(3);
+
+  EMailPart root;
+  QList<EMailSignatureRegion> regions;
+  vmime::shared_ptr<vmime::message> message;
+  ASSERT_TRUE(CheckIfEMLMessage(raw, message));
+  ASSERT_EQ(ParseMimeTree(message, raw, root, regions), 0);
+
+  QList<EMailSignatureResult> results;
+  VerifyEMLRegions(0, raw, root, regions, results);
+
+  ASSERT_EQ(Get().verify.size(), regions.size());
+  for (int i = 0; i < regions.size(); ++i) {
+    const auto expected =
+        raw.mid(static_cast<int>(regions[i].raw_offset),
+                static_cast<int>(regions[i].raw_length));
+    EXPECT_EQ(Get().verify[i].data, expected)
+        << "region " << i << " was verified on the wrong bytes";
+  }
+}
+
 }  // namespace
diff --git a/src/m_email/test/EMailNetTest.cpp b/src/m_email/test/EMailNetTest.cpp
index 2258ad7..6241a1d 100644
--- a/src/m_email/test/EMailNetTest.cpp
+++ b/src/m_email/test/EMailNetTest.cpp
@@ -137,7 +137,7 @@ auto TrySmtp(const QStringList& mechanisms, const QString& password,
                      attempt.error = receipt.error;
                    });
 
-  worker.TestConnection(1, account, password);
+  worker.TestConnection(1, account, EMailSecret::CopyFrom(password));
 
   attempt.commands = server.Commands();
   attempt.seen_username = server.seen_username;
@@ -218,7 +218,8 @@ auto Submit(FakeSmtpServer::AfterBody after_body, bool cancel_at_body = false,
     });
   }
 
-  worker.Submit(1, account, "correct-horse", message);
+  worker.Submit(1, account, EMailSecret::CopyFrom("correct-horse"),
+                message);
 
   stop_watching.store(true);
   if (canceller.joinable()) canceller.join();
@@ -391,7 +392,8 @@ class EMailImapNetTest : public ::testing::Test {
         &worker_, &EMailImapWorker::SignalMessages, &worker_,
         [this](quint64, const EMailMessagePage& page) { page_ = page; });
 
-    worker_.Connect(1, ImapAccount(server_.serverPort()), "password");
+    worker_.Connect(1, ImapAccount(server_.serverPort()),
+                    EMailSecret::CopyFrom("password"));
     ASSERT_TRUE(connected_) << last_error_.title.toStdString();
   }
 
diff --git a/src/m_email/test/EMailPublicCorpusTest.cpp b/src/m_email/test/EMailPublicCorpusTest.cpp
index ec3ab2f..eedca0b 100644
--- a/src/m_email/test/EMailPublicCorpusTest.cpp
+++ b/src/m_email/test/EMailPublicCorpusTest.cpp
@@ -47,6 +47,7 @@
 
 #include <QDir>
 #include <QFile>
+#include <QElapsedTimer>
 #include <QSet>
 #include <array>
 
@@ -527,3 +528,142 @@ TEST(EMailPublicCorpusTest, PreambleAndEpilogueAreNotParts) {
         << "epilogue text leaked into part " << part->index;
   }
 }
+
+// ---------------------------------------------------------------------------
+// Hostile shapes: what must be survivable, not merely refused
+//
+// A message has to be parsed before anything can say whether it is signed, let
+// alone trustworthy, so the parser runs on wholly unauthenticated input. The
+// bar is therefore not "is it rejected" but "does the process survive it".
+// ---------------------------------------------------------------------------
+
+namespace {
+
+/// A message nested @p depth multiparts deep. Roughly 60 bytes per level, so
+/// even a very deep one is a small file -- which is the point.
+auto DeeplyNested(int depth) -> QByteArray {
+  QByteArray head =
+      "From: [email protected]\r\nTo: [email protected]\r\nSubject: s\r\n"
+      "MIME-Version: 1.0\r\n";
+  QByteArray body;
+  for (int i = 0; i < depth; ++i) {
+    const auto boundary = QByteArray("b") + QByteArray::number(i);
+    const auto ct =
+        "Content-Type: multipart/mixed; boundary=\"" + boundary + "\"\r\n\r\n";
+    if (i == 0) {
+      head += ct;
+    } else {
+      body += ct;
+    }
+    body += "--" + boundary + "\r\n";
+  }
+  body += "Content-Type: text/plain\r\n\r\ndeep\r\n";
+  return head + body;
+}
+
+}  // namespace
+
+TEST(EMailHostileShapeTest, NestingFarPastTheLimitDoesNotCrashTheProcess) {
+  // 4000 levels is about 240 KB -- nothing by the size ceiling, and it used to
+  // terminate the process with SIGSEGV before any limit was consulted, because
+  // vmime descends by recursion and had no bound of its own. The measured
+  // crash threshold on an 8 MB stack was between 3000 and 3500 levels.
+  //
+  // Reaching the end of this test at all is the assertion.
+  const auto raw = DeeplyNested(4000);
+  ASSERT_LT(raw.size(), 1024 * 1024) << "the fixture itself must stay small";
+
+  vmime::shared_ptr<vmime::message> message;
+  CheckIfEMLMessage(raw, message);
+
+  SUCCEED() << "parsed without exhausting the stack";
+}
+
+TEST(EMailHostileShapeTest, VeryDeepNestingStaysCheap) {
+  // Depth also multiplies work: every level rescans the buffer for its
+  // boundary, so an unbounded descent is depth x size even when it does not
+  // crash. At vmime's own default this shape took over a minute.
+  const auto raw = DeeplyNested(20000);
+
+  QElapsedTimer timer;
+  timer.start();
+
+  vmime::shared_ptr<vmime::message> message;
+  CheckIfEMLMessage(raw, message);
+
+  EXPECT_LT(timer.elapsed(), 10000)
+      << "parsing " << raw.size() << " bytes took " << timer.elapsed() << "ms";
+}
+
+TEST(EMailHostileShapeTest, DeepNestingIsStillRefusedByTheTreeLimits) {
+  // Surviving is not the same as accepting: the module's own walk must still
+  // refuse the shape, so nothing downstream ever sees a 4000-deep tree.
+  const auto raw = DeeplyNested(4000);
+
+  vmime::shared_ptr<vmime::message> message;
+  if (!CheckIfEMLMessage(raw, message)) SUCCEED() << "refused at the parse";
+
+  if (message != nullptr) {
+    EMailPart root;
+    QList<EMailSignatureRegion> regions;
+    EXPECT_EQ(ParseMimeTree(message, raw, root, regions), -1);
+  }
+}
+
+TEST(EMailHostileShapeTest, AnInputOverTheCeilingIsRefusedBeforeParsing) {
+  // The ceiling lives at the parse rather than at each entry point, so every
+  // route in is covered -- including the decrypted plaintext, whose size the
+  // ciphertext says nothing about.
+  QByteArray raw = "From: [email protected]\r\nSubject: s\r\n\r\n";
+  raw += QByteArray(kMaxParseInputBytes + 1 - raw.size(), 'x');
+  ASSERT_GT(raw.size(), kMaxParseInputBytes);
+
+  vmime::shared_ptr<vmime::message> message;
+  EXPECT_FALSE(CheckIfEMLMessage(raw, message));
+}
+
+TEST(EMailHostileShapeTest, AMessageJustUnderTheCeilingIsStillAccepted) {
+  // The limit must not be so eager that ordinary large mail stops working.
+  QByteArray raw =
+      "From: [email protected]\r\nTo: [email protected]\r\nSubject: s\r\n"
+      "MIME-Version: 1.0\r\nContent-Type: text/plain\r\n\r\n";
+  raw += QByteArray(1024 * 1024, 'x');
+
+  vmime::shared_ptr<vmime::message> message;
+  EXPECT_TRUE(CheckIfEMLMessage(raw, message));
+}
+
+TEST(EMailHostileShapeTest, ManyTinyPartsAreRefusedRatherThanWalked) {
+  QByteArray raw =
+      "From: [email protected]\r\nTo: [email protected]\r\nSubject: s\r\n"
+      "MIME-Version: 1.0\r\n"
+      "Content-Type: multipart/mixed; boundary=\"bb\"\r\n\r\n";
+  for (int i = 0; i < 5000; ++i) {
+    raw += "--bb\r\nContent-Type: text/plain\r\n\r\nx\r\n";
+  }
+  raw += "--bb--\r\n";
+
+  vmime::shared_ptr<vmime::message> message;
+  ASSERT_TRUE(CheckIfEMLMessage(raw, message));
+
+  EMailPart root;
+  QList<EMailSignatureRegion> regions;
+  EXPECT_EQ(ParseMimeTree(message, raw, root, regions), -1)
+      << "5000 parts passed a limit of " << EMailParseLimits{}.max_parts;
+}
+
+TEST(EMailHostileShapeTest, PathologicalBoundariesDoNotHang) {
+  // Boundary-like lines that never actually close anything.
+  QByteArray raw =
+      "From: [email protected]\r\nSubject: s\r\nMIME-Version: 1.0\r\n"
+      "Content-Type: multipart/mixed; boundary=\"x\"\r\n\r\n";
+  for (int i = 0; i < 20000; ++i) raw += "--x\r\n";
+
+  QElapsedTimer timer;
+  timer.start();
+
+  vmime::shared_ptr<vmime::message> message;
+  CheckIfEMLMessage(raw, message);
+
+  EXPECT_LT(timer.elapsed(), 10000);
+}
diff --git a/src/m_email/test/EMailSecretTest.cpp b/src/m_email/test/EMailSecretTest.cpp
new file mode 100644
index 0000000..0523960
--- /dev/null
+++ b/src/m_email/test/EMailSecretTest.cpp
@@ -0,0 +1,166 @@
+/**
+ * Copyright (C) 2021-2024 Saturneric <[email protected]>
+ *
+ * This file is part of GpgFrontend.
+ *
+ * GpgFrontend is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GpgFrontend is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GpgFrontend. If not, see <https://www.gnu.org/licenses/>.
+ *
+ * The initial version of the source code is inherited from
+ * the gpg4usb project, which is under GPL-3.0-or-later.
+ *
+ * All the source code of GpgFrontend was modified and released by
+ * Saturneric <[email protected]> starting on May 12, 2021.
+ *
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ */
+
+// Whether a secret can actually be erased.
+//
+// Every wipe in this module used to be `password.fill(QChar('\0'))` on a
+// QString. QString is implicitly shared and fill() detaches before writing, so
+// whenever anything else held a reference -- a queued signal argument, a
+// worker's parameter, an authenticator's member -- the call zeroed a FRESH
+// buffer and dropped the reference to the original, which was then freed with
+// the plaintext still in it. It read as protection and performed none.
+//
+// The first test below demonstrates that on QString directly, so the reason
+// this class exists is recorded rather than asserted. The rest hold
+// EMailSecret to the behaviour that replaces it.
+
+#include <gtest/gtest.h>
+
+#include <QString>
+#include <cstring>
+
+#include <GFSDKBasic.h>
+
+#include "EMailSecret.h"
+
+namespace {
+
+TEST(EMailSecretTest, AQStringWipeDoesNotReachASharedBuffer) {
+  QString original = QStringLiteral("correct-horse-battery");
+  // Force a heap buffer of our own rather than a literal's shared data.
+  original.detach();
+
+  const QString shared = original;  // refcount 2, same buffer
+  const auto* buffer = shared.constData();
+
+  original.fill(QChar('\0'));
+
+  // The copy still reads the secret: original detached and zeroed elsewhere.
+  EXPECT_EQ(shared, QStringLiteral("correct-horse-battery"));
+  EXPECT_EQ(buffer[0], QChar('c'))
+      << "if this ever fails, QString has changed and the note above is stale";
+}
+
+TEST(EMailSecretTest, WipingClearsTheOnlyCopy) {
+  auto secret = EMailSecret::CopyFrom(QStringLiteral("hunter2"));
+  ASSERT_FALSE(secret->IsEmpty());
+  EXPECT_EQ(secret->Size(), 7U);
+
+  secret->Wipe();
+
+  EXPECT_TRUE(secret->IsEmpty());
+  EXPECT_EQ(secret->Size(), 0U);
+  EXPECT_TRUE(secret->StdStringCopy().empty());
+}
+
+TEST(EMailSecretTest, WipingTwiceIsHarmless) {
+  auto secret = EMailSecret::CopyFrom(QStringLiteral("hunter2"));
+  secret->Wipe();
+  secret->Wipe();
+  EXPECT_TRUE(secret->IsEmpty());
+}
+
+TEST(EMailSecretTest, AnEmptySecretIsUsable) {
+  EMailSecret secret;
+  EXPECT_TRUE(secret.IsEmpty());
+  EXPECT_TRUE(secret.StdStringCopy().empty());
+  secret.Wipe();
+  EXPECT_TRUE(secret.IsEmpty());
+}
+
+TEST(EMailSecretTest, MovingTransfersAndLeavesNothingBehind) {
+  EMailSecret from;
+  {
+    auto source = EMailSecret::CopyFrom(QStringLiteral("s3cret"));
+    from = std::move(*source);
+  }
+  EXPECT_EQ(from.StdStringCopy(), "s3cret");
+
+  EMailSecret to = std::move(from);
+  EXPECT_EQ(to.StdStringCopy(), "s3cret");
+  EXPECT_TRUE(from.IsEmpty()) << "the moved-from secret still holds bytes";
+}
+
+TEST(EMailSecretTest, MoveAssignmentWipesWhatItReplaces) {
+  auto first = EMailSecret::CopyFrom(QStringLiteral("first-secret"));
+  auto second = EMailSecret::CopyFrom(QStringLiteral("second"));
+
+  *first = std::move(*second);
+
+  EXPECT_EQ(first->StdStringCopy(), "second");
+  EXPECT_TRUE(second->IsEmpty());
+}
+
+TEST(EMailSecretTest, TheLastHolderReleasingErasesTheBytes) {
+  // The property the queued-connection hand-off depends on: the GUI thread
+  // drops its reference immediately after posting, and the worker's copy is
+  // the same bytes, not a duplicate.
+  EMailSecretPtr shared;
+  {
+    auto secret = EMailSecret::CopyFrom(QStringLiteral("shared-secret"));
+    shared = secret;
+    EXPECT_EQ(shared.use_count(), 2);
+    secret.reset();
+  }
+  ASSERT_EQ(shared.use_count(), 1);
+  EXPECT_EQ(shared->StdStringCopy(), "shared-secret")
+      << "releasing one holder erased bytes the other still needed";
+}
+
+TEST(EMailSecretTest, AdoptingACStringWipesTheSource) {
+  // The SDK hands back a secure char*. Copying it out has to leave nothing
+  // behind in the buffer it came from.
+  const char* text = "from-the-sdk";
+  const auto length = std::strlen(text);
+
+  auto* raw = static_cast<char*>(GFSecAllocateMemory(length + 1));
+  std::memcpy(raw, text, length + 1);
+
+  auto secret = EMailSecret::AdoptCString(raw);
+
+  EXPECT_EQ(secret->StdStringCopy(), "from-the-sdk");
+  EXPECT_EQ(secret->Size(), length);
+}
+
+TEST(EMailSecretTest, AdoptingNullIsEmptyRatherThanACrash) {
+  auto secret = EMailSecret::AdoptCString(nullptr);
+  ASSERT_NE(secret, nullptr);
+  EXPECT_TRUE(secret->IsEmpty());
+}
+
+TEST(EMailSecretTest, ASecretIsNotCopyable) {
+  // Compile-time, and the point of the class: an accidental copy is another
+  // plaintext buffer nobody remembers to erase.
+  static_assert(!std::is_copy_constructible_v<EMailSecret>);
+  static_assert(!std::is_copy_assignable_v<EMailSecret>);
+  static_assert(std::is_move_constructible_v<EMailSecret>);
+  static_assert(std::is_move_assignable_v<EMailSecret>);
+  SUCCEED();
+}
+
+}  // namespace