https://git.reactos.org/?p=reactos.git;a=commitdiff;h=c5158963a3be6c19da30a2f9000e88e7f88a4986
commit c5158963a3be6c19da30a2f9000e88e7f88a4986 Author: Timo Kreuzer <[email protected]> AuthorDate: Sat Dec 10 14:07:16 2022 +0200 Commit: Timo Kreuzer <[email protected]> CommitDate: Wed Jan 4 10:32:28 2023 +0100 [ADVAPI32] Fix a buffer overflow in RegQueryValueExA The code was trying to check whether the output string was already NULL terminated by RtlUnicodeToMultiByteN before NULL terminating it by checking DataStr[*count - 1] for a NULL terminator. But since RtlUnicodeToMultiByteSize always returns the size without the NULL terminator, DataStr[*count - 1] would always be the last actual character, never an optional NULL terminator. For 0 sized strings this would actually lead to accessing the output buffer at position -1 (on 32 bit) or 0xFFFFFFFF (on 64 bit). Fix this by removing the check. This fixes a crash in advapi32_winetest:registry on x64. --- dll/win32/advapi32/reg/reg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dll/win32/advapi32/reg/reg.c b/dll/win32/advapi32/reg/reg.c index 9beecc99d2c..904ca010eca 100644 --- a/dll/win32/advapi32/reg/reg.c +++ b/dll/win32/advapi32/reg/reg.c @@ -4088,6 +4088,7 @@ RegQueryValueExA( /* We don't need this anymore */ RtlFreeUnicodeString(&nameW); + /* Get the length for the multi-byte string (without the terminating NULL!) */ DataLength = *count; RtlUnicodeToMultiByteSize(count, Buffer, BufferSize); @@ -4101,7 +4102,7 @@ RegQueryValueExA( RtlUnicodeToMultiByteN(DataStr, DataLength, NULL, Buffer, BufferSize); /* NULL-terminate if there is enough room */ - if ((DataLength > *count) && (DataStr[*count - 1] != '\0')) + if (DataLength > *count) DataStr[*count] = '\0'; RtlFreeHeap(RtlGetProcessHeap(), 0, Buffer);
