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:
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 s
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? MSD
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 |