summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoão Abecasis <joao.abecasis@nokia.com>2011-08-05 10:45:08 +0200
committerQt by Nokia <qt-info@nokia.com>2011-10-21 22:20:18 +0200
commitf4f729cb7bccd8bc4812dd15855dfc5af4ff6c8c (patch)
tree681f214f3429836cca66905bdb58e4949e953f45
parenta4a09488ee979849f4e9b9b3b8590ad2a2472590 (diff)
Atomic implementation of create file and obtain handle for Win/Symbian
Besides generating a unique name, createFileFromTemplate now also acquires a file handle on all platforms. The file engine's native handle is passed by reference and modified in place. This fixes a long standing security issue on Windows. On Windows and Symbian platforms we directly use the "native" file path when processing the template and generating the unique name. Since the native encoding is known, conversions at this point are safe. Errors other than "file exists" are propagated to Q(Temporary)File, and result in a failure in open(). The changes also unify error handling and should give consistent behaviour across all platforms. Worthy of note, there's a change in behaviour on Windows and Symbian: fileNames returned by QTemporaryFile on Windows and Symbian are always absolute after open has been called. This has to do with how QFileSystemEntry::nativeFilePath works on these platforms. (Test was updated to reflect change in behaviour.) Reviewed-by: Gareth Stockwell Reviewed-by: Shane Kearns (cherry picked from commit ff9b69838ec146aeb43d4af8a03043f9c5f0454d) Conflicts: tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp Change-Id: Ibc9affb321ea4f4b193efc1f7336c9770b43d8df Reviewed-by: João Abecasis <joao.abecasis@nokia.com>
-rw-r--r--src/corelib/io/qtemporaryfile.cpp151
-rw-r--r--tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp31
2 files changed, 122 insertions, 60 deletions
diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp
index 79f7a108d1..a0b6c6bbfa 100644
--- a/src/corelib/io/qtemporaryfile.cpp
+++ b/src/corelib/io/qtemporaryfile.cpp
@@ -49,6 +49,11 @@
#include "private/qfile_p.h"
#include "private/qabstractfileengine_p.h"
#include "private/qfsfileengine_p.h"
+#include "private/qsystemerror_p.h"
+
+#if defined(Q_OS_SYMBIAN)
+#include "private/qcore_symbian_p.h"
+#endif
#if !defined(Q_OS_WIN) && !defined(Q_OS_SYMBIAN)
#include "private/qcore_unix_p.h" // overrides QT_OPEN
@@ -83,9 +88,20 @@ struct QConcatenable<Char>
}
};
+# ifdef Q_OS_WIN
+typedef HANDLE NativeFileHandle;
+# else // Q_OS_SYMBIAN
+# ifdef SYMBIAN_ENABLE_64_BIT_FILE_SERVER_API
+typedef RFile64 NativeFileHandle;
+# else
+typedef RFile NativeFileHandle;
+# endif
+# endif
+
#else // POSIX
typedef char Char;
typedef char Latin1Char;
+typedef int NativeFileHandle;
#endif
struct Placeholder
@@ -161,8 +177,9 @@ struct QConcatenable<Placeholder>
handle otherwise. In both cases, the string in \a path will be changed and
contain the generated path name.
*/
-static int createFileFromTemplate(QFileSystemEntry::NativePath &path,
- size_t pos, size_t length)
+static bool createFileFromTemplate(NativeFileHandle &file,
+ QFileSystemEntry::NativePath &path, size_t pos, size_t length,
+ QSystemError &error)
{
Q_ASSERT(length != 0);
Q_ASSERT(pos < size_t(path.length()));
@@ -192,21 +209,50 @@ static int createFileFromTemplate(QFileSystemEntry::NativePath &path,
}
}
+#ifdef Q_OS_SYMBIAN
+ RFs& fs = qt_s60GetRFs();
+#endif
+
for (;;) {
// Atomically create file and obtain handle
-#if !defined(Q_OS_WIN) && !defined(Q_OS_SYMBIAN)
- {
- int fd = QT_OPEN(path.constData(),
- QT_OPEN_CREAT | O_EXCL | QT_OPEN_RDWR | QT_OPEN_LARGEFILE,
- 0600);
- if (fd != -1)
- return fd;
- if (errno != EEXIST)
- return -1;
+#if defined(Q_OS_WIN)
+ file = CreateFile((const wchar_t *)path.constData(),
+ GENERIC_READ | GENERIC_WRITE,
+ FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, CREATE_NEW,
+ FILE_ATTRIBUTE_NORMAL, NULL);
+
+ if (file != INVALID_HANDLE_VALUE)
+ return true;
+
+ DWORD err = GetLastError();
+ if (err != ERROR_FILE_EXISTS) {
+ error = QSystemError(err, QSystemError::NativeError);
+ return false;
+ }
+#elif defined(Q_OS_SYMBIAN)
+ TInt err = file.Create(fs, qt_QString2TPtrC(path),
+ EFileRead | EFileWrite | EFileShareReadersOrWriters);
+
+ if (err == KErrNone)
+ return true;
+
+ if (err != KErrAlreadyExists) {
+ error = QSystemError(err, QSystemError::NativeError);
+ return false;
+ }
+#else // POSIX
+ file = QT_OPEN(path.constData(),
+ QT_OPEN_CREAT | O_EXCL | QT_OPEN_RDWR | QT_OPEN_LARGEFILE,
+ 0600);
+
+ if (file != -1)
+ return true;
+
+ int err = errno;
+ if (err != EEXIST) {
+ error = QSystemError(err, QSystemError::NativeError);
+ return false;
}
-#else
- if (!QFileInfo(path).exists())
- return 1;
#endif
/* tricky little algorwwithm for backward compatibility */
@@ -217,8 +263,11 @@ static int createFileFromTemplate(QFileSystemEntry::NativePath &path,
case 'Z':
// Rollover, advance next character
*iter = Latin1Char('a');
- if (++iter == placeholderEnd)
- return -1;
+ if (++iter == placeholderEnd) {
+ // Out of alternatives. Return file exists error, previously set.
+ error = QSystemError(err, QSystemError::NativeError);
+ return false;
+ }
continue;
@@ -319,7 +368,14 @@ bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode)
if (!filePathIsTemplate)
return QFSFileEngine::open(openMode);
- const QString qfilename = d->fileEntry.filePath();
+ const QString qfilename =
+#if !defined(Q_OS_WIN) && !defined(Q_OS_SYMBIAN)
+ // Since the native encoding is out of our control, we need to process
+ // the path as UTF-16 before doing any conversions
+ d->fileEntry.filePath();
+#else
+ d->fileEntry.nativeFilePath();
+#endif
// Find placeholder string.
uint phPos = qfilename.length();
@@ -333,8 +389,14 @@ bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode)
continue;
}
- if (qfilename[phPos] == QLatin1Char('/')
- || phLength >= 6) {
+ if (phLength >= 6
+ || qfilename[phPos] ==
+#if !defined(Q_OS_WIN) && !defined(Q_OS_SYMBIAN)
+ QLatin1Char('/')
+#else
+ QLatin1Char('\\')
+#endif
+ ) {
++phPos;
break;
}
@@ -366,44 +428,35 @@ bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode)
filename = qfilename % Latin1Char('.') % Placeholder(phLength);
} else
filename = qfilename;
-
- // No native separators, not a "native path"
#endif
- int fd = createFileFromTemplate(filename, phPos, phLength);
+ QSystemError error;
+#if defined(Q_OS_WIN)
+ NativeFileHandle &file = d->fileHandle;
+#elif defined(Q_OS_SYMBIAN)
+ NativeFileHandle &file = d->symbianFile;
+#else // POSIX
+ NativeFileHandle &file = d->fd;
+#endif
-#if !defined(Q_OS_WIN) && !defined(Q_OS_SYMBIAN)
- if (fd != -1) {
- // First open the fd as an external file descriptor to
- // initialize the engine properly.
- if (QFSFileEngine::open(openMode, fd)) {
+ if (!createFileFromTemplate(file, filename, phPos, phLength, error)) {
+ setError(QFile::OpenError, error.toString());
+ return false;
+ }
- // Allow the engine to close the handle even if it's "external".
- d->closeFileHandle = true;
+ d->fileEntry = QFileSystemEntry(filename, QFileSystemEntry::FromNativePath());
- // Restore the file names (open() resets them).
- d->fileEntry = QFileSystemEntry(QString::fromLocal8Bit(filename.constData(), filename.length())); //note that filename is NOT a native path
- filePathIsTemplate = false;
- return true;
- }
+#if !defined(Q_OS_WIN)
+ d->closeFileHandle = true;
+#endif
- QT_CLOSE(fd);
- }
- setError(errno == EMFILE ? QFile::ResourceError : QFile::OpenError, qt_error_string(errno));
- return false;
-#else
- if (fd == -1)
- return false;
+ filePathIsTemplate = false;
- d->fileEntry = QFileSystemEntry(filename, QFileSystemEntry::FromInternalPath());
- if (QFSFileEngine::open(openMode)) {
- filePathIsTemplate = false;
- return true;
- }
+ d->openMode = openMode;
+ d->lastFlushFailed = false;
+ d->tried_stat = 0;
- d->fileEntry = QFileSystemEntry(qfilename, QFileSystemEntry::FromInternalPath());
- return false;
-#endif
+ return true;
}
bool QTemporaryFileEngine::remove()
diff --git a/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp b/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp
index 916393eee7..655c167c21 100644
--- a/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp
+++ b/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp
@@ -199,11 +199,12 @@ void tst_QTemporaryFile::fileTemplate()
QCOMPARE(file.open(), true);
+ QString fileName = QFileInfo(file).fileName();
if (prefix.length())
- QCOMPARE(file.fileName().left(prefix.length()), prefix);
+ QCOMPARE(fileName.left(prefix.length()), prefix);
if (suffix.length())
- QCOMPARE(file.fileName().right(suffix.length()), suffix);
+ QCOMPARE(fileName.right(suffix.length()), suffix);
}
@@ -716,20 +717,28 @@ void tst_QTemporaryFile::QTBUG_4796()
<< file5.fileName()
<< file6.fileName();
- QVERIFY(file1.fileName().startsWith(fileTemplate1 + QLatin1Char('.')));
- QVERIFY(file2.fileName().startsWith(fileTemplate2 + QLatin1Char('.')));
- QVERIFY(file5.fileName().startsWith("test-XXXXXX/" + fileTemplate1 + QLatin1Char('.')));
- QVERIFY(file6.fileName().startsWith("test-XXXXXX/" + prefix));
+ QDir currentDir;
+ QString fileName1 = currentDir.relativeFilePath(file1.fileName());
+ QString fileName2 = currentDir.relativeFilePath(file2.fileName());
+ QString fileName3 = currentDir.relativeFilePath(file3.fileName());
+ QString fileName4 = currentDir.relativeFilePath(file4.fileName());
+ QString fileName5 = currentDir.relativeFilePath(file5.fileName());
+ QString fileName6 = currentDir.relativeFilePath(file6.fileName());
+
+ QVERIFY(fileName1.startsWith(fileTemplate1 + QLatin1Char('.')));
+ QVERIFY(fileName2.startsWith(fileTemplate2 + QLatin1Char('.')));
+ QVERIFY(fileName5.startsWith("test-XXXXXX/" + fileTemplate1 + QLatin1Char('.')));
+ QVERIFY(fileName6.startsWith("test-XXXXXX/" + prefix));
if (!prefix.isEmpty()) {
- QVERIFY(file3.fileName().startsWith(prefix));
- QVERIFY(file4.fileName().startsWith(prefix));
+ QVERIFY(fileName3.startsWith(prefix));
+ QVERIFY(fileName4.startsWith(prefix));
}
if (!suffix.isEmpty()) {
- QVERIFY(file3.fileName().endsWith(suffix));
- QVERIFY(file4.fileName().endsWith(suffix));
- QVERIFY(file6.fileName().endsWith(suffix));
+ QVERIFY(fileName3.endsWith(suffix));
+ QVERIFY(fileName4.endsWith(suffix));
+ QVERIFY(fileName6.endsWith(suffix));
}
}
}