From 6d4fcb8fd597c39ba0b7f404e64f500b11b75e1c Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sun, 2 Aug 2026 17:49:48 +0200 Subject: fix: enforce !-free cidPrefix invariant at both concatenation sites The cid: namespacing scheme (cid:!) 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" 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. --- src/htmlbuilder.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src/htmlbuilder.cpp') 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:!" 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. -- cgit v1.2.3