summaryrefslogtreecommitdiffstats
path: root/src/corelib
diff options
context:
space:
mode:
authorJan-Arve Sæther <jan-arve.saether@nokia.com>2010-08-31 08:23:30 (GMT)
committerJan-Arve Sæther <jan-arve.saether@nokia.com>2010-09-03 08:22:25 (GMT)
commit5738dcd705e7edde816940f9c0ab2c364c81ad20 (patch)
treee6be0a4b0f11289a7f0e8f6b4d4cce1de28aee8e /src/corelib
parentdac9e5dd5644d29d6a8dde752e7c594727f16661 (diff)
downloadQt-5738dcd705e7edde816940f9c0ab2c364c81ad20.zip
Qt-5738dcd705e7edde816940f9c0ab2c364c81ad20.tar.gz
Qt-5738dcd705e7edde816940f9c0ab2c364c81ad20.tar.bz2
Ensure that we load system libraries from the correct location.
This was a security hole that has been there for a while, but the public awareness have recently rised so the threat is more imminent now. The solution is to fix all places where we dynamically load system libraries. More specifically, we now load all system libraries with an absolute path that points to a library in the system directory (usually c:\windows\system32). We therefore introduce a small class named QSystemLibrary that only loads libraries located in the system path. This shares some of the API with QLibrary (in order to make the patch as small as possible). We don't fix QLibrary due to risk of regressions. In addition, applications can fix the code that calls QLibrary themselves. The problem does not apply to Windows CE, since the search order is documented as not searching in the current directory. However, it touches some CE-specific code - therefore QSystemLibrary is sometimes used on WinCE (however, it will just do a normal LoadLibrary() since its safe anyway). This change does not affect the testability plugin (it is not clearly documented where that plugin is located, and the plugin should never be used in production code anyway) Loading OpenSSL libraries The ssl libraries are handled specially, and searched in this order (we cannot expect them to always be in the system folder): 1. Application path 2. System libraries path 3. Trying all paths inside the PATH environment variable Task-number: QT-3825 Reviewed-by: Thiago Macieira Reviewed-by: Peter Hartmann
Diffstat (limited to 'src/corelib')
-rw-r--r--src/corelib/io/qfsfileengine_win.cpp7
-rw-r--r--src/corelib/io/qsettings.cpp6
-rw-r--r--src/corelib/kernel/qeventdispatcher_win.cpp10
-rw-r--r--src/corelib/plugin/plugin.pri7
-rw-r--r--src/corelib/plugin/qsystemlibrary.cpp90
-rw-r--r--src/corelib/plugin/qsystemlibrary_p.h61
6 files changed, 168 insertions, 13 deletions
diff --git a/src/corelib/io/qfsfileengine_win.cpp b/src/corelib/io/qfsfileengine_win.cpp
index 139075f..44db59b 100644
--- a/src/corelib/io/qfsfileengine_win.cpp
+++ b/src/corelib/io/qfsfileengine_win.cpp
@@ -43,6 +43,7 @@
#include "qplatformdefs.h"
#include "qabstractfileengine.h"
#include "private/qfsfileengine_p.h"
+#include <private/qsystemlibrary_p.h>
#include <qdebug.h>
#include "qfile.h"
@@ -181,7 +182,7 @@ void QFSFileEnginePrivate::resolveLibs()
triedResolve = true;
#if !defined(Q_OS_WINCE)
- HINSTANCE advapiHnd = LoadLibraryW(L"advapi32");
+ HINSTANCE advapiHnd = QSystemLibrary::load(L"advapi32");
if (advapiHnd) {
ptrGetNamedSecurityInfoW = (PtrGetNamedSecurityInfoW)GetProcAddress(advapiHnd, "GetNamedSecurityInfoW");
ptrLookupAccountSidW = (PtrLookupAccountSidW)GetProcAddress(advapiHnd, "LookupAccountSidW");
@@ -213,7 +214,7 @@ void QFSFileEnginePrivate::resolveLibs()
ptrFreeSid(pWorld);
}
}
- HINSTANCE userenvHnd = LoadLibraryW(L"userenv");
+ HINSTANCE userenvHnd = QSystemLibrary::load(L"userenv");
if (userenvHnd)
ptrGetUserProfileDirectoryW = (PtrGetUserProfileDirectoryW)GetProcAddress(userenvHnd, "GetUserProfileDirectoryW");
#endif
@@ -245,7 +246,7 @@ bool QFSFileEnginePrivate::resolveUNCLibs()
#endif
triedResolve = true;
#if !defined(Q_OS_WINCE)
- HINSTANCE hLib = LoadLibraryW(L"Netapi32");
+ HINSTANCE hLib = QSystemLibrary::load(L"Netapi32");
if (hLib) {
ptrNetShareEnum = (PtrNetShareEnum)GetProcAddress(hLib, "NetShareEnum");
if (ptrNetShareEnum)
diff --git a/src/corelib/io/qsettings.cpp b/src/corelib/io/qsettings.cpp
index 64015ce..c3dece9 100644
--- a/src/corelib/io/qsettings.cpp
+++ b/src/corelib/io/qsettings.cpp
@@ -69,7 +69,7 @@
#ifdef Q_OS_WIN // for homedirpath reading from registry
#include "qt_windows.h"
-#include "qlibrary.h"
+#include <private/qsystemlibrary_p.h>
#endif // Q_OS_WIN
#endif // QT_NO_QOBJECT
@@ -1046,9 +1046,9 @@ static QString windowsConfigPath(int type)
// We can't use QLibrary if there is QT_NO_QOBJECT is defined
// This only happens when bootstrapping qmake.
#ifndef Q_OS_WINCE
- QLibrary library(QLatin1String("shell32"));
+ QSystemLibrary library(QLatin1String("shell32"));
#else
- QLibrary library(QLatin1String("coredll"));
+ QSystemLibrary library(QLatin1String("coredll"));
#endif // Q_OS_WINCE
typedef BOOL (WINAPI*GetSpecialFolderPath)(HWND, LPWSTR, int, BOOL);
GetSpecialFolderPath SHGetSpecialFolderPath = (GetSpecialFolderPath)library.resolve("SHGetSpecialFolderPathW");
diff --git a/src/corelib/kernel/qeventdispatcher_win.cpp b/src/corelib/kernel/qeventdispatcher_win.cpp
index f63fa1d..2da02f9 100644
--- a/src/corelib/kernel/qeventdispatcher_win.cpp
+++ b/src/corelib/kernel/qeventdispatcher_win.cpp
@@ -43,7 +43,7 @@
#include "qcoreapplication.h"
#include "qhash.h"
-#include "qlibrary.h"
+#include <private/qsystemlibrary_p.h>
#include "qpair.h"
#include "qset.h"
#include "qsocketnotifier.h"
@@ -324,11 +324,11 @@ static void resolveTimerAPI()
#endif
triedResolve = true;
#if !defined(Q_OS_WINCE)
- qtimeSetEvent = (ptimeSetEvent)QLibrary::resolve(QLatin1String("winmm"), "timeSetEvent");
- qtimeKillEvent = (ptimeKillEvent)QLibrary::resolve(QLatin1String("winmm"), "timeKillEvent");
+ qtimeSetEvent = (ptimeSetEvent)QSystemLibrary::resolve(QLatin1String("winmm"), "timeSetEvent");
+ qtimeKillEvent = (ptimeKillEvent)QSystemLibrary::resolve(QLatin1String("winmm"), "timeKillEvent");
#else
- qtimeSetEvent = (ptimeSetEvent)QLibrary::resolve(QLatin1String("Mmtimer"), "timeSetEvent");
- qtimeKillEvent = (ptimeKillEvent)QLibrary::resolve(QLatin1String("Mmtimer"), "timeKillEvent");
+ qtimeSetEvent = (ptimeSetEvent)QSystemLibrary::resolve(QLatin1String("Mmtimer"), "timeSetEvent");
+ qtimeKillEvent = (ptimeKillEvent)QSystemLibrary::resolve(QLatin1String("Mmtimer"), "timeKillEvent");
#endif
}
}
diff --git a/src/corelib/plugin/plugin.pri b/src/corelib/plugin/plugin.pri
index c05ff48..ba86353 100644
--- a/src/corelib/plugin/plugin.pri
+++ b/src/corelib/plugin/plugin.pri
@@ -7,7 +7,8 @@ HEADERS += \
plugin/qlibrary_p.h \
plugin/qplugin.h \
plugin/quuid.h \
- plugin/qfactoryloader_p.h
+ plugin/qfactoryloader_p.h \
+ plugin/qsystemlibrary_p.h
SOURCES += \
plugin/qpluginloader.cpp \
@@ -16,7 +17,9 @@ SOURCES += \
plugin/qlibrary.cpp
win32 {
- SOURCES += plugin/qlibrary_win.cpp
+ SOURCES += \
+ plugin/qlibrary_win.cpp \
+ plugin/qsystemlibrary.cpp
}
unix {
diff --git a/src/corelib/plugin/qsystemlibrary.cpp b/src/corelib/plugin/qsystemlibrary.cpp
new file mode 100644
index 0000000..7e9fdde
--- /dev/null
+++ b/src/corelib/plugin/qsystemlibrary.cpp
@@ -0,0 +1,90 @@
+#include "qsystemlibrary_p.h"
+#include <QtCore/qvarlengtharray.h>
+#include <QtCore/qstringlist.h>
+#include <QtCore/qfileinfo.h>
+
+/*!
+
+ \internal
+ \class QSystemLibrary
+
+ The purpose of this class is to load only libraries that are located in
+ well-known and trusted locations on the filesystem. It does not suffer from
+ the security problem that QLibrary has, therefore it will never search in
+ the current directory.
+
+ The search order is the same as the order in DLL Safe search mode Windows,
+ except that we don't search:
+ * The current directory
+ * The 16-bit system directory. (normally c:\windows\system)
+ * The Windows directory. (normally c:\windows)
+
+ This means that the effective search order is:
+ 1. Application path.
+ 2. System libraries path.
+ 3. Trying all paths inside the PATH environment variable.
+
+ Note, when onlySystemDirectory is true it will skip 1) and 3).
+
+ DLL Safe search mode is documented in the "Dynamic-Link Library Search
+ Order" document on MSDN.
+
+ Since library loading code is sometimes shared between Windows and WinCE,
+ this class can also be used on WinCE. However, its implementation just
+ calls the LoadLibrary() function. This is ok since it is documented as not
+ loading from the current directory on WinCE. This behaviour is documented
+ in the documentation for LoadLibrary for Windows CE at MSDN.
+ (http://msdn.microsoft.com/en-us/library/ms886736.aspx)
+*/
+#if !defined(QT_BOOTSTRAPPED)
+extern QString qAppFileName();
+#endif
+
+static QString qSystemDirectory()
+{
+ QVarLengthArray<wchar_t, MAX_PATH> fullPath;
+
+ UINT retLen = ::GetSystemDirectory(fullPath.data(), MAX_PATH);
+ if (retLen > MAX_PATH) {
+ fullPath.resize(retLen);
+ retLen = ::GetSystemDirectory(fullPath.data(), retLen);
+ }
+ // in some rare cases retLen might be 0
+ return QString::fromWCharArray(fullPath.constData(), int(retLen));
+}
+
+HINSTANCE QSystemLibrary::load(const wchar_t *libraryName, bool onlySystemDirectory/*= true*/)
+{
+#if defined(Q_OS_WINCE)
+ return ::LoadLibrary(lpFileName);
+#else
+ QStringList searchOrder;
+
+#if !defined(QT_BOOTSTRAPPED)
+ if (!onlySystemDirectory)
+ searchOrder << QFileInfo(qAppFileName()).path();
+#endif
+ searchOrder << qSystemDirectory();
+
+ if (!onlySystemDirectory) {
+ const QString PATH(QLatin1String(qgetenv("PATH").constData()));
+ searchOrder << PATH.split(QLatin1Char(';'), QString::SkipEmptyParts);
+ }
+ QString fileName = QString::fromWCharArray(libraryName);
+ fileName.append(QLatin1String(".dll"));
+
+ // Start looking in the order specified
+ for (int i = 0; i < searchOrder.count(); ++i) {
+ QString fullPathAttempt = searchOrder.at(i);
+ if (!fullPathAttempt.endsWith(QLatin1Char('\\'))) {
+ fullPathAttempt.append(QLatin1Char('\\'));
+ }
+ fullPathAttempt.append(fileName);
+ HINSTANCE inst = ::LoadLibrary((const wchar_t *)fullPathAttempt.utf16());
+ if (inst != 0)
+ return inst;
+ }
+ return 0;
+#endif
+}
+
diff --git a/src/corelib/plugin/qsystemlibrary_p.h b/src/corelib/plugin/qsystemlibrary_p.h
new file mode 100644
index 0000000..60c59e2
--- /dev/null
+++ b/src/corelib/plugin/qsystemlibrary_p.h
@@ -0,0 +1,61 @@
+#ifndef QSYSTEMLIBRARY_P_H
+#define QSYSTEMLIBRARY_P_H
+
+#include <QtCore/qglobal.h>
+#ifdef Q_OS_WIN
+#include <qt_windows.h>
+#include <QtCore/qstring.h>
+
+class QSystemLibrary
+{
+public:
+ explicit QSystemLibrary(const QString &libraryName)
+ {
+ m_libraryName = libraryName;
+ m_handle = 0;
+ m_didLoad = false;
+ }
+
+ explicit QSystemLibrary(const wchar_t *libraryName)
+ {
+ m_libraryName = QString::fromWCharArray(libraryName);
+ m_handle = 0;
+ m_didLoad = false;
+ }
+
+ bool load(bool onlySystemDirectory = true)
+ {
+ m_handle = load((const wchar_t *)m_libraryName.utf16(), onlySystemDirectory);
+ m_didLoad = true;
+ return (m_handle != 0);
+ }
+
+ bool isLoaded()
+ {
+ return (m_handle != 0);
+ }
+
+ void *resolve(const char *symbol)
+ {
+ if (!m_didLoad)
+ load();
+ if (!m_handle)
+ return 0;
+ return (void*)GetProcAddress(m_handle, symbol);
+ }
+
+ static void *resolve(const QString &libraryName, const char *symbol)
+ {
+ return QSystemLibrary(libraryName).resolve(symbol);
+ }
+
+ static Q_CORE_EXPORT HINSTANCE load(const wchar_t *lpFileName, bool onlySystemDirectory = true);
+private:
+ HINSTANCE m_handle;
+ QString m_libraryName;
+ bool m_didLoad;
+};
+
+#endif //Q_OS_WIN
+
+#endif //QSYSTEMLIBRARY_P_H