Mishandled NtQuerySecurityObject syscall

[derekbruening]

From timurrrr@google.com on May 20, 2011 10:20:04

Seen on Chromium: http://build.chromium.org/p/chromium.fyi/builders/Windows%20Tests%20%28DrMemory%29/builds/3735/steps/memory%20test%3A%20base/logs/stdio UNADDRESSABLE ACCESS: reading 0x001fd148-0x001fd15c 20 byte(s) within 0x001fd148-0x001fd15c
Note: next higher malloc: 0x001fd258-0x001fd278
#1 <sys.call> NtQuerySecurityObject
#2 0x77df39eb RegGetKeySecurity ADVAPI32.dll
#3 0x71aa1c6c WahOpenNotificationHandleHelper WS2HELP.dll
#4 0x71aa20f2 WahCreateNotificationHandle WS2HELP.dll
#5 0x71ab8cb6 WSAProviderConfigChange WS2_32.dll
#6 0x77e9743f RpcServerUseProtseqExW RPCRT4.dll
#7 0x77e772a0 I_RpcBCacheFree RPCRT4.dll
#8 0x77e77328 I_RpcBCacheFree RPCRT4.dll
#9 0x77e76ad1 I_RpcBCacheFree RPCRT4.dll
#10 0x77e76c97 I_RpcBCacheFree RPCRT4.dll
#11 0x7c80b729 GetModuleFileNameA KERNEL32.dll

Short repro:

#include <windows.h>
#include <stdio.h>

#pragma comment(lib, "advapi32.lib")

int main() {
HKEY hklm_key;
DWORD disp;
if (RegCreateKeyExW(HKEY_LOCAL_MACHINE, L"Software\Google\Common\Rlz",
0, NULL,
REG_OPTION_NON_VOLATILE,
KEY_ALL_ACCESS | KEY_WOW64_32KEY,
NULL, &hklm_key, &disp) != ERROR_SUCCESS) {
printf("Failed to access the key\n");
return 1;
}

// Create a SID that represents ALL USERS.
DWORD users_sid_size = SECURITY_MAX_SID_SIZE;
SID users_sid[SECURITY_MAX_SID_SIZE];
::CreateWellKnownSid(WinBuiltinUsersSid, NULL, users_sid, &users_sid_size);

// Get the security descriptor for the registry key.
DWORD original_sd_size = 0;
::RegGetKeySecurity(hklm_key, DACL_SECURITY_INFORMATION, NULL,
&original_sd_size);
return 0;
}

Error #1: UNADDRESSABLE ACCESS: reading 0x001522e8-0x001522fc 20 byte(s) within 0x001522e8-0x001522fc
@0:00:01.047 in thread 12444
Note: next higher malloc: 0x00152328-0x0015234a
Note: prev lower malloc: 0x001520b8-0x001522cc
system call NtQuerySecurityObject

0x77df39eb <ADVAPI32.dll+0x239eb> ADVAPI32.dll!RegGetKeySecurity
0x004010aa <test.exe+0x10aa> test.exe!main
test.cpp:26

Original issue: http://code.google.com/p/drmemory/issues/detail?id=408

[derekbruening]

From bruen...@google.com on May 20, 2011 07:25:10

how is this different from issue #405 ?

[derekbruening]

From timurrrr@google.com on May 20, 2011 07:29:44

Issue 405 has been merged into this issue.

[derekbruening]

From timurrrr@google.com on May 20, 2011 07:30:11

It has a repro :)
Sorry, I've forgotten about 405

[derekbruening]

From timurrrr@google.com on May 20, 2011 09:00:00

Larger test (just for the reference)

#include <windows.h>
#include <stdio.h>

#pragma comment(lib, "advapi32.lib")

int main() {
HKEY hklm_key;
DWORD disp;
if (RegCreateKeyExW(HKEY_LOCAL_MACHINE, L"Software\Google\Common\Rlz",
0, NULL,
REG_OPTION_NON_VOLATILE,
KEY_ALL_ACCESS | KEY_WOW64_32KEY,
NULL, &hklm_key, &disp) != ERROR_SUCCESS) {
printf("Failed to access the key\n");
return 1;
}

// Create a SID that represents ALL USERS.
DWORD users_sid_size = SECURITY_MAX_SID_SIZE;
SID users_sid[SECURITY_MAX_SID_SIZE];
::CreateWellKnownSid(WinBuiltinUsersSid, NULL, users_sid, &users_sid_size);

// Get the security descriptor for the registry key.
DWORD original_sd_size = 0;
::RegGetKeySecurity(hklm_key, DACL_SECURITY_INFORMATION, NULL,
&original_sd_size);

SECURITY_DESCRIPTOR original_sd[10]; // this is a hack :)
if (sizeof(original_sd) < original_sd_size) {
printf("Oops wrong size assumption!\n");
return 2;
}

LONG result = ::RegGetKeySecurity(hklm_key.Handle(),
DACL_SECURITY_INFORMATION, original_sd, &original_sd_size);
if (result != ERROR_SUCCESS) {
printf("Failed reading the key\n");
return 3;
}
return 0;
}

[derekbruening]

From bruen...@google.com on May 20, 2011 10:12:05

from discussion: the underlying cause was that the syscall is called w/ output length 0 to obtain the require capacity and then called again and the syscall entry erroneously was not based on the passed-in length

[derekbruening]

From timurrrr@google.com on May 20, 2011 14:22:26

amending: with length=0 the wrong syscall entry resulted in accessing random pointer passed as arg #4 (0-based = #3), hence UNADDR accesses.

[derekbruening]

From timurrrr@google.com on May 21, 2011 07:14:48

Should be fixed by r304

Status: Fixed