From 85f0fb63f37d72783c2c7372452682d371a3b63f Mon Sep 17 00:00:00 2001 From: Aleksey Bragin Date: Fri, 4 Mar 2011 18:18:05 +0000 Subject: [PATCH] [USETUP] - 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 | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/reactos/base/setup/usetup/settings.c b/reactos/base/setup/usetup/settings.c index ffc20ac286a..90fd2634585 100644 --- a/reactos/base/setup/usetup/settings.c +++ b/reactos/base/setup/usetup/settings.c @@ -637,6 +637,10 @@ ProcessLocaleRegistry(PGENERIC_LIST List) 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"); @@ -660,13 +664,12 @@ ProcessLocaleRegistry(PGENERIC_LIST List) /* Set default language */ RtlInitUnicodeString(&ValueName, L"Default"); - 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); @@ -681,8 +684,8 @@ ProcessLocaleRegistry(PGENERIC_LIST List) &ValueName, 0, REG_SZ, - (PVOID)(LanguageId + 4), - 8 * sizeof(PWCHAR)); + (PVOID)LanguageId, + (wcslen(LanguageId) + 1) * sizeof(WCHAR)); NtClose(KeyHandle); if (!NT_SUCCESS(Status)) { -- 2.17.1