summaryrefslogtreecommitdiffstats
path: root/tests/test_mimeparser.cpp
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-02 17:32:59 +0200
committerDanilo M. <danix@danix.xyz>2026-08-02 17:32:59 +0200
commitd774f0e94e7c5a7864ca585c5b3493ee9e33fcf6 (patch)
treeaa438d80f2421419f5595a93a423371753df3061 /tests/test_mimeparser.cpp
parent35d3e4ff127b0213fa4d34f630c4a019e533a4df (diff)
downloadqtmaildir-d774f0e94e7c5a7864ca585c5b3493ee9e33fcf6.tar.gz
qtmaildir-d774f0e94e7c5a7864ca585c5b3493ee9e33fcf6.zip
fix: make attachment path-containment guard separator-aware
Attachment::saveTo()'s escape guard compared paths with a bare QString::startsWith(), which is not a path-boundary test: "/tmp/safe-evil" textually starts with "/tmp/safe", so a sibling directory whose name merely extends the target's name would incorrectly pass as contained within it. Extract the check into Attachment::isPathInsideDirectory(), comparing QDir::cleanPath()'d absolute paths and requiring an exact match or a prefix ending at a '/' boundary. Not exploitable today since safeFilename() always reduces the name to a bare basename before saveTo() builds the target, so the guard is unreachable via saveTo()'s public interface; comments on both now say so plainly instead of implying it is currently load-bearing. Add pathInsideDirectoryRejectsSiblingPrefix, testing the guard directly (independent of safeFilename(), which would mask a broken guard by never producing an escaping path), and safeFilenameStripsPathComponents, testing the sanitiser that actually stops traversal today.
Diffstat (limited to 'tests/test_mimeparser.cpp')
-rw-r--r--tests/test_mimeparser.cpp58
1 files changed, 58 insertions, 0 deletions
diff --git a/tests/test_mimeparser.cpp b/tests/test_mimeparser.cpp
index 72bdff7..030f23e 100644
--- a/tests/test_mimeparser.cpp
+++ b/tests/test_mimeparser.cpp
@@ -19,6 +19,8 @@ private slots:
void missingFileIsReported();
void hostileFilenameIsSanitised();
void savedAttachmentMatchesBytes();
+ void safeFilenameStripsPathComponents();
+ void pathInsideDirectoryRejectsSiblingPrefix();
private:
QString fixture(const QString &name) const
@@ -167,5 +169,61 @@ void TestMimeParser::savedAttachmentMatchesBytes()
QCOMPARE(f.readAll(), msg.attachments.first().data);
}
+void TestMimeParser::safeFilenameStripsPathComponents()
+{
+ // This is the control that genuinely stops traversal: saveTo() always
+ // routes through safeFilename() first, so whatever this function
+ // guarantees is what actually protects a write to disk. Constructed by
+ // hand since these are adversarial names not tied to any fixture.
+ Attachment a;
+ a.mimeType = QStringLiteral("text/plain");
+ a.data = QByteArrayLiteral("x");
+
+ a.filename = QStringLiteral("../../../../tmp/pwned.txt");
+ QCOMPARE(a.safeFilename(), QStringLiteral("pwned.txt"));
+
+ a.filename = QStringLiteral("../xyz-evil/x.txt");
+ QCOMPARE(a.safeFilename(), QStringLiteral("x.txt"));
+
+ a.filename = QStringLiteral("..\\..\\windows\\evil.txt");
+ QCOMPARE(a.safeFilename(), QStringLiteral("evil.txt"));
+
+ a.filename = QStringLiteral("plain.txt");
+ QCOMPARE(a.safeFilename(), QStringLiteral("plain.txt"));
+
+ // Nothing usable remains: a generated name is produced instead. Assert
+ // its shape rather than an exact value, since it embeds a fresh UUID.
+ a.filename = QStringLiteral("..");
+ QString generated = a.safeFilename();
+ QVERIFY(!generated.isEmpty());
+ QVERIFY(generated != QStringLiteral(".."));
+ QVERIFY(!generated.contains(QLatin1Char('/')));
+
+ a.filename = QString();
+ generated = a.safeFilename();
+ QVERIFY(!generated.isEmpty());
+ QVERIFY(!generated.contains(QLatin1Char('/')));
+}
+
+void TestMimeParser::pathInsideDirectoryRejectsSiblingPrefix()
+{
+ // Direct test of the containment guard's own comparison, independent of
+ // safeFilename() (which always runs first inside saveTo() and would
+ // mask a broken guard, since it never produces an escaping path). This
+ // targets exactly the defect that was found: a plain string
+ // startsWith() incorrectly treats a sibling directory whose name merely
+ // extends the target's name (e.g. "/tmp/safe-evil") as contained within
+ // it (e.g. "/tmp/safe").
+ const QString base = QStringLiteral("/tmp/safe");
+
+ QVERIFY(Attachment::isPathInsideDirectory(base, base + QStringLiteral("/notes.txt")));
+ QVERIFY(Attachment::isPathInsideDirectory(base, base + QStringLiteral("/sub/notes.txt")));
+ QVERIFY(Attachment::isPathInsideDirectory(base, base));
+
+ QVERIFY(!Attachment::isPathInsideDirectory(base, QStringLiteral("/tmp/safe-evil/x")));
+ QVERIFY(!Attachment::isPathInsideDirectory(base, QStringLiteral("/tmp/safe/../etc/passwd")));
+ QVERIFY(!Attachment::isPathInsideDirectory(base, QStringLiteral("/etc/passwd")));
+}
+
QTEST_MAIN(TestMimeParser)
#include "test_mimeparser.moc"