Fix potential buffer overrun in credguard disable

The call to `GetModuleFileNameEx` passes in `sizeof(szModulename)` for the size parameter. The documentation for that API says the size parameter is a character count, not a byte count ("The size of the lpFilename buffer, in characters.").  Since the code currently passes in a byte count, this opens up the possibility for a stack buffer overrun on UNICODE compilations of this tool where the byte count will be `2*MAX_PATH` which `GetModuleFileNameEx` will interpret as a character count and potentially write up to `2*2*MAX_PATH' bytes into the buffer.  Fix by passing in a character count.  You could also use a macro like `ARRAYSIZE(szModulename)`.


```diff
    TCHAR szModulename[MAX_PATH];
    for (DWORD i = 0; i < (lpcbNeeded / sizeof(HMODULE)); i++) {
        if (hModulesArray[i] && !GetModuleFileNameEx(hLsass, hModulesArray[i], szModulename, sizeof(szModulename))) {
...        }
```

[1] Docs for GetModuleFileNameEx are here (https://docs.microsoft.com/en-us/windows/win32/api/psapi/nf-psapi-getmodulefilenameexa)
This commit is contained in:
John Lambert
2021-12-08 07:15:06 -08:00
committed by GitHub
parent f3147ecb8a
commit 2072b71d05
@@ -54,7 +54,7 @@ DWORD WINAPI disableCredGuardByPatchingLSASS(void) {
BOOL returnStatus = FALSE;
TCHAR szModulename[MAX_PATH];
for (DWORD i = 0; i < (lpcbNeeded / sizeof(HMODULE)); i++) {
if (hModulesArray[i] && !GetModuleFileNameEx(hLsass, hModulesArray[i], szModulename, sizeof(szModulename))) {
if (hModulesArray[i] && !GetModuleFileNameEx(hLsass, hModulesArray[i], szModulename, MAX_PATH)) {
_tprintf(TEXT("[!] Cred Guard bypass non fatal error: couldn't get module name for module at index 0x%lx (GetModuleFileNameEx, error code 0x%lx)\n"), i, GetLastError());
continue;
}
@@ -167,4 +167,4 @@ DWORD WINAPI disableCredGuardByPatchingLSASS(void) {
CloseHandle(hLsass);
return returnStatus;
}
}