diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-02 17:37:48 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-02 17:37:48 +0200 |
| commit | 3e540e8b2c79006524ad11d3629361bde663db27 (patch) | |
| tree | 24592b2059a81d806486a9d295c900776e9cc503 | |
| parent | d774f0e94e7c5a7864ca585c5b3493ee9e33fcf6 (diff) | |
| download | qtmaildir-3e540e8b2c79006524ad11d3629361bde663db27.tar.gz qtmaildir-3e540e8b2c79006524ad11d3629361bde663db27.zip | |
feat: add deny-by-default web request interceptor
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/requestinterceptor.cpp | 67 | ||||
| -rw-r--r-- | src/requestinterceptor.h | 42 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_interceptor.cpp | 228 |
5 files changed, 339 insertions, 0 deletions
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8dfd212..e9da44e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -2,6 +2,7 @@ add_library(qtmaildir_lib STATIC keymap.cpp config.cpp mimeparser.cpp + requestinterceptor.cpp ) target_include_directories(qtmaildir_lib diff --git a/src/requestinterceptor.cpp b/src/requestinterceptor.cpp new file mode 100644 index 0000000..0ce95ca --- /dev/null +++ b/src/requestinterceptor.cpp @@ -0,0 +1,67 @@ +#include "requestinterceptor.h" + +#include <QWebEngineUrlRequestInfo> + +RequestInterceptor::RequestInterceptor(QObject *parent) + : QWebEngineUrlRequestInterceptor(parent) +{ +} + +bool RequestInterceptor::shouldAllow(const QUrl &url) +{ + // QUrl::scheme() always normalizes to lowercase (verified: QUrl("HTTP://x/y") + // .scheme() == "http"), so a lowercase-literal compare cannot be bypassed + // by unusual casing, in either the allow or the deny direction. + const QString scheme = url.scheme(); + + // The document itself is loaded via setHtml() with a qtmaildir: base URL, + // so that scheme must pass or nothing renders at all. This is unconditional + // on any path/host because Task 11's scheme handler is the only thing that + // can ever originate a qtmaildir: navigation in the first place; the message + // body cannot cause a request with this scheme, only reference cid:/http(s):. + if (scheme == QLatin1String("qtmaildir")) + return true; + + // Inline parts of the current message only. + if (scheme == QLatin1String("cid")) { + // QUrl keeps a cid: body in path(), not host() or userName(), even + // when it contains '@' (verified empirically: QUrl("cid:logo@example.org") + // .path() == "logo@example.org", host() and userName() are empty). + // path() also returns the percent-decoded form, so a percent-encoded + // id (e.g. "%6Cogo@example.org") compares equal to its decoded form, + // not to some other allowed id: it cannot be used to smuggle a + // foreign id past the allowlist, only to spell an already-legitimate + // id differently. + const QString id = url.path(); + if (m_allowedCids.contains(id)) + return true; + m_blockedAnything = true; + return false; + } + + if (scheme == QLatin1String("http") || scheme == QLatin1String("https")) { + if (m_allowRemote) + return true; + m_blockedAnything = true; + return false; + } + + // Everything else, including file:, javascript:, data:, blob:, about:, + // chrome:, qrc:, filesystem:, protocol-relative URLs (empty scheme with a + // host), and empty/malformed URLs (empty scheme), is denied + // unconditionally. There is no flag that enables it. + m_blockedAnything = true; + return false; +} + +void RequestInterceptor::interceptRequest(QWebEngineUrlRequestInfo &info) +{ + if (!shouldAllow(info.requestUrl())) + info.block(true); +} + +void RequestInterceptor::resetForNewMessage() +{ + m_allowRemote = false; + m_blockedAnything = false; +} diff --git a/src/requestinterceptor.h b/src/requestinterceptor.h new file mode 100644 index 0000000..817a4d1 --- /dev/null +++ b/src/requestinterceptor.h @@ -0,0 +1,42 @@ +#pragma once + +#include <QSet> +#include <QUrl> +#include <QWebEngineUrlRequestInterceptor> + +/// Deny-by-default request policy for the message view. +/// +/// A message body is untrusted input from a stranger. Everything is blocked +/// unless explicitly permitted: remote loads leak the fact that a message was +/// read (tracking pixels) and file: loads would expose the local filesystem. +class RequestInterceptor : public QWebEngineUrlRequestInterceptor +{ + Q_OBJECT +public: + explicit RequestInterceptor(QObject *parent = nullptr); + + /// The whole policy, as a pure function so it can be tested directly. + bool shouldAllow(const QUrl &url); + + void interceptRequest(QWebEngineUrlRequestInfo &info) override; + + /// Content-IDs belonging to the currently displayed message. + void setAllowedCids(const QSet<QString> &cids) { m_allowedCids = cids; } + + /// Per-message opt-in, triggered by the user clicking "Load remote content". + /// Never persisted, never carried to the next message. + void setAllowRemote(bool allow) { m_allowRemote = allow; } + bool allowRemote() const { return m_allowRemote; } + + /// True once any request has been denied, so the UI can offer the button. + bool blockedAnything() const { return m_blockedAnything; } + + /// Called before rendering a new message: clears both the remote grant and + /// the blocked flag. + void resetForNewMessage(); + +private: + QSet<QString> m_allowedCids; + bool m_allowRemote = false; + bool m_blockedAnything = false; +}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 13cb5da..cab8d6c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -10,3 +10,4 @@ add_qtmaildir_test(config) add_qtmaildir_test(mimeparser) target_compile_definitions(test_mimeparser PRIVATE FIXTURE_DIR="${CMAKE_CURRENT_SOURCE_DIR}/fixtures") +add_qtmaildir_test(interceptor) diff --git a/tests/test_interceptor.cpp b/tests/test_interceptor.cpp new file mode 100644 index 0000000..498a997 --- /dev/null +++ b/tests/test_interceptor.cpp @@ -0,0 +1,228 @@ +#include <QtTest> +#include "requestinterceptor.h" + +class TestInterceptor : public QObject +{ + Q_OBJECT +private slots: + void blocksRemoteHttpByDefault(); + void blocksRemoteHttpsByDefault(); + void blocksFileUrlsAlways(); + void allowsCidForCurrentMessage(); + void blocksCidForForeignMessage(); + void allowRemoteFlagPermitsHttpButNotFile(); + void recordsThatSomethingWasBlocked(); + void resetClearsBlockedFlag(); + + // Adversarial additions. + void schemeIsCaseInsensitiveAndStillBlocked(); + void qtmaildirSchemeIsCaseInsensitiveAllow(); + void cidUrlDoesNotParseAsUserinfo(); + void cidPercentEncodingDoesNotBypassAllowlist(); + void javascriptSchemeBlocked(); + void dataSchemeBlocked(); + void blobSchemeBlocked(); + void aboutSchemeBlocked(); + void chromeSchemeBlocked(); + void qrcSchemeBlocked(); + void filesystemSchemeBlocked(); + void protocolRelativeUrlBlocked(); + void emptyUrlBlocked(); + void blankUrlBlocked(); + void colonOnlyUrlBlocked(); + void fragmentOnlyUrlBlocked(); +}; + +void TestInterceptor::blocksRemoteHttpByDefault() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("http://tracker.example/pixel.gif")))); +} + +void TestInterceptor::blocksRemoteHttpsByDefault() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("https://cdn.example/style.css")))); +} + +void TestInterceptor::blocksFileUrlsAlways() +{ + RequestInterceptor interceptor; + interceptor.setAllowRemote(true); + // Even with remote content explicitly allowed, local files stay blocked: + // a message must never read the filesystem. + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("file:///etc/passwd")))); +} + +void TestInterceptor::allowsCidForCurrentMessage() +{ + RequestInterceptor interceptor; + interceptor.setAllowedCids({ QStringLiteral("logo@example.org") }); + QVERIFY(interceptor.shouldAllow(QUrl(QStringLiteral("cid:logo@example.org")))); +} + +void TestInterceptor::blocksCidForForeignMessage() +{ + RequestInterceptor interceptor; + interceptor.setAllowedCids({ QStringLiteral("logo@example.org") }); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("cid:other@example.org")))); +} + +void TestInterceptor::allowRemoteFlagPermitsHttpButNotFile() +{ + RequestInterceptor interceptor; + interceptor.setAllowRemote(true); + QVERIFY(interceptor.shouldAllow(QUrl(QStringLiteral("https://cdn.example/img.png")))); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("file:///etc/passwd")))); +} + +void TestInterceptor::recordsThatSomethingWasBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.blockedAnything()); + interceptor.shouldAllow(QUrl(QStringLiteral("http://tracker.example/p.gif"))); + // Drives the "Remote content blocked" banner in the message header. + QVERIFY(interceptor.blockedAnything()); +} + +void TestInterceptor::resetClearsBlockedFlag() +{ + RequestInterceptor interceptor; + interceptor.shouldAllow(QUrl(QStringLiteral("http://tracker.example/p.gif"))); + QVERIFY(interceptor.blockedAnything()); + + interceptor.resetForNewMessage(); + QVERIFY(!interceptor.blockedAnything()); + // Remote permission never carries over to the next message. + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("https://cdn.example/x.png")))); +} + +void TestInterceptor::schemeIsCaseInsensitiveAndStillBlocked() +{ + // QUrl::scheme() normalizes to lowercase, so "HTTP://..." must still hit + // the http branch (and be blocked without allowRemote), not fall through + // unexpectedly to an allow path. + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("HTTP://tracker.example/pixel.gif")))); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("HTTPS://tracker.example/pixel.gif")))); + + interceptor.setAllowRemote(true); + QVERIFY(interceptor.shouldAllow(QUrl(QStringLiteral("HTTP://tracker.example/pixel.gif")))); +} + +void TestInterceptor::qtmaildirSchemeIsCaseInsensitiveAllow() +{ + RequestInterceptor interceptor; + QVERIFY(interceptor.shouldAllow(QUrl(QStringLiteral("QTMAILDIR://body/index.html")))); +} + +void TestInterceptor::cidUrlDoesNotParseAsUserinfo() +{ + // Pin down QUrl's actual parsing of a cid: URL containing '@', so a + // future Qt version change would be caught here rather than silently + // breaking the allowlist comparison in shouldAllow(). + QUrl url(QStringLiteral("cid:logo@example.org")); + QCOMPARE(url.path(), QStringLiteral("logo@example.org")); + QCOMPARE(url.host(), QString()); + QCOMPARE(url.userName(), QString()); + + RequestInterceptor interceptor; + interceptor.setAllowedCids({ QStringLiteral("logo@example.org") }); + QVERIFY(interceptor.shouldAllow(url)); +} + +void TestInterceptor::cidPercentEncodingDoesNotBypassAllowlist() +{ + // QUrl::path() returns the percent-DECODED form (verified empirically: + // QUrl("cid:%6Cogo@example.org").path() == "logo@example.org", and this + // holds for full-string encodings too). A percent-encoded spelling of an + // allowed id therefore compares equal to that same allowed id, which is + // correct URI equivalence, not a bypass: decoding cannot turn a foreign + // id into a *different* allowed id's literal string, only into its own + // canonical form. What matters for the security boundary is that a + // genuinely foreign id, encoded or not, is still rejected. + RequestInterceptor interceptor; + interceptor.setAllowedCids({ QStringLiteral("logo@example.org") }); + QVERIFY(interceptor.shouldAllow(QUrl(QStringLiteral("cid:%6Cogo@example.org")))); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("cid:other@example.org")))); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("cid:%6Fther@example.org")))); +} + +void TestInterceptor::javascriptSchemeBlocked() +{ + RequestInterceptor interceptor; + interceptor.setAllowRemote(true); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("javascript:alert(1)")))); +} + +void TestInterceptor::dataSchemeBlocked() +{ + RequestInterceptor interceptor; + interceptor.setAllowRemote(true); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("data:text/html,<script>alert(1)</script>")))); +} + +void TestInterceptor::blobSchemeBlocked() +{ + RequestInterceptor interceptor; + interceptor.setAllowRemote(true); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("blob:https://example.org/uuid")))); +} + +void TestInterceptor::aboutSchemeBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("about:blank")))); +} + +void TestInterceptor::chromeSchemeBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("chrome://settings")))); +} + +void TestInterceptor::qrcSchemeBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("qrc:/icons/foo.png")))); +} + +void TestInterceptor::filesystemSchemeBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("filesystem:https://example.org/temporary/foo")))); +} + +void TestInterceptor::protocolRelativeUrlBlocked() +{ + RequestInterceptor interceptor; + interceptor.setAllowRemote(true); + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("//tracker.example/pixel.gif")))); +} + +void TestInterceptor::emptyUrlBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl())); +} + +void TestInterceptor::blankUrlBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("")))); +} + +void TestInterceptor::colonOnlyUrlBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral(":")))); +} + +void TestInterceptor::fragmentOnlyUrlBlocked() +{ + RequestInterceptor interceptor; + QVERIFY(!interceptor.shouldAllow(QUrl(QStringLiteral("#fragment")))); +} + +QTEST_MAIN(TestInterceptor) +#include "test_interceptor.moc" |
