Add a restricted ACL to installer temp directory

chrome/installer/mini_installer/mini_installer.cc

Index: chrome/installer/mini_installer/mini_installer.cc
diff --git a/chrome/installer/mini_installer/mini_installer.cc b/chrome/installer/mini_installer/mini_installer.cc
index 3ecd6e5fa90c156117f7d6d00e846d551a1a1003..bd3d81b16435a4b281fcb00cf0a67506a2933e3f 100644
--- a/chrome/installer/mini_installer/mini_installer.cc
+++ b/chrome/installer/mini_installer/mini_installer.cc
@@ -22,6 +22,15 @@
 #pragma comment(linker, "/MERGE:.rdata=.text")
 
 #include <windows.h>
+
+// #define needed to link in RtlGenRandom(), a.k.a. SystemFunction036.  See the
+// "Community Additions" comment on MSDN here:
+// http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694.aspx
+#define SystemFunction036 NTAPI SystemFunction036
+#include <NTSecAPI.h>
+#undef SystemFunction036
+
+#include <sddl.h>
 #include <shellapi.h>
 #include <stdlib.h>
 
@@ -563,19 +572,39 @@ bool CreateWorkDir(const wchar_t* base_path, PathString* work_dir) {
   if ((work_dir->capacity() - end) < (_countof("fffff.tmp") + 1))
     return false;
 
-  // Generate a unique id.  We only use the lowest 20 bits, so take the top
-  // 12 bits and xor them with the lower bits.
-  DWORD id = ::GetTickCount();
-  id ^= (id >> 12);
+  // Add an ACL if supported by the filesystem. Otherwise system-level installs
+  // are potentially vulnerable to file squatting attacks.
+  SECURITY_ATTRIBUTES sa = {};
+  sa.nLength = sizeof(SECURITY_ATTRIBUTES);
+  wchar_t volume[MAX_PATH];

[grt (UTC plus 2), 2015/12/04 15:37:35]

use PathString here rather than a wchar_t array:
  PathString volume;
  DWORD flags = 0;
  if (::GetVolumePathName(base_path, volume.capacity(), volume.get()) &&
      ::GetVolumeInformation(volume.get(), ...))

(and prefix win32 calls with :: since that seems to be the style of the file)

[jschuh, 2015/12/05 01:04:39]

Done.

+  DWORD flags = 0;
+  if (GetVolumePathName(base_path, volume, sizeof(volume)) &&
+      GetVolumeInformation(volume, nullptr, 0, nullptr, nullptr, &flags,

[grt (UTC plus 2), 2015/12/04 15:37:35]

sadly, we can't use C++-11 yet in mini_installer since (last i checked) the
signing bots are using an old toolchain. for now, use NULL everywhere rather
than nullptr. mmoss is working on resolving this.

[jschuh, 2015/12/05 01:04:39]

Done.

+                           nullptr, 0) &&
+      (flags & FILE_PERSISTENT_ACLS)) {
+    static const wchar_t sddl[] =
+        L"D:PAI(A;ID;FA;;;BA)"  // Admin: Full control.
+        L"(A;OICIIOID;GA;;;BA)"
+        L"(A;ID;FA;;;SY)"  // System: Full control.
+        L"(A;OICIIOID;GA;;;SY)"
+        L"(A;ID;FA;;;CO)"  // Owner: Full control.
+        L"(A;OICIIOID;GA;;;CO)";
+    if (!ConvertStringSecurityDescriptorToSecurityDescriptor(
+            sddl, SDDL_REVISION, &sa.lpSecurityDescriptor, NULL)) {

[grt (UTC plus 2), 2015/12/04 15:37:35]

is it safer to explicitly use SDDL_REVISION_1? MSDN says StringSDRevision must
be that value. i see that sddl.h defines SDDL_REVISION to SDDL_REVISION_1 for
now, but would we necessarily want to pick up a change in a future version of
the SDK automatically?

[jschuh, 2015/12/05 01:04:39]

Done.

+      return false;
+    }
+  }
 
-  int max_attempts = 10;
-  while (max_attempts--) {
+  bool result = false;
+  unsigned int id;
+  RtlGenRandom(&id, sizeof(id));
+  for (int max_attempts = 10; max_attempts; --max_attempts) {
     // This converts 'id' to a string in the format "78563412" on windows
     // because of little endianness, but we don't care since it's just
     // a name.
     if (!HexEncode(&id, sizeof(id), work_dir->get() + end,
                    work_dir->capacity() - end)) {
-      return false;
+      break;
     }
 
     // We only want the first 5 digits to remain within the 8.3 file name
@@ -585,14 +614,19 @@ bool CreateWorkDir(const wchar_t* base_path, PathString* work_dir) {
     // for consistency with the previous implementation which relied on
     // GetTempFileName, we append the .tmp extension.
     work_dir->append(L".tmp");
-    if (::CreateDirectory(work_dir->get(), NULL)) {
+
+    if (::CreateDirectory(work_dir->get(),
+                          sa.lpSecurityDescriptor ? &sa : nullptr)) {
       // Yay!  Now let's just append the backslash and we're done.
-      return work_dir->append(L"\\");
+      result = work_dir->append(L"\\");
+      break;
     }
     ++id;  // Try a different name.
   }
 
-  return false;
+  if (sa.lpSecurityDescriptor)
+    LocalFree(sa.lpSecurityDescriptor);
+  return result;
 }
 
 // Creates and returns a temporary directory in |work_dir| that can be used to