aboutsummaryrefslogtreecommitdiffstats
path: root/src/mimeparser.h
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 /src/mimeparser.h
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 'src/mimeparser.h')
-rw-r--r--src/mimeparser.h18
1 files changed, 18 insertions, 0 deletions
diff --git a/src/mimeparser.h b/src/mimeparser.h
index 27a239f..891478a 100644
--- a/src/mimeparser.h
+++ b/src/mimeparser.h
@@ -27,6 +27,24 @@ struct Attachment
/// Writes the attachment into directory. Returns the full path written, or
/// an empty string on failure with *error set.
QString saveTo(const QString &directory, QString *error) const;
+
+ /// True if candidatePath (need not exist) is directory itself or strictly
+ /// beneath it, by path-boundary comparison after QDir::cleanPath on both
+ /// sides (so ".." segments are resolved rather than compared textually).
+ /// A bare QString::startsWith() is NOT sufficient here: it would let
+ /// "/tmp/safe-evil" pass against "/tmp/safe" since one string is a
+ /// textual prefix of the other despite being sibling directories.
+ ///
+ /// This is defence-in-depth, not currently load-bearing: saveTo() always
+ /// sanitises the name with safeFilename() first, which reduces it to a
+ /// plain basename, so no path reaching this check via saveTo()'s public
+ /// interface can actually fail it today. It exists for a future change
+ /// that stops sanitising, or that accepts a caller-supplied subpath.
+ /// Exposed as its own function so that guarantee can be tested directly,
+ /// independent of safeFilename() — a test driven purely through saveTo()
+ /// cannot exercise this comparison at all, since safeFilename() always
+ /// runs first and never produces a path that could fail it.
+ static bool isPathInsideDirectory(const QString &directory, const QString &candidatePath);
};
struct ParsedMessage