moveToTrash/Unix: refactor to use openat()/mkdirat()/renameat()

This ensures much better security against race conditions and attacks,
at the expense of a few more system calls.

On first run (when no trash dir is yet present):

  openat(AT_FDCWD, "/home/tjmaciei/.qttest/share/Trash", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
  mkdirat(AT_FDCWD, "/home/tjmaciei/.qttest/share/Trash", 0700) = 0
  openat(AT_FDCWD, "/home/tjmaciei/.qttest/share/Trash", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = 5
  newfstatat(5, "", {st_mode=S_IFDIR|0700, st_size=0, ...}, AT_EMPTY_PATH) = 0
  getuid()                                = 1000
  openat(5, "files", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
  mkdirat(5, "files", 0700)               = 0
  openat(5, "files", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = 6
  openat(5, "info", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
  mkdirat(5, "info", 0700)                = 0
  openat(5, "info", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = 7
  close(5)                                = 0
  openat(7, "tst_qfile.moveToTrashOpenFile.fjYRxv.trashinfo", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 5
  openat(AT_FDCWD, "/usr/share/zoneinfo/UTC", O_RDONLY|O_CLOEXEC) = 8
  newfstatat(8, "", {st_mode=S_IFREG|0644, st_size=114, ...}, AT_EMPTY_PATH) = 0
  newfstatat(8, "", {st_mode=S_IFREG|0644, st_size=114, ...}, AT_EMPTY_PATH) = 0
  read(8, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 114
  lseek(8, -60, SEEK_CUR)                 = 54
  read(8, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 60
  close(8)                                = 0
  write(5, "[Trash Info]\nPath=/home/tjmaciei"..., 103) = 103
  renameat(AT_FDCWD, "/home/tjmaciei/tst_qfile.moveToTrashOpenFile.fjYRxv", 6, "tst_qfile.moveToTrashOpenFile.fjYRxv") = 0
  close(5)                                = 0
  close(6)                                = 0
  close(7)                                = 0

On subsequent runs:

  openat(AT_FDCWD, "/home/tjmaciei/.qttest/share/Trash", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = 5
  newfstatat(5, "", {st_mode=S_IFDIR|0700, st_size=18, ...}, AT_EMPTY_PATH) = 0
  getuid()                                = 1000
  openat(5, "files", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = 6
  openat(5, "info", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY) = 7
  close(5)                                = 0
  openat(7, "tst_qfile.moveToTrashOpenFile.sPjrcA.trashinfo", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = 5
  openat(AT_FDCWD, "/usr/share/zoneinfo/UTC", O_RDONLY|O_CLOEXEC) = 8
  newfstatat(8, "", {st_mode=S_IFREG|0644, st_size=114, ...}, AT_EMPTY_PATH) = 0
  newfstatat(8, "", {st_mode=S_IFREG|0644, st_size=114, ...}, AT_EMPTY_PATH) = 0
  read(8, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 114
  lseek(8, -60, SEEK_CUR)                 = 54
  read(8, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 60
  close(8)                                = 0
  write(5, "[Trash Info]\nPath=/home/tjmaciei"..., 103) = 103
  renameat(AT_FDCWD, "/home/tjmaciei/tst_qfile.moveToTrashOpenFile.sPjrcA", 6, "tst_qfile.moveToTrashOpenFile.sPjrcA") = 0
  close(5)                                = 0
  close(6)                                = 0
  close(7)                                = 0

Change-Id: I9d43e5b91eb142d6945cfffd1787117927650dab
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
This commit is contained in:
Thiago Macieira 2023-09-21 17:36:36 -07:00
parent 25b1990784
commit c94bed69b7
3 changed files with 139 additions and 81 deletions

View File

@ -1180,7 +1180,7 @@ bool QFileSystemEngine::createLink(const QFileSystemEntry &source, const QFileSy
#ifdef Q_OS_DARWIN #ifdef Q_OS_DARWIN
// see qfilesystemengine_mac.mm // see qfilesystemengine_mac.mm
#elif defined(QT_BOOTSTRAPPED) #elif defined(QT_BOOTSTRAPPED) || !defined(AT_FDCWD)
// bootstrapped tools don't need this, and we don't want QStorageInfo // bootstrapped tools don't need this, and we don't want QStorageInfo
//static //static
bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &, QFileSystemEntry &, bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &, QFileSystemEntry &,
@ -1197,29 +1197,49 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &, QFileSystemEnt
namespace { namespace {
struct FreeDesktopTrashOperation struct FreeDesktopTrashOperation
{ {
QString infoDir; /*
QFileSystemEntry infoFilePath; "A trash directory contains two subdirectories, named info and files."
int infoFileFd = -1; */
QString trashPath;
int filesDirFd = -1;
int infoDirFd = -1;
qsizetype volumePrefixLength = 0;
// relative to infoDirFd from above
QByteArray infoFilePath;
int infoFileFd = -1; // if we've already opened it
~FreeDesktopTrashOperation() ~FreeDesktopTrashOperation()
{ {
close(); close();
} }
constexpr bool isTrashDirOpen() const { return filesDirFd != -1 && infoDirFd != -1; }
void close() void close()
{ {
int savedErrno = errno;
if (infoFileFd != -1) { if (infoFileFd != -1) {
Q_ASSERT(infoDirFd != -1);
Q_ASSERT(!infoFilePath.isEmpty());
Q_ASSERT(!trashPath.isEmpty());
QT_CLOSE(infoFileFd); QT_CLOSE(infoFileFd);
QSystemError ignoredError; unlinkat(infoDirFd, infoFilePath, 0);
QFileSystemEngine::removeFile(infoFilePath, ignoredError); infoFileFd = -1;
} }
if (filesDirFd >= 0)
QT_CLOSE(filesDirFd);
if (infoDirFd >= 0)
QT_CLOSE(infoDirFd);
filesDirFd = infoDirFd = -1;
errno = savedErrno;
} }
bool tryCreateInfoFile(const QString &filePath, QSystemError &error) bool tryCreateInfoFile(const QString &filePath, QSystemError &error)
{ {
Q_ASSERT(filePath.startsWith(u'/')); QByteArray p = QFile::encodeName(filePath) + ".trashinfo";
QFileSystemEntry p(infoDir + filePath + ".trashinfo"_L1); infoFileFd = qt_safe_openat(infoDirFd, p, QT_OPEN_RDWR | QT_OPEN_CREAT | QT_OPEN_EXCL, 0666);
infoFileFd = QT_OPEN(p.nativeFilePath(), QT_OPEN_RDWR | QT_OPEN_CREAT | QT_OPEN_EXCL, 0666);
if (infoFileFd < 0) { if (infoFileFd < 0) {
error = QSystemError(errno, QSystemError::StandardLibraryError); error = QSystemError(errno, QSystemError::StandardLibraryError);
return false; return false;
@ -1233,31 +1253,67 @@ struct FreeDesktopTrashOperation
QT_CLOSE(infoFileFd); QT_CLOSE(infoFileFd);
infoFileFd = -1; infoFileFd = -1;
} }
};
static auto freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemError &error) // opens a directory and returns the file descriptor
static int openDirFd(int dfd, const char *path, int mode = 0)
{ {
auto makeTrashDir = [](const QDir &topDir, const QString &trashDir, QSystemError &error) { mode |= QT_OPEN_RDONLY | O_NOFOLLOW | O_DIRECTORY;
auto ownerPerms = QFileDevice::ReadOwner return qt_safe_openat(dfd, path, mode);
| QFileDevice::WriteOwner }
| QFileDevice::ExeOwner;
QString targetDir = topDir.filePath(trashDir); // opens an XDG Trash directory that is a subdirectory of dfd, creating if necessary
// deliberately not using mkpath, since we want to fail if topDir doesn't exist static int openOrCreateDir(int dfd, const char *path)
bool created = QFileSystemEngine::createDirectory(QFileSystemEntry(targetDir), false, ownerPerms); {
if (created) // try to open it as a dir, first
return targetDir; int fd = openDirFd(dfd, path);
if (fd >= 0 || errno != ENOENT)
return fd;
// try to mkdirat
if (mkdirat(dfd, path, 0700) < 0)
return -1;
// try to open it again
return openDirFd(dfd, path);
}
// opens or makes the XDG Trash hierarchy on parentfd (may be -1) called targetDir
bool getTrashDir(int parentfd, QString targetDir, QSystemError &error)
{
if (parentfd == AT_FDCWD)
trashPath = targetDir;
QByteArray nativePath = QFile::encodeName(targetDir);
// open the directory
int trashfd = openOrCreateDir(parentfd, nativePath);
if (trashfd < 0 && errno != ENOENT) {
error = QSystemError(errno, QSystemError::StandardLibraryError); error = QSystemError(errno, QSystemError::StandardLibraryError);
return false;
}
// maybe it already exists and is a directory // check if it is ours (even if we've just mkdirat'ed it)
if (QFileInfo(targetDir).isDir()) if (QT_STATBUF st; QT_FSTAT(trashfd, &st) < 0) {
return targetDir; return false;
return QString(); } else if (st.st_uid != getuid()) {
error = QSystemError(EPERM, QSystemError::StandardLibraryError);
return false;
}
filesDirFd = openOrCreateDir(trashfd, "files");
if (filesDirFd >= 0)
infoDirFd = openOrCreateDir(trashfd, "info");
error = QSystemError(errno, QSystemError::StandardLibraryError);
if (infoDirFd < 0)
close();
QT_CLOSE(trashfd);
return infoDirFd >= 0;
}
bool findTrashFor(const QFileSystemEntry &source, QSystemError &error);
}; };
struct R {
QString trashDir;
qsizetype volumePrefixLength = 0;
} r;
bool FreeDesktopTrashOperation::findTrashFor(const QFileSystemEntry &source, QSystemError &error)
{
// first, check if they are in the same device // first, check if they are in the same device
QString homePath = QFileSystemEngine::homePath(); QString homePath = QFileSystemEngine::homePath();
const QString sourcePath = source.filePath(); const QString sourcePath = source.filePath();
@ -1265,7 +1321,7 @@ static auto freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemErro
if (QT_STAT(QFile::encodeName(sourcePath), &sourceInfo) != 0 || if (QT_STAT(QFile::encodeName(sourcePath), &sourceInfo) != 0 ||
QT_STAT(QFile::encodeName(homePath), &homeInfo) != 0) { QT_STAT(QFile::encodeName(homePath), &homeInfo) != 0) {
error = QSystemError(errno, QSystemError::StandardLibraryError); error = QSystemError(errno, QSystemError::StandardLibraryError);
return r; return false;
} }
const QStorageInfo sourceStorage(sourcePath); const QStorageInfo sourceStorage(sourcePath);
@ -1276,8 +1332,6 @@ static auto freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemErro
isHomeVolume = sourceStorage == QStorageInfo(QFileSystemEngine::homePath()); isHomeVolume = sourceStorage == QStorageInfo(QFileSystemEngine::homePath());
} }
QString &trash = r.trashDir;
const QStorageInfo homeStorage(QDir::home());
// We support trashing of files outside the users home partition // We support trashing of files outside the users home partition
if (!isHomeVolume) { if (!isHomeVolume) {
const auto dotTrash = "/.Trash"_L1; const auto dotTrash = "/.Trash"_L1;
@ -1295,20 +1349,27 @@ static auto freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemErro
*/ */
const QString userID = QString::number(::getuid()); const QString userID = QString::number(::getuid());
if (QT_STATBUF st; QT_LSTAT(dotTrashDir.nativeFilePath(), &st) == 0) {
// we MUST check that the sticky bit is set, and that it is not a symlink // we MUST check that the sticky bit is set, and that it is not a symlink
if (S_ISLNK(st.st_mode)) { int genericTrashFd = openDirFd(AT_FDCWD, dotTrashDir.nativeFilePath());
QT_STATBUF st = {};
if (genericTrashFd < 0 && errno != ENOENT && errno != EACCES) {
// O_DIRECTORY + O_NOFOLLOW produces ENOTDIR on Linux
if (QT_LSTAT(dotTrashDir.nativeFilePath(), &st) == 0 && S_ISLNK(st.st_mode)) {
// we SHOULD report the failed check to the administrator // we SHOULD report the failed check to the administrator
qCritical("Warning: '%s' is a symlink to '%s'", qCritical("Warning: '%s' is a symlink to '%s'",
dotTrashDir.nativeFilePath().constData(), dotTrashDir.nativeFilePath().constData(),
qt_readlink(dotTrashDir.nativeFilePath()).constData()); qt_readlink(dotTrashDir.nativeFilePath()).constData());
error = QSystemError(ELOOP, QSystemError::StandardLibraryError); error = QSystemError(ELOOP, QSystemError::StandardLibraryError);
} else if ((st.st_mode & S_ISVTX) == 0) { }
} else if (genericTrashFd >= 0) {
QT_FSTAT(genericTrashFd, &st);
if ((st.st_mode & S_ISVTX) == 0) {
// we SHOULD report the failed check to the administrator // we SHOULD report the failed check to the administrator
qCritical("Warning: '%s' doesn't have sticky bit set!", qCritical("Warning: '%s' doesn't have sticky bit set!",
dotTrashDir.nativeFilePath().constData()); dotTrashDir.nativeFilePath().constData());
error = QSystemError(EPERM, QSystemError::StandardLibraryError); error = QSystemError(EPERM, QSystemError::StandardLibraryError);
} else if (S_ISDIR(st.st_mode)) { } else {
/* /*
"If the directory exists and passes the checks, a subdirectory of the "If the directory exists and passes the checks, a subdirectory of the
$topdir/.Trash directory is to be used as the user's trash directory $topdir/.Trash directory is to be used as the user's trash directory
@ -1318,9 +1379,14 @@ static auto freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemErro
the implementation MUST immediately create it, without any warnings or the implementation MUST immediately create it, without any warnings or
delays for the user." delays for the user."
*/ */
trash = makeTrashDir(dotTrashDir.filePath(), userID, error); if (getTrashDir(genericTrashFd, userID, error)) {
// recreate the resulting path
trashPath = dotTrashDir.filePath() + u'/' + userID;
} }
} }
QT_CLOSE(genericTrashFd);
}
/* /*
Method 2: Method 2:
"If an $topdir/.Trash directory is absent, an $topdir/.Trash-$uid directory is to be "If an $topdir/.Trash directory is absent, an $topdir/.Trash-$uid directory is to be
@ -1328,19 +1394,18 @@ static auto freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemErro
file, if an $topdir/.Trash-$uid directory does not exist, the implementation MUST file, if an $topdir/.Trash-$uid directory does not exist, the implementation MUST
immediately create it, without any warnings or delays for the user." immediately create it, without any warnings or delays for the user."
*/ */
if (trash.isEmpty()) { if (!isTrashDirOpen())
const QString userTrashDir = dotTrash + u'-' + userID; getTrashDir(AT_FDCWD, sourceStorage.rootPath() + dotTrash + u'-' + userID, error);
trash = makeTrashDir(QDir(sourceStorage.rootPath() + userTrashDir), QString(), error);
if (isTrashDirOpen()) {
volumePrefixLength = sourceStorage.rootPath().size();
if (volumePrefixLength == 1)
volumePrefixLength = 0; // isRoot
else
++volumePrefixLength; // to include the slash
}
} }
if (!trash.isEmpty()) {
r.volumePrefixLength = sourceStorage.rootPath().size();
if (r.volumePrefixLength == 1)
r.volumePrefixLength = 0; // isRoot
else
++r.volumePrefixLength; // to include the slash
}
}
/* /*
"If both (1) and (2) fail [...], the implementation MUST either trash the "If both (1) and (2) fail [...], the implementation MUST either trash the
file into the user's home trash or refuse to trash it." file into the user's home trash or refuse to trash it."
@ -1351,16 +1416,13 @@ static auto freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemErro
QStandardPaths returns for GenericDataLocation. If that doesn't exist, then QStandardPaths returns for GenericDataLocation. If that doesn't exist, then
we are not running on a freedesktop.org-compliant environment, and give up. we are not running on a freedesktop.org-compliant environment, and give up.
*/ */
if (trash.isEmpty()) { if (!isTrashDirOpen()) {
QDir topDir = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation); QString topDir = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation);
trash = makeTrashDir(topDir, "Trash"_L1, error); if (!getTrashDir(AT_FDCWD, topDir + "/Trash"_L1, error))
if (trash.isEmpty()) { qWarning("Unable to establish trash directory in %ls", qUtf16Printable(topDir));
qWarning("Unable to establish trash directory in %s",
topDir.path().toLocal8Bit().constData());
}
} }
return r; return isTrashDirOpen();
} }
} // unnamed namespace } // unnamed namespace
@ -1375,26 +1437,9 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source,
} }
return absoluteName(source); return absoluteName(source);
}(); }();
auto [trashPath, volumePrefixLength] = freeDesktopTrashLocation(sourcePath, error);
FreeDesktopTrashOperation op; FreeDesktopTrashOperation op;
if (trashPath.isEmpty()) if (!op.findTrashFor(sourcePath, error))
return false; return false;
QDir trashDir(trashPath);
/*
"A trash directory contains two subdirectories, named info and files."
*/
const auto filesDir = "files"_L1;
const auto infoDir = "info"_L1;
trashDir.mkdir(filesDir);
int savedErrno = errno;
trashDir.mkdir(infoDir);
if (!savedErrno)
savedErrno = errno;
if (!trashDir.exists(filesDir) || !trashDir.exists(infoDir)) {
error = QSystemError(savedErrno, QSystemError::StandardLibraryError);
return false;
}
op.infoDir = trashDir.filePath(infoDir);
/* /*
"The $trash/files directory contains the files and directories that were trashed. "The $trash/files directory contains the files and directories that were trashed.
@ -1416,7 +1461,7 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source,
filename, and then opening with O_EXCL. If that succeeds the creation was atomic filename, and then opening with O_EXCL. If that succeeds the creation was atomic
(at least on the same machine), if it fails you need to pick another filename." (at least on the same machine), if it fails you need to pick another filename."
*/ */
QString uniqueTrashedName = u'/' + sourcePath.fileName(); QString uniqueTrashedName = sourcePath.fileName();
if (!op.tryCreateInfoFile(uniqueTrashedName, error) && error.errorCode == EEXIST) { if (!op.tryCreateInfoFile(uniqueTrashedName, error) && error.errorCode == EEXIST) {
// we'll use a counter, starting with the file's inode number to avoid // we'll use a counter, starting with the file's inode number to avoid
// collisions // collisions
@ -1441,24 +1486,28 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source,
QByteArray info = QByteArray info =
"[Trash Info]\n" "[Trash Info]\n"
"Path=" + QUrl::toPercentEncoding(sourcePath.filePath().mid(volumePrefixLength), "/") + "\n" "Path=" + QUrl::toPercentEncoding(source.filePath().mid(op.volumePrefixLength), "/") + "\n"
"DeletionDate=" + QDateTime::currentDateTime().toString(Qt::ISODate).toUtf8() "DeletionDate=" + QDateTime::currentDateTime().toString(Qt::ISODate).toUtf8()
+ "\n"; + "\n";
if (QT_WRITE(op.infoFileFd, info.data(), info.size()) < 0) { if (QT_WRITE(op.infoFileFd, info.data(), info.size()) < 0) {
error = QSystemError(errno, QSystemError::StandardLibraryError); error = QSystemError(errno, QSystemError::StandardLibraryError);
return false; return false;
} }
/* /*
We might fail to rename if source and target are on different file systems. We might fail to rename if source and target are on different file systems.
In that case, we don't try further, i.e. copying and removing the original In that case, we don't try further, i.e. copying and removing the original
is usually not what the user would expect to happen. is usually not what the user would expect to happen.
*/ */
QFileSystemEntry target(trashDir.filePath(filesDir) + uniqueTrashedName); bool renamed = renameat(AT_FDCWD, source.nativeFilePath(), op.filesDirFd,
if (!renameFile(source, target, error)) QFile::encodeName(uniqueTrashedName)) == 0;
if (!renamed) {
error = QSystemError(errno, QSystemError::StandardLibraryError);
return false; return false;
}
op.commit(); op.commit();
newLocation = std::move(target); newLocation = QFileSystemEntry(op.trashPath + "/files/"_L1 + uniqueTrashedName);
return true; return true;
} }
#endif // !Q_OS_DARWIN && !QT_BOOTSTRAPPED #endif // !Q_OS_DARWIN && !QT_BOOTSTRAPPED

View File

@ -204,6 +204,16 @@ Q_CORE_EXPORT int qt_open64(const char *pathname, int flags, mode_t);
# endif # endif
#endif #endif
#ifdef AT_FDCWD
static inline int qt_safe_openat(int dfd, const char *pathname, int flags, mode_t mode = 0777)
{
// everyone already has O_CLOEXEC
int fd;
EINTR_LOOP(fd, openat(dfd, pathname, flags | O_CLOEXEC, mode));
return fd;
}
#endif
// don't call QT_OPEN or ::open // don't call QT_OPEN or ::open
// call qt_safe_open // call qt_safe_open
static inline int qt_safe_open(const char *pathname, int flags, mode_t mode = 0777) static inline int qt_safe_open(const char *pathname, int flags, mode_t mode = 0777)

View File

@ -4289,7 +4289,6 @@ void tst_QFile::moveToTrashXdgSafety()
// ditto for our user's subdir now // ditto for our user's subdir now
chmod(QFile::encodeName(genericTrashDir.path()), 01755); chmod(QFile::encodeName(genericTrashDir.path()), 01755);
genericTrashDir.mkdir(QString::number(getuid()), QFile::ReadOwner); genericTrashDir.mkdir(QString::number(getuid()), QFile::ReadOwner);
QEXPECT_FAIL("", "Fall back not working", Continue);
QVERIFY(tryTrashing()); QVERIFY(tryTrashing());
} }
#endif #endif