[PATCH 0/2] MR6049: ntdll: Double-null-terminate registry multi-strings in RtlQueryRegistryValues.
This reverts most of commit 2821a850d401dfabc5d76a4f40b0d6a42045fc5e which was a partial and ultimately improper solution. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6049
From: Alex Henrie <alexhenrie24(a)gmail.com> This reverts most of commit 2821a850d401dfabc5d76a4f40b0d6a42045fc5e which was a partial and ultimately improper solution. --- dlls/ntdll/reg.c | 32 ++++++++++++++++++++++---------- dlls/ntdll/tests/reg.c | 18 ++++++++++++++---- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/dlls/ntdll/reg.c b/dlls/ntdll/reg.c index 934124a0360..fb16f6d2dc6 100644 --- a/dlls/ntdll/reg.c +++ b/dlls/ntdll/reg.c @@ -254,6 +254,16 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, type = pInfo->Type; data = (WCHAR*)((char*)pInfo + pInfo->DataOffset); len = pInfo->DataLength; + + /* Ensure that multi-strings from the registry are double-null-terminated */ + if (type == REG_MULTI_SZ) + { + while (len < 2 * sizeof(WCHAR) || data[len / sizeof(WCHAR) - 2] || data[len / sizeof(WCHAR) - 1]) + { + data[len / sizeof(WCHAR)] = 0; + len += sizeof(WCHAR); + } + } } else { @@ -275,10 +285,13 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, break; case REG_MULTI_SZ: - for (wstr = data; *wstr; wstr += count) + wstr = data; + for (;;) { count = wcslen(wstr) + 1; len += count * sizeof(WCHAR); + if (!*wstr) break; + wstr += count; } break; } @@ -322,7 +335,6 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, if (!(pQuery->Flags & RTL_QUERY_REGISTRY_NOEXPAND)) return STATUS_INVALID_PARAMETER; - len += sizeof(WCHAR); if (str->Buffer == NULL) { str->Buffer = RtlAllocateHeap(GetProcessHeap(), 0, len); @@ -330,10 +342,8 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, } else if (str->MaximumLength < len) return STATUS_BUFFER_TOO_SMALL; - len -= sizeof(WCHAR); memcpy(str->Buffer, data, len); - str->Buffer[len / sizeof(WCHAR)] = 0; - str->Length = len; + str->Length = (len >= sizeof(WCHAR) ? len - sizeof(WCHAR) : len); break; default: @@ -379,7 +389,7 @@ static NTSTATUS RTL_ReportRegistryValue(PKEY_VALUE_FULL_INFORMATION pInfo, } else /* REG_MULTI_SZ */ { - for (offset = 0; offset < len; offset += count) + for (offset = 0; offset + 2 * sizeof(WCHAR) < len; offset += count) { wstr = (WCHAR*)((char*)data + offset); count = (wcslen(wstr) + 1) * sizeof(WCHAR); @@ -544,9 +554,10 @@ NTSTATUS WINAPI RtlQueryRegistryValues(IN ULONG RelativeTo, IN PCWSTR Path, if (status == STATUS_NO_MORE_ENTRIES) break; if (status == STATUS_BUFFER_OVERFLOW || - status == STATUS_BUFFER_TOO_SMALL) + status == STATUS_BUFFER_TOO_SMALL || + (status == STATUS_SUCCESS && pInfo->Type == REG_MULTI_SZ && buflen < len + 2 * sizeof(L'\0'))) { - buflen = len; + buflen = len + 2 * sizeof(L'\0'); RtlFreeHeap(GetProcessHeap(), 0, pInfo); pInfo = RtlAllocateHeap(GetProcessHeap(), 0, buflen); NtEnumerateValueKey(handle, i, KeyValueFullInformation, @@ -578,9 +589,10 @@ NTSTATUS WINAPI RtlQueryRegistryValues(IN ULONG RelativeTo, IN PCWSTR Path, status = NtQueryValueKey(handle, &Value, KeyValueFullInformation, pInfo, buflen, &len); if (status == STATUS_BUFFER_OVERFLOW || - status == STATUS_BUFFER_TOO_SMALL) + status == STATUS_BUFFER_TOO_SMALL || + (status == STATUS_SUCCESS && pInfo->Type == REG_MULTI_SZ && buflen < len + 2 * sizeof(L'\0'))) { - buflen = len; + buflen = len + 2 * sizeof(L'\0'); RtlFreeHeap(GetProcessHeap(), 0, pInfo); pInfo = RtlAllocateHeap(GetProcessHeap(), 0, buflen); status = NtQueryValueKey(handle, &Value, diff --git a/dlls/ntdll/tests/reg.c b/dlls/ntdll/tests/reg.c index b9047c41972..3f41856e4d1 100644 --- a/dlls/ntdll/tests/reg.c +++ b/dlls/ntdll/tests/reg.c @@ -2770,7 +2770,7 @@ static struct query_reg_values_test query_reg_values_tests[] = }, { {{ query_routine, RTL_QUERY_REGISTRY_NOEXPAND, (WCHAR*)L"CapitalsOfEurope" }}, - STATUS_SUCCESS, 1, WINE_TODO_SIZE, REG_MULTI_SZ, L"Brussels\0Paris\0%PATH%\0", sizeof(L"Brussels\0Paris\0%PATH%\0") + STATUS_SUCCESS, 1, 0, REG_MULTI_SZ, L"Brussels\0Paris\0%PATH%\0", sizeof(L"Brussels\0Paris\0%PATH%\0") }, /* The default value is used if the registry value does not exist */ { @@ -2789,6 +2789,16 @@ static struct query_reg_values_test query_reg_values_tests[] = {{ query_routine, 0, (WCHAR*)L"I don't exist", NULL, REG_MULTI_SZ, (WCHAR*)L"Brussels\0Paris\0%PATH%\0" }}, STATUS_SUCCESS, 3, EXPECT_DEFAULT_DATA | SPLIT_MULTI }, + { + {{ query_routine, 0, (WCHAR*)L"I don't exist", + NULL, REG_MULTI_SZ, (WCHAR*)L"A\0B\0C", sizeof(L"A\0B\0C") }}, + STATUS_SUCCESS, 2, EXPECT_DEFAULT_DATA | SPLIT_MULTI + }, + { + {{ query_routine, 0, (WCHAR*)L"I don't exist", + NULL, REG_MULTI_SZ, (WCHAR*)L"A\0B\0C", sizeof(L"A\0B\0C") - sizeof(L'\0') }}, + STATUS_SUCCESS, 2, EXPECT_DEFAULT_DATA | SPLIT_MULTI + }, { {{ query_routine, 0, (WCHAR*)L"I don't exist", NULL, REG_DWORD, (WCHAR*)0xdeadbeef }}, STATUS_SUCCESS, 1, EXPECT_DEFAULT_DATA @@ -2866,8 +2876,8 @@ static struct query_reg_values_test query_reg_values_tests[] = }, */ { {{ NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOEXPAND, (WCHAR*)L"I don't exist", - &query_reg_values_direct_str, REG_MULTI_SZ, (WCHAR*)L"A\0B\0C\0", sizeof(L"A\0B\0C\0") }}, - STATUS_SUCCESS, 0, EXPECT_DEFAULT_DATA | WINE_TODO_SIZE + &query_reg_values_direct_str, REG_MULTI_SZ, (WCHAR*)L"A\0B\0C", sizeof(L"A\0B\0C") - sizeof(L'\0') }}, + STATUS_SUCCESS, 0, EXPECT_DEFAULT_DATA }, /* The default value is not used if it is not valid */ { @@ -2920,7 +2930,7 @@ static void test_RtlQueryRegistryValues(void) ok(status == ERROR_SUCCESS, "Failed to create registry value WindowsDrive: %lu\n", status); status = RegSetKeyValueW(HKEY_CURRENT_USER, L"WineTest", L"CapitalsOfEurope", REG_MULTI_SZ, - L"Brussels\0Paris\0%PATH%", sizeof(L"Brussels\0Paris\0%PATH%")); + L"Brussels\0Paris\0%PATH%", sizeof(L"Brussels\0Paris\0%PATH%") - sizeof(L'\0')); ok(status == ERROR_SUCCESS, "Failed to create registry value CapitalsOfEurope: %lu\n", status); status = RegSetKeyValueW(HKEY_CURRENT_USER, L"WineTest", L"MeaningOfLife32", REG_DWORD, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6049
From: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/ntdll/tests/reg.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/dlls/ntdll/tests/reg.c b/dlls/ntdll/tests/reg.c index 3f41856e4d1..f264f42e98d 100644 --- a/dlls/ntdll/tests/reg.c +++ b/dlls/ntdll/tests/reg.c @@ -2547,7 +2547,6 @@ struct query_reg_values_test WINE_TODO_NAME = 0x40, WINE_TODO_TYPE = 0x80, WINE_TODO_SIZE = 0x100, - WINE_TODO_DATA = 0x200, } flags; ULONG expected_type; @@ -2611,7 +2610,6 @@ static NTSTATUS WINAPI query_routine(const WCHAR *value_name, ULONG value_type, todo_wine_if(test->flags & WINE_TODO_SIZE) ok(value_data_size == expected_size, "Expected size %lu, got %lu\n", expected_size, value_data_size); - todo_wine_if(test->flags & WINE_TODO_DATA && !(test->flags & SPLIT_MULTI && query_routine_calls == 0)) if (expected_data == query->DefaultData || expected_data == NULL) ok(value_data == expected_data, "Expected data %p, got %p\n", expected_data, value_data); else @@ -3024,7 +3022,6 @@ static void test_RtlQueryRegistryValues(void) if (expected_data) { - todo_wine_if(test->flags & WINE_TODO_DATA) ok(!memcmp(query_reg_values_direct_str.Buffer, expected_data, expected_size), "Expected data %s, got %s\n", debugstr_w(expected_data), debugstr_w(query_reg_values_direct_str.Buffer)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6049
participants (2)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)