Don't pass const strings to CreateProcess.

The documentation says that CreateProcess() can modify its second parameter in UNICODE builds. BUG=396705 R=scottmg@chromium.org, vitalybuka@chromium.org TBR=cpu Review URL: https://codereview.chromium.org/487303004 Cr-Commit-Position: refs/heads/master@{#290890} git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290890 0039d316-1c4b-4281-b951-d872f2087c98
SaschaDeWaal · Aug 20, 2014 · d4d772e · d4d772e
1 parent 069c7b1
commit d4d772e
Show file tree

Hide file tree

Showing 10 changed files with 32 additions and 22 deletions.
diff --git a/base/process/launch_win.cc b/base/process/launch_win.cc
@@ -177,6 +177,7 @@ bool LaunchProcess(const string16& cmdline,
 
   PROCESS_INFORMATION temp_process_info = {};
 
+  string16 writable_cmdline(cmdline);
   if (options.as_user) {
     flags |= CREATE_UNICODE_ENVIRONMENT;
     void* enviroment_block = NULL;
@@ -188,7 +189,7 @@ bool LaunchProcess(const string16& cmdline,
 
     BOOL launched =
         CreateProcessAsUser(options.as_user, NULL,
-                            const_cast<wchar_t*>(cmdline.c_str()),
+                            &writable_cmdline[0],
                             NULL, NULL, inherit_handles, flags,
                             enviroment_block, NULL, startup_info,
                             &temp_process_info);
@@ -200,7 +201,7 @@ bool LaunchProcess(const string16& cmdline,
     }
   } else {
     if (!CreateProcess(NULL,
-                       const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL,
+                       &writable_cmdline[0], NULL, NULL,
                        inherit_handles, flags, NULL, NULL,
                        startup_info, &temp_process_info)) {
       DPLOG(ERROR) << "Command line:" << std::endl << UTF16ToUTF8(cmdline)

diff --git a/base/win/scoped_process_information_unittest.cc b/base/win/scoped_process_information_unittest.cc
@@ -50,8 +50,7 @@ void ScopedProcessInformationTest::DoCreateProcess(
   STARTUPINFO startup_info = {};
   startup_info.cb = sizeof(startup_info);
 
-  EXPECT_TRUE(::CreateProcess(NULL,
-                              const_cast<wchar_t*>(cmd_line.c_str()),
+  EXPECT_TRUE(::CreateProcess(NULL, &cmd_line[0],
                               NULL, NULL, false, 0, NULL, NULL,
                               &startup_info, process_handle));
 }

diff --git a/base/win/startup_information_unittest.cc b/base/win/startup_information_unittest.cc
@@ -65,7 +65,7 @@ TEST_F(StartupInformationTest, InheritStdOut) {
       MakeCmdLine("FireInheritedEvents").GetCommandLineString();
 
   PROCESS_INFORMATION temp_process_info = {};
-  ASSERT_TRUE(::CreateProcess(NULL, const_cast<wchar_t*>(cmd_line.c_str()),
+  ASSERT_TRUE(::CreateProcess(NULL, &cmd_line[0],
                               NULL, NULL, true, EXTENDED_STARTUPINFO_PRESENT,
                               NULL, NULL, startup_info.startup_info(),
                               &temp_process_info)) << ::GetLastError();

diff --git a/chrome/installer/util/delete_tree_work_item_unittest.cc b/chrome/installer/util/delete_tree_work_item_unittest.cc
@@ -189,8 +189,9 @@ TEST_F(DeleteTreeWorkItemTest, DeleteTreeInUse) {
   // Run the key path file to keep it in use.
   STARTUPINFOW si = {sizeof(si)};
   PROCESS_INFORMATION pi = {0};
+  base::FilePath::StringType writable_key_path = key_path.value();
   ASSERT_TRUE(
-      ::CreateProcessW(NULL, const_cast<wchar_t*>(key_path.value().c_str()),
+      ::CreateProcessW(NULL, &writable_key_path[0],
                        NULL, NULL, FALSE, CREATE_NO_WINDOW | CREATE_SUSPENDED,
                        NULL, NULL, &si, &pi));
 

diff --git a/cloud_print/service/win/chrome_launcher.cc b/cloud_print/service/win/chrome_launcher.cc
@@ -76,8 +76,10 @@ bool LaunchProcess(const CommandLine& cmdline,
   startup_info.wShowWindow = SW_SHOW;
 
   PROCESS_INFORMATION temp_process_info = {};
+  base::FilePath::StringType writable_cmdline_str(
+      cmdline.GetCommandLineString());
   if (!CreateProcess(NULL,
-      const_cast<wchar_t*>(cmdline.GetCommandLineString().c_str()), NULL, NULL,
+      &writable_cmdline_str[0], NULL, NULL,
       FALSE, 0, NULL, NULL, &startup_info, &temp_process_info)) {
     return false;
   }

diff --git a/sandbox/win/src/policy_target_test.cc b/sandbox/win/src/policy_target_test.cc
@@ -151,8 +151,11 @@ SBOX_TESTS_COMMAND int PolicyTargetTest_process(int argc, wchar_t **argv) {
   STARTUPINFO startup_info = {0};
   startup_info.cb = sizeof(startup_info);
   PROCESS_INFORMATION temp_process_info = {};
-  if (!::CreateProcessW(L"foo.exe", L"foo.exe", NULL, NULL, FALSE, 0,
-                        NULL, NULL, &startup_info, &temp_process_info))
+  // Note: CreateProcessW() can write to its lpCommandLine, don't pass a
+  // raw string literal.
+  base::string16 writable_cmdline_str(L"foo.exe");
+  if (!::CreateProcessW(L"foo.exe", &writable_cmdline_str[0], NULL, NULL, FALSE,
+                        0, NULL, NULL, &startup_info, &temp_process_info))
     return SBOX_TEST_SUCCEEDED;
   base::win::ScopedProcessInformation process_info(temp_process_info);
   return SBOX_TEST_FAILED;

diff --git a/sandbox/win/src/process_policy_test.cc b/sandbox/win/src/process_policy_test.cc
@@ -44,15 +44,21 @@ sandbox::SboxTestResult CreateProcessHelper(const base::string16& exe,
   if (!exe.empty())
     exe_name = exe.c_str();
 
-  const wchar_t *cmd_line = NULL;
-  if (!command.empty())
-    cmd_line = command.c_str();
+  base::string16 writable_command = command;
 
   // Create the process with the unicode version of the API.
   sandbox::SboxTestResult ret1 = sandbox::SBOX_TEST_FAILED;
   PROCESS_INFORMATION temp_process_info = {};
-  if (::CreateProcessW(exe_name, const_cast<wchar_t*>(cmd_line), NULL, NULL,
-                       FALSE, 0, NULL, NULL, &si, &temp_process_info)) {
+  if (::CreateProcessW(exe_name,
+                       command.empty() ? NULL : &writable_command[0],
+                       NULL,
+                       NULL,
+                       FALSE,
+                       0,
+                       NULL,
+                       NULL,
+                       &si,
+                       &temp_process_info)) {
     pi.Set(temp_process_info);
     ret1 = sandbox::SBOX_TEST_SUCCEEDED;
   } else {
@@ -72,12 +78,11 @@ sandbox::SboxTestResult CreateProcessHelper(const base::string16& exe,
   STARTUPINFOA sia = {sizeof(sia)};
   sandbox::SboxTestResult ret2 = sandbox::SBOX_TEST_FAILED;
 
-  std::string narrow_cmd_line;
-  if (cmd_line)
-    narrow_cmd_line = base::SysWideToMultiByte(cmd_line, CP_UTF8);
+  std::string narrow_cmd_line =
+      base::SysWideToMultiByte(command.c_str(), CP_UTF8);
   if (::CreateProcessA(
         exe_name ? base::SysWideToMultiByte(exe_name, CP_UTF8).c_str() : NULL,
-        cmd_line ? const_cast<char*>(narrow_cmd_line.c_str()) : NULL,
+        command.empty() ? NULL : &narrow_cmd_line[0],
         NULL, NULL, FALSE, 0, NULL, NULL, &sia, &temp_process_info)) {
     pi.Set(temp_process_info);
     ret2 = sandbox::SBOX_TEST_SUCCEEDED;

diff --git a/tools/win/link_limiter/limiter.cc b/tools/win/link_limiter/limiter.cc
@@ -74,7 +74,7 @@ static DWORD RunExe(const tstring& exe_name) {
   }
 
   if (!CreateProcess(NULL,  // lpApplicationName
-                     const_cast<TCHAR*>(cmdline.c_str()),
+                     &cmdline[0],
                      NULL,  // lpProcessAttributes
                      NULL,  // lpThreadAttributes
                      TRUE,  // bInheritHandles

diff --git a/tools/win/split_link/split_link.cc b/tools/win/split_link/split_link.cc
@@ -135,8 +135,7 @@ static void RunLinker(const vector<wstring>& prefix, const wchar_t* msg) {
     fflush(stdout);
   }
   if (!CreateProcess(NULL,
-                     reinterpret_cast<LPWSTR>(const_cast<wchar_t *>(
-                             cmd.c_str())),
+                     &cmd[0],
                      NULL,
                      NULL,
                      TRUE,

diff --git a/win8/delegate_execute/command_execute_impl.cc b/win8/delegate_execute/command_execute_impl.cc
@@ -399,7 +399,7 @@ HRESULT CommandExecuteImpl::LaunchDesktopChrome() {
 
   PROCESS_INFORMATION temp_process_info = {};
   BOOL ret = CreateProcess(chrome_exe_.value().c_str(),
-                           const_cast<LPWSTR>(command_line.c_str()),
+                           &command_line[0],
                            NULL, NULL, FALSE, 0, NULL, NULL, &start_info_,
                            &temp_process_info);
   if (ret) {