Detecting and fixing stringprintf.h format bugs

content/common/sandbox_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/common/sandbox_win.h"
 
 #include <string>
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
@@ skipping 206 matching lines @@
 // Eviction of injected DLLs is done by the sandbox so that the injected module
 // does not get a chance to execute any code.
 void AddGenericDllEvictionPolicy(sandbox::TargetPolicy* policy) {
   for (int ix = 0; ix != arraysize(kTroublesomeDlls); ++ix)
     BlacklistAddOneDll(kTroublesomeDlls[ix], true, policy);
 }
 
 // Returns the object path prepended with the current logon session.
 base::string16 PrependWindowsSessionPath(const base::char16* object) {
   // Cache this because it can't change after process creation.
-  static uintptr_t s_session_id = 0;
+  static uint64_t s_session_id = 0;

[Will Harris, 2015/09/29 00:29:17]

This should be DWORD, since GetTokenInformation with TokenSessionId always
returns a DWORD

[brucedawson, 2015/09/29 21:39:40]

Done.

   if (s_session_id == 0) {
     HANDLE token;
     DWORD session_id_length;
     DWORD session_id = 0;
 
     CHECK(::OpenProcessToken(::GetCurrentProcess(), TOKEN_QUERY, &token));
     CHECK(::GetTokenInformation(token, TokenSessionId, &session_id,
         sizeof(session_id), &session_id_length));
     CloseHandle(token);
     if (session_id)
       s_session_id = session_id;
   }
 
-  return base::StringPrintf(L"\\Sessions\\%d%ls", s_session_id, object);
+  return base::StringPrintf(L"\\Sessions\\%llu%ls", s_session_id, object);

[Will Harris, 2015/09/29 00:29:17]

%lu seems sensible here?

[brucedawson, 2015/09/29 21:39:40]

If s_session_id is a DWORD then %lu does seem best.

 }
 
 // Checks if the sandbox should be let to run without a job object assigned.
 bool ShouldSetJobLevel(const base::CommandLine& cmd_line) {
   if (!cmd_line.HasSwitch(switches::kAllowNoSandboxJob))
     return true;
 
   // Windows 8 allows nested jobs so we don't need to check if we are in other
   // job.
   if (base::win::GetVersion() >= base::win::VERSION_WIN8)
@@ skipping 615 matching lines @@
   }
 
   return false;
 }
 
 bool BrokerAddTargetPeer(HANDLE peer_process) {
   return g_broker_services->AddTargetPeer(peer_process) == sandbox::SBOX_ALL_OK;
 }
 
 }  // namespace content