From 549db7d3318d8c6479d180fc249ea9b0d6e7b27c Mon Sep 17 00:00:00 2001 From: LostDragonist Date: Fri, 21 Dec 2018 17:38:07 -0600 Subject: Escape quotes in arguments when elevating MO --- src/spawn.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/spawn.cpp') diff --git a/src/spawn.cpp b/src/spawn.cpp index f92387d5..6bb34aea 100644 --- a/src/spawn.cpp +++ b/src/spawn.cpp @@ -159,8 +159,9 @@ HANDLE startBinary(const QFileInfo &binary, "(you will be asked if you want to allow ModOrganizer.exe to make changes to the system)").arg( QDir::toNativeSeparators(binary.absoluteFilePath())), QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) { + std::wstring parameters = ToWString("\"" + QDir::toNativeSeparators(binary.absoluteFilePath()) + "\" " + QString(arguments).replace("\"", "\\\"")); ::ShellExecuteW(nullptr, L"runas", ToWString(QCoreApplication::applicationFilePath()).c_str(), - (std::wstring(L"\"") + binaryName + L"\" " + ToWString(arguments)).c_str(), currentDirectoryName.c_str(), SW_SHOWNORMAL); + parameters.c_str(), currentDirectoryName.c_str(), SW_SHOWNORMAL); return INVALID_HANDLE_VALUE; } else { return INVALID_HANDLE_VALUE; -- cgit v1.3.1 From 85eb57eebb21e49d430fd8327b1074dcea2405be Mon Sep 17 00:00:00 2001 From: LostDragonist Date: Tue, 25 Dec 2018 03:51:32 -0600 Subject: Relaunch MO as admin when elevation is required Previously, a new process of MO would be created as administrator when elevation was required. This had some unwanted side effects. The GUI lock would not function. The virtual file system would likely break as the non-admin process could not access data from the new admin instance. --- src/helper.cpp | 34 ++++++++++++++++++++++++++++++---- src/helper.h | 12 ++++++++++-- src/spawn.cpp | 30 +++++++++++++++++++----------- 3 files changed, 59 insertions(+), 17 deletions(-) (limited to 'src/spawn.cpp') diff --git a/src/helper.cpp b/src/helper.cpp index b7fc866c..59a2d3d1 100644 --- a/src/helper.cpp +++ b/src/helper.cpp @@ -26,6 +26,7 @@ along with Mod Organizer. If not, see . #include #include +#include using MOBase::reportError; @@ -33,7 +34,7 @@ using MOBase::reportError; namespace Helper { -static bool helperExec(LPCWSTR moDirectory, LPCWSTR commandLine) +static bool helperExec(LPCWSTR moDirectory, LPCWSTR commandLine, BOOL async) { wchar_t fileName[MAX_PATH]; _snwprintf(fileName, MAX_PATH, L"%ls\\helper.exe", moDirectory); @@ -51,7 +52,16 @@ static bool helperExec(LPCWSTR moDirectory, LPCWSTR commandLine) ::ShellExecuteExW(&execInfo); - if ((execInfo.hProcess == 0) || (::WaitForSingleObject(execInfo.hProcess, INFINITE) != WAIT_OBJECT_0)) { + if (execInfo.hProcess == 0) { + reportError(QObject::tr("helper failed")); + return false; + } + + if (async) { + return true; + } + + if (::WaitForSingleObject(execInfo.hProcess, INFINITE) != WAIT_OBJECT_0) { reportError(QObject::tr("helper failed")); return false; } @@ -76,7 +86,7 @@ bool init(const std::wstring &moPath, const std::wstring &dataPath) _snwprintf(commandLine, 32768, L"init \"%ls\" \"%ls\"", dataPath.c_str(), userName); - bool res = helperExec(moPath.c_str(), commandLine); + bool res = helperExec(moPath.c_str(), commandLine, FALSE); delete [] commandLine; return res; @@ -89,7 +99,23 @@ bool backdateBSAs(const std::wstring &moPath, const std::wstring &dataPath) _snwprintf(commandLine, 32768, L"backdateBSA \"%ls\"", dataPath.c_str()); - bool res = helperExec(moPath.c_str(), commandLine); + bool res = helperExec(moPath.c_str(), commandLine, FALSE); + delete [] commandLine; + + return res; +} + + +bool adminLaunch(const std::wstring &moPath, const std::wstring &moFile, const std::wstring &workingDir) +{ + wchar_t *commandLine = new wchar_t[32768]; + _snwprintf(commandLine, 32768, L"adminLaunch %d \"%ls\" \"%ls\"", + ::GetCurrentProcessId(), + moFile.c_str(), + workingDir.c_str() + ); + + bool res = helperExec(moPath.c_str(), commandLine, TRUE); delete [] commandLine; return res; diff --git a/src/helper.h b/src/helper.h index cd4b7883..f6667a84 100644 --- a/src/helper.h +++ b/src/helper.h @@ -26,7 +26,7 @@ along with Mod Organizer. If not, see . /** * @brief Convenience functions to work with the external helper program. - * + * * The mo_helper program is used to make changes on the system that require administrative * rights, so that ModOrganizer itself can run without special privileges **/ @@ -34,7 +34,7 @@ namespace Helper { /** * @brief initialise the specified directory for use with mod organizer. - * + * * This will create all required sub-directories and give the user running ModOrganizer * write-access * @@ -50,6 +50,14 @@ bool init(const std::wstring &moPath, const std::wstring &dataPath); **/ bool backdateBSAs(const std::wstring &moPath, const std::wstring &dataPath); +/** + * @brief waits for the current process to exit and restarts it as an administrator + * @param moPath absolute path to the modOrganizer base directory + * @param moFile file name of modOrganizer + * @param workingDir current working directory + **/ +bool adminLaunch(const std::wstring &moPath, const std::wstring &moFile, const std::wstring &workingDir); + } diff --git a/src/spawn.cpp b/src/spawn.cpp index 6bb34aea..1459aadb 100644 --- a/src/spawn.cpp +++ b/src/spawn.cpp @@ -26,6 +26,7 @@ along with Mod Organizer. If not, see . #include #include #include +#include "helper.h" #include #include @@ -148,24 +149,31 @@ HANDLE startBinary(const QFileInfo &binary, } } catch (const windows_error &e) { if (e.getErrorCode() == ERROR_ELEVATION_REQUIRED) { - // TODO: check if this is really correct. Are all settings updated that the secondary instance may use? - - if (QMessageBox::question(nullptr, QObject::tr("Elevation required"), + if (QMessageBox::question(QApplication::activeModalWidget(), QObject::tr("Elevation required"), QObject::tr("This process requires elevation to run.\n" "This is a potential security risk so I highly advice you to investigate if\n" "\"%1\"\n" "can be installed to work without elevation.\n\n" - "Start elevated anyway? " - "(you will be asked if you want to allow ModOrganizer.exe to make changes to the system)").arg( + "Restart Mod Organizer as an elevated process?\n" + "You will be asked if you want to allow helper.exe to make changes to the system." + "You will need to relaunch the process above manually.").arg( QDir::toNativeSeparators(binary.absoluteFilePath())), QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) { - std::wstring parameters = ToWString("\"" + QDir::toNativeSeparators(binary.absoluteFilePath()) + "\" " + QString(arguments).replace("\"", "\\\"")); - ::ShellExecuteW(nullptr, L"runas", ToWString(QCoreApplication::applicationFilePath()).c_str(), - parameters.c_str(), currentDirectoryName.c_str(), SW_SHOWNORMAL); - return INVALID_HANDLE_VALUE; - } else { - return INVALID_HANDLE_VALUE; + WCHAR cwd[MAX_PATH]; + if (!GetCurrentDirectory(MAX_PATH, cwd)) { + reportError(QObject::tr("failed to spawn \"%1\": %2").arg(binary.fileName()).arg(::GetLastError())); + cwd[0] = L'\0'; + } + if (!Helper::adminLaunch( + qApp->applicationDirPath().toStdWString(), + qApp->applicationFilePath().toStdWString(), + std::wstring(cwd))) { + return INVALID_HANDLE_VALUE; + } + qApp->exit(0); } + return INVALID_HANDLE_VALUE; + } else { reportError(QObject::tr("failed to spawn \"%1\": %2").arg(binary.fileName()).arg(e.what())); return INVALID_HANDLE_VALUE; -- cgit v1.3.1 From 2d9cf25657f0775431beda75713cba6156f2180b Mon Sep 17 00:00:00 2001 From: LostDragonist Date: Thu, 27 Dec 2018 18:49:19 -0600 Subject: Fix typo --- src/organizer_en.ts | 16 ++++++++-------- src/spawn.cpp | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'src/spawn.cpp') diff --git a/src/organizer_en.ts b/src/organizer_en.ts index ad2d669c..46bb42cf 100644 --- a/src/organizer_en.ts +++ b/src/organizer_en.ts @@ -2088,6 +2088,13 @@ Error: %1 Everything seems to be in order + + + + + Endorse + + Won't Endorse @@ -2815,13 +2822,6 @@ You can also use online editors and converters instead. Un-Endorse - - - - - Endorse - - Won't endorse @@ -5703,7 +5703,7 @@ This is a potential security risk so I highly advice you to investigate if can be installed to work without elevation. Restart Mod Organizer as an elevated process? -You will be asked if you want to allow helper.exe to make changes to the system.You will need to relaunch the process above manually. +You will be asked if you want to allow helper.exe to make changes to the system. You will need to relaunch the process above manually. diff --git a/src/spawn.cpp b/src/spawn.cpp index 1459aadb..f8a7bb5f 100644 --- a/src/spawn.cpp +++ b/src/spawn.cpp @@ -155,7 +155,7 @@ HANDLE startBinary(const QFileInfo &binary, "\"%1\"\n" "can be installed to work without elevation.\n\n" "Restart Mod Organizer as an elevated process?\n" - "You will be asked if you want to allow helper.exe to make changes to the system." + "You will be asked if you want to allow helper.exe to make changes to the system. " "You will need to relaunch the process above manually.").arg( QDir::toNativeSeparators(binary.absoluteFilePath())), QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) { -- cgit v1.3.1 From cc7c3adcad973949ac976c3dc71bb8cf2677056c Mon Sep 17 00:00:00 2001 From: Al Date: Fri, 11 Jan 2019 13:50:51 +0100 Subject: Fix or ignore a number of compiler warnings. --- src/aboutdialog.ui | 10 +++++----- src/overwriteinfodialog.cpp | 2 +- src/pluginlist.cpp | 2 +- src/selfupdater.cpp | 3 +-- src/settings.cpp | 2 +- src/shared/leaktrace.cpp | 13 ++++++++++++- src/spawn.cpp | 2 +- 7 files changed, 22 insertions(+), 12 deletions(-) (limited to 'src/spawn.cpp') diff --git a/src/aboutdialog.ui b/src/aboutdialog.ui index ac7db1fc..86b95c83 100644 --- a/src/aboutdialog.ui +++ b/src/aboutdialog.ui @@ -161,7 +161,7 @@ - + QAbstractItemView::NoSelection @@ -202,7 +202,7 @@ - + QAbstractItemView::NoSelection @@ -231,9 +231,9 @@ Translators - + - + QAbstractItemView::NoSelection @@ -363,7 +363,7 @@ Other Supporters && Contributors - + diff --git a/src/overwriteinfodialog.cpp b/src/overwriteinfodialog.cpp index 1fe3f645..4d35a2c3 100644 --- a/src/overwriteinfodialog.cpp +++ b/src/overwriteinfodialog.cpp @@ -220,7 +220,7 @@ void OverwriteInfoDialog::openFile(const QModelIndex &index) QString fileName = m_FileSystemModel->filePath(index); HINSTANCE res = ::ShellExecuteW(nullptr, L"open", ToWString(fileName).c_str(), nullptr, nullptr, SW_SHOW); - if ((int)res <= 32) { + if ((INT_PTR)res <= 32) { qCritical("failed to invoke %s: %d", fileName.toUtf8().constData(), res); } } diff --git a/src/pluginlist.cpp b/src/pluginlist.cpp index 6f417331..25f9c545 100644 --- a/src/pluginlist.cpp +++ b/src/pluginlist.cpp @@ -153,7 +153,7 @@ void PluginList::highlightPlugins(const QItemSelectionModel *selection, const MO } } } - emit dataChanged(this->index(0, 0), this->index(m_ESPs.size() - 1, this->columnCount() - 1)); + emit dataChanged(this->index(0, 0), this->index(static_cast(m_ESPs.size()) - 1, this->columnCount() - 1)); } void PluginList::refresh(const QString &profileName diff --git a/src/selfupdater.cpp b/src/selfupdater.cpp index c913af6a..b5e7684c 100644 --- a/src/selfupdater.cpp +++ b/src/selfupdater.cpp @@ -338,9 +338,8 @@ void SelfUpdater::installUpdate() } else { reportError(tr("Failed to start %1: %2") .arg(m_UpdateFile.fileName()) - .arg((int)res)); + .arg((INT_PTR)res)); } - m_UpdateFile.remove(); } diff --git a/src/settings.cpp b/src/settings.cpp index f981f811..9267cccf 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -142,7 +142,7 @@ void Settings::registerAsNXMHandler(bool force) } parameters += L" \"" + executable + L"\""; HINSTANCE res = ::ShellExecuteW(nullptr, L"open", nxmPath.c_str(), parameters.c_str(), nullptr, SW_SHOWNORMAL); - if ((int)res <= 32) { + if ((INT_PTR)res <= 32) { QMessageBox::critical(nullptr, tr("Failed"), tr("Sorry, failed to start the helper application")); } diff --git a/src/shared/leaktrace.cpp b/src/shared/leaktrace.cpp index 1ff1fa9d..466162a4 100644 --- a/src/shared/leaktrace.cpp +++ b/src/shared/leaktrace.cpp @@ -1,3 +1,8 @@ +//disable warning messages 4302 , 4311 and 4312 +#pragma warning( disable : 4302 ) +#pragma warning( disable : 4311 ) +#pragma warning( disable : 4312 ) + #include "leaktrace.h" #include "stackdata.h" #include @@ -31,7 +36,7 @@ static struct __TraceData { for (auto iter = result.begin(); iter != result.end(); ++iter) { printf("-----------------------------------\n" "%d objects not freed, allocated at:\n%s", - iter->second.size(), iter->first.toString().c_str()); + static_cast(iter->second.size()), iter->first.toString().c_str()); printf("Addresses: "); for (int i = 0; i < (std::min)(5, static_cast(iter->second.size())); ++i) { @@ -55,3 +60,9 @@ void LeakTrace::TraceDealloc(void *ptr) { __trace.deregTrace(ptr); } + +//re-enable warning messages 4302 , 4311 and 4312 +#pragma warning( default : 4302 ) +#pragma warning( default : 4311 ) +#pragma warning( default : 4312 ) + diff --git a/src/spawn.cpp b/src/spawn.cpp index f8a7bb5f..e1de5c0f 100644 --- a/src/spawn.cpp +++ b/src/spawn.cpp @@ -94,7 +94,7 @@ static bool spawn(LPCWSTR binary, LPCWSTR arguments, LPCWSTR currentDirectory, PROCESS_INFORMATION pi; BOOL success = FALSE; if (hooked) { - qDebug() << "Creating process hooked: <" << QString::fromWCharArray(commandLine, length) <<">"; + qDebug() << "Creating process hooked: <" << QString::fromWCharArray(commandLine, static_cast(length)) <<">"; success = ::CreateProcessHooked(nullptr, commandLine, nullptr, nullptr, // no special process or thread attributes -- cgit v1.3.1