aboutsummaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-18 16:04:37 +0200
committerDanilo M. <danix@danix.xyz>2026-09-18 16:04:37 +0200
commit7454096cae63d87c75a676751bb07f5650280cd0 (patch)
treeff2d1b2c60f25bbdb5419c9a9259e7d4f160e2e5 /tests
parentf7815097104ae6da9bb763d1b63df55217818445 (diff)
downloadqtmaildir-7454096cae63d87c75a676751bb07f5650280cd0.tar.gz
qtmaildir-7454096cae63d87c75a676751bb07f5650280cd0.zip
fix: sanitise untrusted contact names on insertion
A vCard FN is untrusted and ContactStore faithfully decodes `\n` to a real newline, so `Evil\nBcc: x` was inserted raw into a recipient field and flowed through splitRecipients() to MessageBuilder. contactInsertionText() only quoted a name carrying a comma or a double quote, so every other RFC 5322 special (<, >, ;, @) and any control character reached the header unguarded. The name now has control characters and whitespace runs replaced by single spaces, and every non-empty name is quoted, with `\` escaped before `"`. Quoting contains all the specials in one step. The parser is left faithful; this is fixed at the consumer/trust boundary. Tests: a name with a decoded newline inserts no control character and yields one recipient; a name with <, >, ;, @ is quoted and yields one recipient. The three tests that expected an unquoted plain name now expect the quoted form.
Diffstat (limited to 'tests')
-rw-r--r--tests/test_composewindow.cpp83
1 files changed, 76 insertions, 7 deletions
diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp
index e2673b7..bedd793 100644
--- a/tests/test_composewindow.cpp
+++ b/tests/test_composewindow.cpp
@@ -114,6 +114,8 @@ private slots:
void completionStillWorksAfterAComma();
void insertingACommaNameQuotesItAndKeepsOneRecipient();
void insertingANameWithAQuoteEscapesIt();
+ void insertingANameWithAControlCharacterStripsIt();
+ void insertingANameWithHeaderSpecialsQuotesIt();
void anEmptyNameInsertsTheBareAddress();
void noContactsLeavesTheBehaviourUnchanged();
@@ -1222,10 +1224,11 @@ void TestComposeWindow::completionOffersAContactOnTheFirstRecipient()
QVERIFY2(contactPopup() && contactPopup()->isVisible(),
"typing a contact's name must offer it");
QCOMPARE(contactPopup()->model()->index(0, 0).data(Qt::DisplayRole).toString(),
- QStringLiteral("Alice Example <alice@example.org>"));
+ QStringLiteral("\"Alice Example\" <alice@example.org>"));
acceptFirstPopupRow();
- QCOMPARE(to->text(), QStringLiteral("Alice Example <alice@example.org>"));
+ QCOMPARE(to->text(),
+ QStringLiteral("\"Alice Example\" <alice@example.org>"));
}
/// The candidate is matched on the ADDRESS as well as the name. A single
@@ -1253,10 +1256,11 @@ void TestComposeWindow::completionMatchesTheAddressAsWellAsTheName()
QVERIFY2(contactPopup() && contactPopup()->isVisible(),
"typing an address must offer the contact");
QCOMPARE(contactPopup()->model()->index(0, 0).data(Qt::DisplayRole).toString(),
- QStringLiteral("Alice Example <alice@example.org>"));
+ QStringLiteral("\"Alice Example\" <alice@example.org>"));
acceptFirstPopupRow();
- QCOMPARE(to->text(), QStringLiteral("Alice Example <alice@example.org>"));
+ QCOMPARE(to->text(),
+ QStringLiteral("\"Alice Example\" <alice@example.org>"));
}
/// The case this whole task exists for: `setCompleter()` would set the
@@ -1286,11 +1290,13 @@ void TestComposeWindow::completionStillWorksAfterAComma()
// Bob, not Alice: the prefix is the token after the comma, so Alice must
// not be offered any more.
QCOMPARE(contactPopup()->model()->index(0, 0).data(Qt::DisplayRole).toString(),
- QStringLiteral("Bob Example <bob@example.org>"));
+ QStringLiteral("\"Bob Example\" <bob@example.org>"));
acceptFirstPopupRow();
- QCOMPARE(to->text(),
- QStringLiteral("alice@example.org, Bob Example <bob@example.org>"));
+ QCOMPARE(
+ to->text(),
+ QStringLiteral(
+ "alice@example.org, \"Bob Example\" <bob@example.org>"));
}
/// A display name containing a comma must be QUOTED on insertion, or
@@ -1360,6 +1366,69 @@ void TestComposeWindow::insertingANameWithAQuoteEscapesIt()
QCOMPARE(message.to.size(), 1);
}
+/// FN is untrusted and ContactStore faithfully decodes `\n` to a real newline,
+/// so a hostile card can try to smuggle a header line through a display name.
+/// Nothing the completion inserts may carry a control character.
+void TestComposeWindow::insertingANameWithAControlCharacterStripsIt()
+{
+ const Config config = configWithDrafts();
+
+ ComposeContext context;
+ context.kind = ComposeContext::Kind::New;
+ context.accountKey = QStringLiteral("work");
+
+ ComposeWindow window(context, config, m_dir->path());
+ window.setContacts({ { QStringLiteral("Evil\nBcc: x"),
+ QStringLiteral("evil@example.org") } });
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ auto *to = window.findChild<QLineEdit *>(QStringLiteral("to"));
+ QVERIFY(to);
+ to->setFocus();
+
+ QTest::keyClicks(to, QStringLiteral("Evil"));
+ QVERIFY(contactPopup() && contactPopup()->isVisible());
+ acceptFirstPopupRow();
+
+ for (const QChar c : to->text())
+ QVERIFY2(c.category() != QChar::Other_Control,
+ "the inserted name must carry no control character");
+
+ QCOMPARE(to->text(),
+ QStringLiteral("\"Evil Bcc: x\" <evil@example.org>"));
+ QCOMPARE(window.currentMessage().to.size(), 1);
+}
+
+/// `<`, `>`, `;` and `@` are RFC 5322 specials. Quoting the whole name keeps
+/// every one of them inside the display name instead of in the header grammar.
+void TestComposeWindow::insertingANameWithHeaderSpecialsQuotesIt()
+{
+ const Config config = configWithDrafts();
+
+ ComposeContext context;
+ context.kind = ComposeContext::Kind::New;
+ context.accountKey = QStringLiteral("work");
+
+ ComposeWindow window(context, config, m_dir->path());
+ window.setContacts({ { QStringLiteral("Weird <Name>; @here"),
+ QStringLiteral("weird@example.org") } });
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ auto *to = window.findChild<QLineEdit *>(QStringLiteral("to"));
+ QVERIFY(to);
+ to->setFocus();
+
+ QTest::keyClicks(to, QStringLiteral("Weird"));
+ QVERIFY(contactPopup() && contactPopup()->isVisible());
+ acceptFirstPopupRow();
+
+ QCOMPARE(to->text(),
+ QStringLiteral("\"Weird <Name>; @here\" <weird@example.org>"));
+ QCOMPARE(window.currentMessage().to.size(), 1);
+}
+
/// A card with an address but no name completes on the address alone, with no
/// empty angle brackets.
void TestComposeWindow::anEmptyNameInsertsTheBareAddress()