aboutsummaryrefslogtreecommitdiffstats
path: root/src/htmlbuilder.cpp
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-02 17:49:48 +0200
committerDanilo M. <danix@danix.xyz>2026-08-04 12:52:32 +0200
commitb09a44173e8671cbf25aff7db73b34f947ed49cf (patch)
tree9b414cddaef9d9941061cb68abc4283d0f5768c0 /src/htmlbuilder.cpp
parente8bf3a12cd24780dbe59c8798b1eb6536c9828d0 (diff)
downloadqtmaildir-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 'src/htmlbuilder.cpp')
-rw-r--r--src/htmlbuilder.cpp13
1 files changed, 13 insertions, 0 deletions
diff --git a/src/htmlbuilder.cpp b/src/htmlbuilder.cpp
index 1830383..89501ea 100644
--- a/src/htmlbuilder.cpp
+++ b/src/htmlbuilder.cpp
@@ -56,6 +56,19 @@ QString HtmlBuilder::namespaceCids(const QString &html, const QString &prefix)
if (prefix.isEmpty())
return html;
+ // The '!' separator is only unambiguous if prefix itself never contains
+ // one: the FIRST '!' in "cid:<prefix>!<id>" is always taken as the
+ // separator, so a hostile Content-ID containing '!' only extends the id
+ // half, never collides with a different prefix. This concatenation is
+ // performed independently in two places (here and
+ // CidSchemeHandler::namespacedKey); neither trusts the other to have
+ // checked it, so both assert it directly. Q_ASSERT is compiled out in
+ // release builds — the property that matters there is pinned by
+ // TestHtmlBuilder::namespacedKeyRejectsPrefixContainingSeparator instead.
+ Q_ASSERT_X(!prefix.contains(QLatin1Char('!')), "HtmlBuilder::namespaceCids",
+ "cidPrefix must never contain '!': it is the separator, and a "
+ "prefix containing one would make the split ambiguous");
+
// This runs on the sender's raw, unescaped HTML markup (not on text that
// has been through toHtmlEscaped()), so no double-escaping happens here;
// it is purely a URL rewrite over the existing markup.