diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-02 17:32:59 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-04 12:52:22 +0200 |
| commit | 18dffc348f722fc79ed336523982541283bc84e1 (patch) | |
| tree | 975f2de0e891dd988c4c19471cc4620a35926089 /src/mimeparser.h | |
| parent | 5f22f80b6b5cef5e768086338ae4b22120f3e67c (diff) | |
| download | qtmaildir-18dffc348f722fc79ed336523982541283bc84e1.tar.gz qtmaildir-18dffc348f722fc79ed336523982541283bc84e1.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.h | 18 |
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 |
