Skip to content

Commit 90828ad

Browse files
annejanclaude
andauthored
test(storemodel): three regression tests for path-traversal rejection in dropMimeData (#1466)
#1464 added a canonical-path check in StoreModel::executeDropAction that refuses any drop whose source or destination resolves outside the password store. The patch shipped with unit tests on the Util::isPathInStore helper, but the integration of that helper into the drop flow was only manually verified. These three tests close that gap by crafting mime data with out-of-store source paths and asserting dropMimeData() returns false without ever reaching Pass::Move/Copy. - dropMimeDataRejectsSourceOutsideStore: mime carries /etc/passwd as the source. canDropMimeData() lets it through (it's a UI policy layer, not a fs check); executeDropAction's Util::isPathInStore call must reject. - dropMimeDataRejectsAbsoluteOutsideSource: source is a real filesystem path inside a sibling QTemporaryDir, so the check runs through canonical resolution on an existing path rather than a non-existent one. - dropMimeDataRejectsSymlinkEscape: creates a symlink physically inside the store that resolves to a sibling QTemporaryDir, then drops it onto a store-internal folder. canonicalFilePath() must follow the link and reject. Unix-only — symlink creation on Windows needs developer-mode / elevation. Validated by temporarily defanging the guard (replacing the if condition with `false && (...)`) — all three tests fail without the check, confirming they actually exercise the security boundary. With the guard in place, the qWarning output names each rejection. Build clean. 33/33 storemodel tests pass (was 30, +3 new). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 318fd78 commit 90828ad

1 file changed

Lines changed: 62 additions & 0 deletions

File tree

tests/auto/model/tst_storemodel.cpp

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ private Q_SLOTS:
3131
void canDropFileOnDir();
3232
void canDropFileOnFile();
3333
void canDropDirOnFile();
34+
void dropMimeDataRejectsSourceOutsideStore();
35+
void dropMimeDataRejectsAbsoluteOutsideSource();
36+
void dropMimeDataRejectsSymlinkEscape();
3437
void lessThan();
3538
void lessThanDirsFirst();
3639
void supportedDropActions();
@@ -456,5 +459,64 @@ void tst_storemodel::canDropDirOnFile() {
456459
!fx.sm.canDropMimeData(mime.get(), Qt::MoveAction, 0, 0, fx.fileProxy()));
457460
}
458461

462+
// ---------------------------------------------------------------------------
463+
// dropMimeData() path-traversal rejection (security regression for #1464)
464+
//
465+
// canDropMimeData is a UI policy layer (kind/column/MIME-type constraints)
466+
// and intentionally does not touch the filesystem. The store-boundary check
467+
// lives one layer down in executeDropAction. These tests exercise that
468+
// layer with crafted mime data whose source path escapes the store and
469+
// verify the drop is refused (returns false) without ever reaching Pass::Move
470+
// or Pass::Copy.
471+
// ---------------------------------------------------------------------------
472+
473+
void tst_storemodel::dropMimeDataRejectsSourceOutsideStore() {
474+
DropFixture fx;
475+
// The fs model rooted at the store has no concept of /etc/passwd as a
476+
// child; crafting that path into the mime payload simulates a malicious
477+
// (or buggy) source that doesn't go through StoreModel::mimeData().
478+
auto mime = makeMimeData(dragAndDropInfoPasswordStore::ItemKind::File,
479+
QStringLiteral("/etc/passwd"));
480+
QVERIFY2(
481+
!fx.sm.dropMimeData(mime.get(), Qt::MoveAction, 0, 0, fx.folderProxy()),
482+
"drop with src=/etc/passwd must be refused");
483+
}
484+
485+
void tst_storemodel::dropMimeDataRejectsAbsoluteOutsideSource() {
486+
DropFixture fx;
487+
// Use a path inside QTemporaryDir's parent (system tempdir) but outside
488+
// the store to exercise the canonical-path check on a path that exists
489+
// on disk, not just one that the filesystem can't resolve.
490+
QTemporaryDir outside;
491+
QVERIFY2(outside.isValid(), "outside temp dir should be valid");
492+
auto mime = makeMimeData(dragAndDropInfoPasswordStore::ItemKind::File,
493+
outside.path() + "/elsewhere.gpg");
494+
QVERIFY2(
495+
!fx.sm.dropMimeData(mime.get(), Qt::CopyAction, 0, 0, fx.folderProxy()),
496+
"drop with src in a sibling tempdir must be refused");
497+
}
498+
499+
void tst_storemodel::dropMimeDataRejectsSymlinkEscape() {
500+
#ifdef Q_OS_WIN
501+
QSKIP("symlink creation requires elevation on Windows");
502+
#else
503+
DropFixture fx;
504+
// A symlink physically inside the store that resolves outside —
505+
// canonical-path resolution catches this even though the path string
506+
// starts with the store root.
507+
QTemporaryDir outside;
508+
QVERIFY2(outside.isValid(), "outside temp dir should be valid");
509+
const QString linkPath = fx.tempDir.path() + "/escape";
510+
QVERIFY2(QFile::link(outside.path(), linkPath),
511+
"QFile::link should create symlink in store");
512+
513+
auto mime =
514+
makeMimeData(dragAndDropInfoPasswordStore::ItemKind::File, linkPath);
515+
QVERIFY2(
516+
!fx.sm.dropMimeData(mime.get(), Qt::MoveAction, 0, 0, fx.folderProxy()),
517+
"drop with src=<symlink-out-of-store> must be refused");
518+
#endif
519+
}
520+
459521
QTEST_MAIN(tst_storemodel)
460522
#include "tst_storemodel.moc"

0 commit comments

Comments
 (0)