[USETUP]
authorAleksey Bragin <aleksey@reactos.org>
Fri, 4 Mar 2011 18:18:05 +0000 (18:18 +0000)
committerAleksey Bragin <aleksey@reactos.org>
Fri, 4 Mar 2011 18:18:05 +0000 (18:18 +0000)
- Fix a buffer overflow (overread) when adding a locale key to the registry. The history of this bug is funny:
1. Eric wrote the code, which sets a key of REG_SZ type, as 4 widechars plus terminating zero, but passes 8 as the bytesize of the buffer. It's not fully correct (a terminating zero is absent from the bytesize of the buffer, but MSDN doesn't specify if it should be added or not, and hardcoding "8" is not the best idea too) but not dramatic. That was revision 9596, 7 years ago.
2. Lentin notices something is not right in this code, and decides to "fix" it by multiplying that same hardcoded value by.... guess what? sizeof(PWCHAR)! That is, size of a pointer, which on an x86 would be 4 bytes. Massive out of bounds access obviously happens. That was revision 31642, 3 years ago.
3. Very soon Colin reshuffles and improves the code based on patch #2635, however the problem still goes unnoticed (r31655+).
See issue #5810 for more details.

svn path=/trunk/; revision=50968

reactos/base/setup/usetup/settings.c

index ffc20ac..90fd263 100644 (file)
@@ -637,6 +637,10 @@ ProcessLocaleRegistry(PGENERIC_LIST List)
     if (LanguageId == NULL)
         return FALSE;
 
     if (LanguageId == NULL)
         return FALSE;
 
+    /* Skip first 4 zeroes */
+    if (wcslen(LanguageId) >= 4)
+        LanguageId += 4;
+
     /* Open the NLS language key */
     RtlInitUnicodeString(&KeyName,
                          L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\NLS\\Language");
     /* Open the NLS language key */
     RtlInitUnicodeString(&KeyName,
                          L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\NLS\\Language");
@@ -660,13 +664,12 @@ ProcessLocaleRegistry(PGENERIC_LIST List)
     /* Set default language */
     RtlInitUnicodeString(&ValueName,
                          L"Default");
     /* Set default language */
     RtlInitUnicodeString(&ValueName,
                          L"Default");
-
     Status = NtSetValueKey(KeyHandle,
                            &ValueName,
                            0,
                            REG_SZ,
     Status = NtSetValueKey(KeyHandle,
                            &ValueName,
                            0,
                            REG_SZ,
-                           (PVOID)(LanguageId + 4),
-                           8 * sizeof(PWCHAR));
+                           (PVOID)LanguageId,
+                           (wcslen(LanguageId) + 1) * sizeof(WCHAR));
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("NtSetValueKey() failed (Status %lx)\n", Status);
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("NtSetValueKey() failed (Status %lx)\n", Status);
@@ -681,8 +684,8 @@ ProcessLocaleRegistry(PGENERIC_LIST List)
                             &ValueName,
                             0,
                             REG_SZ,
                             &ValueName,
                             0,
                             REG_SZ,
-                            (PVOID)(LanguageId + 4),
-                            8 * sizeof(PWCHAR));
+                            (PVOID)LanguageId,
+                            (wcslen(LanguageId) + 1) * sizeof(WCHAR));
     NtClose(KeyHandle);
     if (!NT_SUCCESS(Status))
     {
     NtClose(KeyHandle);
     if (!NT_SUCCESS(Status))
     {