From 5d92fc9c28c2f2a326af8a5aa87069774a34a0af Mon Sep 17 00:00:00 2001 From: Sven Strickroth Date: Sat, 13 Aug 2016 15:48:42 +0200 Subject: [PATCH] Don't pass read-only buffer to CreateProcess as lpCommandLine See https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425. Signed-off-by: Sven Strickroth --- src/Git/Git.cpp | 2 +- src/TortoiseMerge/AppUtils.cpp | 2 +- src/TortoiseShell/ContextMenu.cpp | 2 +- src/TortoiseShell/GITPropertyPage.cpp | 2 +- src/TortoiseShell/RemoteCacheLink.cpp | 2 +- src/TortoiseUDiff/MainWindow.cpp | 4 ++-- src/Utils/CommonAppUtils.cpp | 2 +- src/Utils/CreateProcessHelper.h | 19 +++++++++++++++++++ 8 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/Git/Git.cpp b/src/Git/Git.cpp index fb4058bfc..3f18606c5 100644 --- a/src/Git/Git.cpp +++ b/src/Git/Git.cpp @@ -1212,7 +1212,7 @@ int CGit::RunLogFile(CString cmd, const CString &filename, CString *stdErr) } CTraceToOutputDebugString::Instance()(_T(__FUNCTION__) _T(": executing %s\n"), (LPCTSTR)cmd); - if (!CreateProcess(nullptr, (LPWSTR)cmd.GetString(), nullptr, nullptr, TRUE, dwFlags, pEnv, (LPWSTR)m_CurrentDir.GetString(), &si, &pi)) + if (!CreateProcess(nullptr, cmd.GetBuffer(), nullptr, nullptr, TRUE, dwFlags, pEnv, m_CurrentDir.GetBuffer(), &si, &pi)) { CString err = CFormatMessageWrapper(); CTraceToOutputDebugString::Instance()(_T(__FUNCTION__) _T(": failed to create Process: %s\n"), (LPCTSTR)err.Trim()); diff --git a/src/TortoiseMerge/AppUtils.cpp b/src/TortoiseMerge/AppUtils.cpp index 3cbd3d9d9..1661a8910 100644 --- a/src/TortoiseMerge/AppUtils.cpp +++ b/src/TortoiseMerge/AppUtils.cpp @@ -61,7 +61,7 @@ BOOL CAppUtils::GetVersionedFile(CString sPath, CString sVersion, CString sSaveP sSCMPath.Replace(_T("%4"), sTemp); // start the external SCM program to fetch the specific version of the file PROCESS_INFORMATION process; - if (!CCreateProcessHelper::CreateProcess(nullptr, (LPTSTR)(LPCTSTR)sSCMPath, &process)) + if (!CCreateProcessHelper::CreateProcess(nullptr, sSCMPath.GetBuffer(), &process)) { CFormatMessageWrapper errorDetails; MessageBox(nullptr, errorDetails, _T("TortoiseGitMerge"), MB_OK | MB_ICONERROR); diff --git a/src/TortoiseShell/ContextMenu.cpp b/src/TortoiseShell/ContextMenu.cpp index d9b1612a6..57984d156 100644 --- a/src/TortoiseShell/ContextMenu.cpp +++ b/src/TortoiseShell/ContextMenu.cpp @@ -2218,7 +2218,7 @@ bool CShellExt::InsertIgnoreSubmenus(UINT &idCmd, UINT idCmdFirst, HMENU hMenu, void CShellExt::RunCommand(const tstring& path, const tstring& command, LPCTSTR errorMessage) { - if (CCreateProcessHelper::CreateProcessDetached(path.c_str(), const_cast(command.c_str()))) + if (CCreateProcessHelper::CreateProcessDetached(path.c_str(), command.c_str())) { // process started - exit return; diff --git a/src/TortoiseShell/GITPropertyPage.cpp b/src/TortoiseShell/GITPropertyPage.cpp index b00b16dbc..a633f6b7a 100644 --- a/src/TortoiseShell/GITPropertyPage.cpp +++ b/src/TortoiseShell/GITPropertyPage.cpp @@ -294,7 +294,7 @@ void CGitPropertyPage::PageProcOnCommand(WPARAM wParam) void CGitPropertyPage::RunCommand(const tstring& command) { tstring tortoiseProcPath = CPathUtils::GetAppDirectory(g_hmodThisDll) + _T("TortoiseGitProc.exe"); - if (CCreateProcessHelper::CreateProcessDetached(tortoiseProcPath.c_str(), const_cast(command.c_str()))) + if (CCreateProcessHelper::CreateProcessDetached(tortoiseProcPath.c_str(), command.c_str())) { // process started - exit return; diff --git a/src/TortoiseShell/RemoteCacheLink.cpp b/src/TortoiseShell/RemoteCacheLink.cpp index 3be9c70b6..69ca4df29 100644 --- a/src/TortoiseShell/RemoteCacheLink.cpp +++ b/src/TortoiseShell/RemoteCacheLink.cpp @@ -305,7 +305,7 @@ DWORD CRemoteCacheLink::GetProcessIntegrityLevel() const bool CRemoteCacheLink::RunTGitCacheProcess() { const CString sCachePath = GetTGitCachePath(); - if (!CCreateProcessHelper::CreateProcessDetached(sCachePath, nullptr)) + if (!CCreateProcessHelper::CreateProcessDetached(sCachePath, (LPTSTR)nullptr)) { // It's not appropriate to do a message box here, because there may be hundreds of calls CTraceToOutputDebugString::Instance()(__FUNCTION__ ": Failed to start cache\n"); diff --git a/src/TortoiseUDiff/MainWindow.cpp b/src/TortoiseUDiff/MainWindow.cpp index ec8d72e6b..465948ad2 100644 --- a/src/TortoiseUDiff/MainWindow.cpp +++ b/src/TortoiseUDiff/MainWindow.cpp @@ -263,7 +263,7 @@ LRESULT CMainWindow::DoCommand(int id) command += m_filename; command += L"\""; std::wstring tortoiseMergePath = GetAppDirectory() + _T("TortoiseGitMerge.exe"); - CCreateProcessHelper::CreateProcessDetached(tortoiseMergePath.c_str(), const_cast(command.c_str())); + CCreateProcessHelper::CreateProcessDetached(tortoiseMergePath.c_str(), command.c_str()); } break; case ID_FILE_PAGESETUP: @@ -538,7 +538,7 @@ std::wstring CMainWindow::GetAppDirectory() void CMainWindow::RunCommand(const std::wstring& command) { tstring tortoiseProcPath = GetAppDirectory() + _T("TortoiseGitProc.exe"); - CCreateProcessHelper::CreateProcessDetached(tortoiseProcPath.c_str(), const_cast(command.c_str())); + CCreateProcessHelper::CreateProcessDetached(tortoiseProcPath.c_str(), command.c_str()); } LRESULT CMainWindow::SendEditor(UINT Msg, WPARAM wParam, LPARAM lParam) diff --git a/src/Utils/CommonAppUtils.cpp b/src/Utils/CommonAppUtils.cpp index 0b76b8dbb..8891dba2e 100644 --- a/src/Utils/CommonAppUtils.cpp +++ b/src/Utils/CommonAppUtils.cpp @@ -104,7 +104,7 @@ bool CCommonAppUtils::LaunchApplication(const CString& sCommandLine, UINT idErrM startup.cb = sizeof(startup); CString cleanCommandLine(sCommandLine); - if (CreateProcess(nullptr, const_cast((LPCTSTR)cleanCommandLine), nullptr, nullptr, FALSE, CREATE_UNICODE_ENVIRONMENT, nullptr, theCWD, &startup, &process) == 0) + if (CreateProcess(nullptr, cleanCommandLine.GetBuffer(), nullptr, nullptr, FALSE, CREATE_UNICODE_ENVIRONMENT, nullptr, theCWD, &startup, &process) == 0) { if (idErrMessageFormat) { diff --git a/src/Utils/CreateProcessHelper.h b/src/Utils/CreateProcessHelper.h index e50ff6a56..152531c95 100644 --- a/src/Utils/CreateProcessHelper.h +++ b/src/Utils/CreateProcessHelper.h @@ -40,6 +40,7 @@ public: LPCTSTR lpCurrentDirectory); static bool CreateProcessDetached(LPCTSTR lpApplicationName, LPTSTR lpCommandLine); + static bool CreateProcessDetached(LPCTSTR lpApplicationName, LPCTSTR lpCommandLine); }; inline bool CCreateProcessHelper::CreateProcess(LPCTSTR applicationName, @@ -77,3 +78,21 @@ inline bool CCreateProcessHelper::CreateProcessDetached(LPCTSTR lpApplicationNam { return CreateProcessDetached(lpApplicationName, lpCommandLine, 0); } + +inline bool CCreateProcessHelper::CreateProcessDetached(LPCTSTR lpApplicationName, LPCTSTR lpCommandLine) +{ + // CreateProcess may modify buffer specified by lpCommandLine: + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425 + // [[ + // The Unicode version of this function, CreateProcessW, can modify + // the contents of this string. Therefore, this parameter cannot be a + // pointer to read - only memory(such as a const variable or a literal + // string).If this parameter is a constant string, the function may + // cause an access violation. + // ]] + LPWSTR commandLineBuf = nullptr; + if (lpCommandLine) + commandLineBuf = _tcsdup(lpCommandLine); + + return CreateProcessDetached(lpApplicationName, commandLineBuf, 0); +} -- 2.11.4.GIT