Hesham/wintun
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

api: check the stdout reader thread exit status for failures

Browse Source 
...and refactor the ExecuteRunDll32().

Reported-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Simon Rozman <simon@rozman.si>
This commit is contained in:
Simon Rozman 2020-10-30 07:29:40 +01:00
parent 44568f81cb
commit fb6d5b62f1
1 changed files with 38 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
api/adapter.c
View File

@ -1279,13 +1279,13 @@ ProcessStdout(_Inout_ PROCESS_STDOUT_STATE *State)
{ {
DWORD SizeRead; DWORD SizeRead;
if (!ReadFile(State->Stdout, State->Response + Offset, sizeof(WCHAR) * (MaxLen - Offset), &SizeRead, NULL)) if (!ReadFile(State->Stdout, State->Response + Offset, sizeof(WCHAR) * (MaxLen - Offset), &SizeRead, NULL))
return 0; return ERROR_SUCCESS;
if (SizeRead % sizeof(WCHAR)) if (SizeRead % sizeof(WCHAR))
return 1; return ERROR_INVALID_DATA;
Offset += SizeRead / sizeof(WCHAR); Offset += SizeRead / sizeof(WCHAR);
State->Response[Offset] = 0; State->Response[Offset] = 0;
} }
return 2; return ERROR_BUFFER_OVERFLOW;
} }
static DWORD WINAPI static DWORD WINAPI
@ -1308,9 +1308,9 @@ ProcessStderr(_In_ HANDLE Stderr)
WCHAR Buf[0x200]; WCHAR Buf[0x200];
DWORD SizeRead; DWORD SizeRead;
if (!ReadFile(Stderr, Buf, sizeof(Buf), &SizeRead, NULL)) if (!ReadFile(Stderr, Buf, sizeof(Buf), &SizeRead, NULL))
return 0; return ERROR_SUCCESS;
if (SizeRead % sizeof(WCHAR)) if (SizeRead % sizeof(WCHAR))
return 1; return ERROR_INVALID_DATA;
SizeRead /= sizeof(WCHAR); SizeRead /= sizeof(WCHAR);
for (DWORD i = 0; i < SizeRead; ++i) for (DWORD i = 0; i < SizeRead; ++i)
{ {
@ -1401,33 +1401,27 @@ ExecuteRunDll32(
.bInheritHandle = TRUE, .bInheritHandle = TRUE,
.lpSecurityDescriptor = .lpSecurityDescriptor =
SecurityAttributes ? SecurityAttributes->lpSecurityDescriptor : NULL }; SecurityAttributes ? SecurityAttributes->lpSecurityDescriptor : NULL };
typedef enum HANDLE StreamRStdout = INVALID_HANDLE_VALUE, StreamRStderr = INVALID_HANDLE_VALUE,
{ StreamWStdout = INVALID_HANDLE_VALUE, StreamWStderr = INVALID_HANDLE_VALUE;
Stdout = 0, if (!CreatePipe(&StreamRStdout, &StreamWStdout, &sa, 0) || !CreatePipe(&StreamRStderr, &StreamWStderr, &sa, 0))
Stderr
} stdid_t;
HANDLE StreamR[] = { INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE },
StreamW[] = { INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE };
if (!CreatePipe(&StreamR[Stdout], &StreamW[Stdout], &sa, 0) ||
!CreatePipe(&StreamR[Stderr], &StreamW[Stderr], &sa, 0))
{ {
Result = LOG_LAST_ERROR(L"Failed to create pipes"); Result = LOG_LAST_ERROR(L"Failed to create pipes");
goto cleanupPipes; goto cleanupPipes;
} }
if (!SetHandleInformation(StreamR[Stdout], HANDLE_FLAG_INHERIT, 0) || if (!SetHandleInformation(StreamRStdout, HANDLE_FLAG_INHERIT, 0) ||
!SetHandleInformation(StreamR[Stderr], HANDLE_FLAG_INHERIT, 0)) !SetHandleInformation(StreamRStderr, HANDLE_FLAG_INHERIT, 0))
{ {
Result = LOG_LAST_ERROR(L"Failed to set handle info"); Result = LOG_LAST_ERROR(L"Failed to set handle info");
goto cleanupPipes; goto cleanupPipes;
} }
if (ResponseCapacity) if (ResponseCapacity)
Response[0] = 0; Response[0] = 0;
PROCESS_STDOUT_STATE ProcessStdoutState = { .Stdout = StreamR[Stdout], PROCESS_STDOUT_STATE ProcessStdoutState = { .Stdout = StreamRStdout,
.Response = Response, .Response = Response,
.ResponseCapacity = ResponseCapacity }; .ResponseCapacity = ResponseCapacity };
HANDLE Thread[] = { NULL, NULL }; HANDLE ThreadStdout = NULL, ThreadStderr = NULL;
if ((Thread[Stdout] = CreateThread(SecurityAttributes, 0, ProcessStdout, &ProcessStdoutState, 0, NULL)) == NULL || if ((ThreadStdout = CreateThread(SecurityAttributes, 0, ProcessStdout, &ProcessStdoutState, 0, NULL)) == NULL ||
(Thread[Stderr] = CreateThread(SecurityAttributes, 0, ProcessStderr, StreamR[Stderr], 0, NULL)) == NULL) (ThreadStderr = CreateThread(SecurityAttributes, 0, ProcessStderr, StreamRStderr, 0, NULL)) == NULL)
{ {
Result = LOG_LAST_ERROR(L"Failed to spawn reader threads"); Result = LOG_LAST_ERROR(L"Failed to spawn reader threads");
goto cleanupThreads; goto cleanupThreads;
@ -1435,8 +1429,8 @@ ExecuteRunDll32(
STARTUPINFOW si = { .cb = sizeof(STARTUPINFO), STARTUPINFOW si = { .cb = sizeof(STARTUPINFO),
.dwFlags = STARTF_USESHOWWINDOW | STARTF_USESTDHANDLES, .dwFlags = STARTF_USESHOWWINDOW | STARTF_USESTDHANDLES,
.wShowWindow = SW_HIDE, .wShowWindow = SW_HIDE,
.hStdOutput = StreamW[Stdout], .hStdOutput = StreamWStdout,
.hStdError = StreamW[Stderr] }; .hStdError = StreamWStderr };
PROCESS_INFORMATION pi; PROCESS_INFORMATION pi;
if (!CreateProcessW(RunDll32Path, CommandLine, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) if (!CreateProcessW(RunDll32Path, CommandLine, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi))
{ {
@ -1447,19 +1441,29 @@ ExecuteRunDll32(
CloseHandle(pi.hProcess); CloseHandle(pi.hProcess);
CloseHandle(pi.hThread); CloseHandle(pi.hThread);
cleanupThreads: cleanupThreads:
for (size_t i = _countof(Thread); i--;) if (ThreadStderr)
if (Thread[i]) {
{ CloseHandle(StreamWStderr);
CloseHandle(StreamW[i]); StreamWStderr = INVALID_HANDLE_VALUE;
StreamW[i] = INVALID_HANDLE_VALUE; WaitForSingleObject(ThreadStderr, INFINITE);
WaitForSingleObject(Thread[i], INFINITE); CloseHandle(ThreadStderr);
CloseHandle(Thread[i]); }
} if (ThreadStdout)
{
CloseHandle(StreamWStdout);
StreamWStdout = INVALID_HANDLE_VALUE;
WaitForSingleObject(ThreadStdout, INFINITE);
if (!GetExitCodeThread(ThreadStdout, &Result))
Result = LOG_LAST_ERROR(L"Failed to retrieve thread result");
else if (Result != ERROR_SUCCESS)
LOG_ERROR(L"Failed to read process output", Result);
CloseHandle(ThreadStdout);
}
cleanupPipes: cleanupPipes:
CloseHandle(StreamR[Stderr]); CloseHandle(StreamRStderr);
CloseHandle(StreamW[Stderr]); CloseHandle(StreamWStderr);
CloseHandle(StreamR[Stdout]); CloseHandle(StreamRStdout);
CloseHandle(StreamW[Stdout]); CloseHandle(StreamWStdout);
HeapFree(ModuleHeap, 0, CommandLine); HeapFree(ModuleHeap, 0, CommandLine);
cleanupDelete: cleanupDelete:
DeleteFileW(DllPath); DeleteFileW(DllPath);