1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

moveToTrash/Unix: avoid TOCTOU in creating the unique file name

Browse Source 
This is not a security issue because we still use QIODevice::NewOnly
(O_EXCL) and loop again. But because we do so, we don't need to check
for existence with QFile::exists() in the first place.

Change-Id: I9d43e5b91eb142d6945cfffd1786c98a39781517
(cherry picked from commit de24134aa7795a05ba271a751bf76792f9a3ff98)
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
This commit is contained in:
Thiago Macieira 2023-09-20 19:38:25 -07:00 committed by Ahmad Samir
parent 4aba04d2ca
commit 28064908e0
1 changed files with 6 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/corelib/io/qfilesystemengine_unix.cpp
View File

@ -1329,14 +1329,9 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source,
*/ */
QString uniqueTrashedName = u'/' + sourcePath.fileName(); QString uniqueTrashedName = u'/' + sourcePath.fileName();
QString infoFileName; QString infoFileName;
int counter = 0;
QFile infoFile; QFile infoFile;
auto makeUniqueTrashedName = [sourcePath, &counter]() -> QString { auto openMode = QIODevice::NewOnly | QIODevice::WriteOnly | QIODevice::Text;
return QString::asprintf("/%ls-%04d", qUtf16Printable(sourcePath.fileName()), ++counter); for (int counter = 0; !infoFile.open(openMode); ++counter) {
};
do {
while (QFile::exists(trashDir.filePath(filesDir) + uniqueTrashedName))
uniqueTrashedName = makeUniqueTrashedName();
/* /*
"The $trash/info directory contains an "information file" for every file and directory "The $trash/info directory contains an "information file" for every file and directory
in $trash/files. This file MUST have exactly the same name as the file or directory in in $trash/files. This file MUST have exactly the same name as the file or directory in
@ -1349,12 +1344,12 @@ 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."
*/ */
infoFileName = trashDir.filePath(infoDir) uniqueTrashedName = QString::asprintf("/%ls-%04d", qUtf16Printable(sourcePath.fileName()),
counter);
QString infoFileName = trashDir.filePath(infoDir)
+ uniqueTrashedName + ".trashinfo"_L1; + uniqueTrashedName + ".trashinfo"_L1;
infoFile.setFileName(infoFileName); infoFile.setFileName(infoFileName);
if (!infoFile.open(QIODevice::NewOnly | QIODevice::WriteOnly | QIODevice::Text)) }
uniqueTrashedName = makeUniqueTrashedName();
} while (!infoFile.isOpen());
QString pathForInfo = sourcePath.filePath(); QString pathForInfo = sourcePath.filePath();
const QStorageInfo storageInfo(pathForInfo); const QStorageInfo storageInfo(pathForInfo);