Improve trace() security: restrict debugserver impersonation

In certain situations (e.g. when it has the SeImpersonatePrivilege
privilege), a named pipe server can impersonate a named pipe client and
execute in its security context (e.g. open the client user's files).
There's no reason for a winpty debugserver to need this ability, so
restrict it in the client.

It seems unlikely to me that this commit fixes a genuine security issue,
but it also seems like a decent precaution to take.  Clients using winpty
don't connect to the DebugServer named pipe unless the WINPTY_DEBUG
environment variable is set, and Windows restricts the ability to
impersonate named clients (at least as of XP SP2).
This commit is contained in:
Ryan Prichard 2016-04-04 06:22:35 -05:00
parent 977019785c
commit a9661c8f52

View File

@ -28,17 +28,40 @@
#include "winpty_snprintf.h"
const wchar_t *const kPipeName = L"\\\\.\\pipe\\DebugServer";
void *volatile g_debugConfig;
static void sendToDebugServer(const char *message)
{
char response[16];
DWORD responseSize;
CallNamedPipeA(
"\\\\.\\pipe\\DebugServer",
(void*)message, strlen(message),
response, sizeof(response), &responseSize,
NMPWAIT_WAIT_FOREVER);
HANDLE tracePipe = INVALID_HANDLE_VALUE;
do {
// The default impersonation level is SECURITY_IMPERSONATION, which allows
// a sufficiently authorized named pipe server to impersonate the client.
// There's no need for impersonation in this debugging system, so reduce
// the impersonation level to SECURITY_IDENTIFICATION, which allows a
// server to merely identify us.
tracePipe = CreateFileW(
kPipeName,
GENERIC_READ | GENERIC_WRITE,
0, NULL, OPEN_EXISTING,
SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION,
NULL);
} while (tracePipe == INVALID_HANDLE_VALUE &&
GetLastError() == ERROR_PIPE_BUSY &&
WaitNamedPipe(kPipeName, NMPWAIT_WAIT_FOREVER));
if (tracePipe != INVALID_HANDLE_VALUE) {
DWORD newMode = PIPE_READMODE_MESSAGE;
SetNamedPipeHandleState(tracePipe, &newMode, NULL, NULL);
char response[16];
DWORD actual = 0;
TransactNamedPipe(tracePipe,
const_cast<char*>(message), strlen(message),
response, sizeof(response), &actual, NULL);
CloseHandle(tracePipe);
}
}
// Get the current UTC time as milliseconds from the epoch (ignoring leap