Search code examples
c++windowsvisual-studionamed-pipesdll-injection

Microsoft VC++, vsnprintf, and Pipes (IO) Bug


I am using a DLL injection on to start the client end of a file pipe, which talks to a server that logs the messages. The problem is that the server only receives a buffer filled with question mark ('?') characters.

Client / Injectable DLL:

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

#define BUFSIZE 1024*1024

HANDLE hPipe;
BOOL   fSuccess = FALSE;
DWORD  cbToWrite, cbWritten, dwMode;
const wchar_t* lpszPipename = TEXT("\\\\.\\pipe\\listen");
char write_buffer[BUFSIZE];

void init()
{
    hPipe = CreateFile(
        lpszPipename,   // pipe name 
        GENERIC_READ |  // read and write access 
        GENERIC_WRITE,
        0,              // no sharing 
        NULL,           // default security attributes
        OPEN_EXISTING,  // opens existing pipe 
        0,              // default attributes 
        NULL);          // no template file 

// The pipe connected; change to message-read mode. 

    dwMode = PIPE_READMODE_MESSAGE;
    fSuccess = SetNamedPipeHandleState(
        hPipe,    // pipe handle 
        &dwMode,  // new pipe mode 
        NULL,     // don't set maximum bytes 
        NULL);    // don't set maximum time 
}
#pragma warning(disable:4996)
void report(const char* frmt, ...)
{
    va_list args;
    va_start(args, frmt);
    vsnprintf(write_buffer, BUFSIZE, frmt, args);
    va_end(args);

    // Send a message to the pipe server. 

    fSuccess = WriteFile(
        hPipe,                  // pipe handle 
        write_buffer,             // message 
        strlen(write_buffer),              // message length 
        &cbWritten,             // bytes written 
        NULL);                  // not overlapped 

    return;
}

Server:

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

#define BUFSIZE 1024*1024

BOOL   fConnected = FALSE;
DWORD  dwThreadId = 0;
HANDLE hPipe = INVALID_HANDLE_VALUE, hThread = NULL;
const wchar_t* lpszPipename = TEXT("\\\\.\\pipe\\listen");

// The main loop creates an instance of the named pipe and 
// then waits for a client to connect to it. When the client 
// connects, a thread is created to handle communications 
// with that client, and this loop is free to wait for the
// next client connect request. It is an infinite loop.

for (;;)
{
    _tprintf(TEXT("\nPipe Server: Main thread awaiting client connection on %s\n"), lpszPipename);
    hPipe = CreateNamedPipe(
        lpszPipename,             // pipe name 
        PIPE_ACCESS_DUPLEX,       // read/write access 
        PIPE_TYPE_MESSAGE |       // message type pipe 
        PIPE_READMODE_MESSAGE |   // message-read mode 
        PIPE_WAIT,                // blocking mode 
        PIPE_UNLIMITED_INSTANCES, // max. instances  
        BUFSIZE,                  // output buffer size 
        BUFSIZE,                  // input buffer size 
        0,                        // client time-out 
        NULL);                    // default security attribute 

    if (hPipe == INVALID_HANDLE_VALUE)
    {
        _tprintf(TEXT("CreateNamedPipe failed, GLE=%d.\n"), GetLastError());
        return -1;
    }

    // Wait for the client to connect; if it succeeds, 
    // the function returns a nonzero value. If the function
    // returns zero, GetLastError returns ERROR_PIPE_CONNECTED. 

    fConnected = ConnectNamedPipe(hPipe, NULL) ?
        TRUE : (GetLastError() == ERROR_PIPE_CONNECTED);

    if (fConnected)
    {
        printf("Client connected, creating a processing thread.\n");

        // Create a thread for this client. 
        hThread = CreateThread(
            NULL,              // no security attribute 
            0,                 // default stack size 
            InstanceThread,    // thread proc
            (LPVOID)hPipe,    // thread parameter 
            0,                 // not suspended 
            &dwThreadId);      // returns thread ID 

        if (hThread == NULL)
        {
            _tprintf(TEXT("CreateThread failed, GLE=%d.\n"), GetLastError());
            return -1;
        }
        else CloseHandle(hThread);
    }
    else
        // The client could not connect, so close the pipe. 
        CloseHandle(hPipe);
}

DWORD WINAPI InstanceThread(LPVOID lpvParam)
// This routine is a thread processing function to read from and reply to a client
// via the open pipe connection passed from the main loop. Note this allows
// the main loop to continue executing, potentially creating more threads of
// of this procedure to run concurrently, depending on the number of incoming
// client connections.
{
    HANDLE hHeap = GetProcessHeap();
    TCHAR* pchRequest = (TCHAR*)HeapAlloc(hHeap, 0, BUFSIZE * sizeof(TCHAR));
    TCHAR* pchReply = (TCHAR*)HeapAlloc(hHeap, 0, BUFSIZE * sizeof(TCHAR));

    DWORD cbBytesRead = 0, cbReplyBytes = 0, cbWritten = 0;
    BOOL fSuccess = FALSE;
    HANDLE hPipe = NULL;

    // Print verbose messages. In production code, this should be for debugging only.
    printf("InstanceThread created, receiving and processing messages.\n");

    // The thread's parameter is a handle to a pipe object instance. 

    hPipe = (HANDLE)lpvParam;

    // Loop until done reading
    while (1)
    {
        // Read client requests from the pipe. This simplistic code only allows messages
        // up to BUFSIZE characters in length.
        fSuccess = ReadFile(
            hPipe,        // handle to pipe 
            pchRequest,    // buffer to receive data 
            BUFSIZE,    // size of buffer 
            &cbBytesRead, // number of bytes read 
            NULL);        // not overlapped I/O 

        // Process the incoming message.
        _tprintf(TEXT("Client Request String:\"%s\"\n"), pchRequest);
    }

    FlushFileBuffers(hPipe);
    DisconnectNamedPipe(hPipe);
    CloseHandle(hPipe);

    HeapFree(hHeap, 0, pchRequest);
    HeapFree(hHeap, 0, pchReply);

    printf("InstanceThread exitting.\n");
    return 1;
}

P.S. If is some how possible to use a debugger with a injectable DLL please let me know!


Solution

  • There are a few problems with this code, which I will come to at the end. First, some code that works. Please note that I have simplified things slightly by making it all into a single application (so that I could easily test it) and by getting rid of the thread, but none of that matters in the context of your question.

    #define _CRT_SECURE_NO_WARNINGS
    
    #include <windows.h> 
    #include <stdio.h> 
    #include <tchar.h>
    #include <strsafe.h>
    
    #define BUFSIZE     1024*1024
    
    const TCHAR* lpszPipename = TEXT("\\\\.\\pipe\\listen");
    char write_buffer [BUFSIZE];
    
    HANDLE init()
    {
        HANDLE hPipe = CreateFile(
            lpszPipename,   // pipe name 
            GENERIC_READ |  // read and write access 
            GENERIC_WRITE,
            0,              // no sharing 
            NULL,           // default security attributes
            OPEN_EXISTING,  // opens existing pipe 
            0,              // default attributes 
            NULL);          // no template file 
    
        if (hPipe == INVALID_HANDLE_VALUE)
        {
            printf ("CreateFile returned error %d\n", GetLastError ());
            return INVALID_HANDLE_VALUE;
        }
    
    // The pipe connected; change to message-read mode. 
    
        DWORD dwMode = PIPE_READMODE_MESSAGE;
        BOOL fSuccess = SetNamedPipeHandleState(
            hPipe,    // pipe handle 
            &dwMode,  // new pipe mode 
            NULL,     // don't set maximum bytes 
            NULL);    // don't set maximum time 
    
        if (!fSuccess)
        {
            printf ("SetNamedPipeHandleState returned error %d\n", GetLastError ());
            CloseHandle (hPipe);
            return INVALID_HANDLE_VALUE;
        }
    
       return hPipe;    
    }
    
    void report(HANDLE hPipe, const char *frmt, ...)
    {
        va_list args;
        va_start(args, frmt);
        _vsnprintf(write_buffer, BUFSIZE, frmt, args);
        va_end(args);
    
        // Send a message to the pipe server. 
    
        DWORD cbWritten;
        BOOL fSuccess = WriteFile(
            hPipe,                              // pipe handle 
            write_buffer,                       // message 
            (DWORD) strlen (write_buffer) + 1,  // message length, including EOS
            &cbWritten,                         // bytes written 
            NULL);                              // not overlapped 
    
        if (!fSuccess)
            printf ("WriteFile returned error %d\n", GetLastError ());
    }
    
    int _tmain (int argc, TCHAR **argv)
    {
        if (argc > 1 && _tcscmp (argv [1], __T ("send")) == 0)
        {
            // send
            HANDLE hPipe = init ();
            if (hPipe != INVALID_HANDLE_VALUE)
            {
                report (hPipe, "A message to you, Rudi");
                CloseHandle (hPipe);
            }
            return 0;
        }
    
        // receive
        for (;;)
        {
            _tprintf(TEXT("\nPipe Server: Main thread awaiting client connection on %s\n"), lpszPipename);
            HANDLE hPipe = CreateNamedPipe(
                lpszPipename,             // pipe name 
                PIPE_ACCESS_DUPLEX,       // read/write access 
                PIPE_TYPE_MESSAGE |       // message type pipe 
                PIPE_READMODE_MESSAGE |   // message-read mode 
                PIPE_WAIT,                // blocking mode 
                PIPE_UNLIMITED_INSTANCES, // max. instances  
                BUFSIZE,                  // output buffer size 
                BUFSIZE,                  // input buffer size 
                0,                        // client time-out 
                NULL);                    // default security attribute 
    
            if (hPipe == INVALID_HANDLE_VALUE)
            {
                printf ("CreateNamedPipe failed, GLE=%d.\n", GetLastError());
                return -1;
            }
    
            // Wait for the client to connect; if it succeeds, 
            // the function returns a nonzero value. If the function
            // returns zero, GetLastError returns ERROR_PIPE_CONNECTED. 
    
            BOOL fConnected = ConnectNamedPipe(hPipe, NULL) ?
                TRUE : (GetLastError() == ERROR_PIPE_CONNECTED);
    
            if (!fConnected)
            {
                 printf ("Error %d connecting named pipe\n", GetLastError());
                 return 255;
            }
    
            printf ("Client connected\n");
            HANDLE hHeap = GetProcessHeap();
            char* pchRequest = (char*) HeapAlloc(hHeap, 0, BUFSIZE);
    
            // Loop until done reading
            while (1)
            {
                // Read client requests from the pipe. This simplistic code only allows messages
                // up to BUFSIZE characters in length.
                DWORD cbBytesRead = 0;
                BOOL fSuccess = ReadFile(
                    hPipe,        // handle to pipe 
                    pchRequest,    // buffer to receive data 
                    BUFSIZE,    // size of buffer 
                    &cbBytesRead, // number of bytes read 
                    NULL);        // not overlapped I/O 
    
                if (!fSuccess)
                    break;
    
                // Process the incoming message.
                printf("Client Request String:\"%s\"\n", pchRequest);
            }
    
            FlushFileBuffers(hPipe);
            DisconnectNamedPipe(hPipe);
            CloseHandle(hPipe);
            HeapFree(hHeap, 0, pchRequest);
        }
    
        return 0;
    }
    

    To run this in "send" mode, specify send on the command line. Otherwise it runs as a server. My server runs for ever. Kill it with Ctrl+C.

    So what was wrong with your code? Well, mostly, it was a bit of a mishmash of ANSI and UNICODE strings. You need to be much more careful with this kind of thing and you also need to calculate buffer sizes appropriately. This is all fixed in the above code, which is why I posted it. Also, in terms of good programming practise:

    • You should check for errors much more thoroughly.
    • As written, the server assumes that the string sent to it is NUL-terminated but the client doesn't send it that way (so I fixed the client).
    • The server needs to break out of its receive loop when the sender closes its end of the pipe.
    • Declare local variables locally! (And pass them around as parameters, when appropriate.) Don't use unnecessary globals.
    • Using #define _CRT_SECURE_NO_WARNINGS is better than explicitly disabling the warning that you get if you don't.

    My code fixes all of these issues. HTH.