diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-02 17:49:48 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-04 12:52:32 +0200 |
| commit | b09a44173e8671cbf25aff7db73b34f947ed49cf (patch) | |
| tree | 9b414cddaef9d9941061cb68abc4283d0f5768c0 /tests/test_htmlbuilder.cpp | |
| parent | e8bf3a12cd24780dbe59c8798b1eb6536c9828d0 (diff) | |
| download | qtmaildir-b09a44173e8671cbf25aff7db73b34f947ed49cf.tar.gz qtmaildir-b09a44173e8671cbf25aff7db73b34f947ed49cf.zip | |
fix: enforce !-free cidPrefix invariant at both concatenation sites
The cid: namespacing scheme (cid:<prefix>!<id>) is only unambiguous
because the prefix half is guaranteed free of '!': the first '!' in the
result is always the separator, so an attacker-controlled Content-ID
containing '!' only extends the id half rather than colliding with a
different prefix. That invariant previously existed only as a comment.
Add Q_ASSERT_X at both independent call sites that perform this
concatenation (CidSchemeHandler::namespacedKey and
HtmlBuilder::namespaceCids) so a future prefix generator that violates it
traps in debug builds, per Task 5's precedent of not letting one unit's
correctness depend silently on another's future behaviour. Since
Q_ASSERT compiles out in release, pin the property that actually matters
release builds too test: distinct (prefix, id) pairs across a documented
"m<index>" prefix set and hostile Content-IDs (containing '!', percent-
encoded '!', empty, leading/trailing '!') never collide, and the key
always splits at its first '!' back to the exact original prefix.
Diffstat (limited to 'tests/test_htmlbuilder.cpp')
| -rw-r--r-- | tests/test_htmlbuilder.cpp | 56 |
1 files changed, 56 insertions, 0 deletions
diff --git a/tests/test_htmlbuilder.cpp b/tests/test_htmlbuilder.cpp index 38699f4..86a40cd 100644 --- a/tests/test_htmlbuilder.cpp +++ b/tests/test_htmlbuilder.cpp @@ -1,4 +1,6 @@ +#include <QSet> #include <QtTest> +#include "cidschemehandler.h" #include "htmlbuilder.h" class TestHtmlBuilder : public QObject @@ -21,6 +23,7 @@ private slots: void namespacesCidInStyleBlock(); void namespacesMultipleCidRefsOnOneLine(); void namespacesWhitespaceAroundEquals(); + void namespacedKeyRejectsPrefixContainingSeparator(); }; void TestHtmlBuilder::escapesPlainText() @@ -202,5 +205,58 @@ void TestHtmlBuilder::namespacesWhitespaceAroundEquals() QVERIFY(html.contains(QStringLiteral("cid:m0!logo@example.org"))); } +void TestHtmlBuilder::namespacedKeyRejectsPrefixContainingSeparator() +{ + // The property that actually matters, and holds even in release builds + // where Q_ASSERT is compiled out: distinct (prefix, id) pairs, drawn from + // the documented "m<index>" generator form plus hostile Content-IDs + // (including ones containing '!', a percent-encoded '!', and several + // '!'s), always produce distinct keys, and the key always splits at its + // FIRST '!' back to exactly the original prefix. That only holds because + // prefixes generated in the "m<index>" form are themselves '!'-free; the + // attacker only ever controls the id half, which sits after the first + // (and only guaranteed-separator) '!'. + const QStringList prefixes = { QStringLiteral("m0"), QStringLiteral("m1"), + QStringLiteral("m2"), QStringLiteral("m10") }; + const QStringList hostileIds = { + QStringLiteral("logo@example.org"), + QStringLiteral("a!b@x"), + QStringLiteral("a!b!c@x"), + QStringLiteral("%21encoded@x"), + QStringLiteral(""), + QStringLiteral("!leading@x"), + QStringLiteral("trailing!@x"), + }; + + QSet<QString> seenKeys; + for (const QString &prefix : prefixes) { + for (const QString &id : hostileIds) { + const QString key = CidSchemeHandler::namespacedKey(prefix, id); + + // Distinctness: no other (prefix, id) pair already produced this + // exact key. + QVERIFY2(!seenKeys.contains(key), + qPrintable(QStringLiteral("collision for key '%1'").arg(key))); + seenKeys.insert(key); + + // Splitting at the FIRST '!' always recovers the original + // prefix, regardless of how many '!' the hostile id contributes. + const qsizetype sep = key.indexOf(QLatin1Char('!')); + QVERIFY(sep != -1); + QCOMPARE(key.left(sep), prefix); + QCOMPARE(key.mid(sep + 1), id); + } + } + + // Same property, exercised through HtmlBuilder::namespaceCids, which + // performs the identical concatenation independently: the resulting URL + // must contain the CidSchemeHandler-computed key verbatim. + const QString html = HtmlBuilder::namespaceCids( + QStringLiteral("<img src=\"cid:a!b@x\">"), QStringLiteral("m0")); + const QString expectedKey = CidSchemeHandler::namespacedKey( + QStringLiteral("m0"), QStringLiteral("a!b@x")); + QVERIFY(html.contains(QStringLiteral("cid:%1").arg(expectedKey))); +} + QTEST_MAIN(TestHtmlBuilder) #include "test_htmlbuilder.moc" |
