poweradminllc / PAExec

Remote execution, like PsExec
523 stars 177 forks source link

Handle leak in error paths in SendRequest() #43

Open JohnLaTwC opened 2 years ago

JohnLaTwC commented 2 years ago

The function SendRequest() creates an unnamed event.

!   HANDLE hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);

    OVERLAPPED ol = {0};
    ol.hEvent = hEvent;

https://github.com/poweradminllc/PAExec/blob/954f83e789223d2b5be59575ce39f6d721b5020c/Remote.cpp#L470

There are several error paths where the SendRequest() function exits early and does not call CloseHandle on the event.

bool SendRequest(LPCWSTR remoteServer, HANDLE& hPipe, RemMsg& msgOut, RemMsg& msgReturned, Settings& settings)
{
...

    HANDLE hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);

    OVERLAPPED ol = {0};
    ol.hEvent = hEvent;

    if (!fSuccess) 
    {
        gle = GetLastError();
        if(ERROR_IO_PENDING != gle)
        {
            Log(StrFormat(L"Error communicating with %s.", remoteServer), gle);
            CloseHandle(hPipe);
            hPipe = INVALID_HANDLE_VALUE;
+           CloseHandle(hEvent);
            return false;
        }
    }

    while(true)
    {
        if(WAIT_OBJECT_0 == WaitForSingleObject(ol.hEvent, 0))
        {
            GetOverlappedResult(hPipe, &ol, &cbWritten, FALSE);
            break;
        }
        if(gbStop)
+       {
+           CloseHandle(hEvent);
            return false;
+       }
        Sleep(100);
    }

    bool bFirstRead = true;
    do 
    { 
        BYTE buffer[16384] = {0};

        ol.Offset = 0;
        ol.OffsetHigh = 0;

        DWORD cbRead = 0;
        // Read from the pipe. 
        fSuccess = ReadFile( 
            hPipe,          // pipe handle 
            buffer,         // buffer to receive reply 
            sizeof(buffer), // size of buffer 
            &cbRead,  // number of bytes read 
            &ol);    

        if(!fSuccess) 
        {
            gle = GetLastError();
            if(ERROR_IO_PENDING != gle)
            {
                Log(StrFormat(L"Error reading response from %s.", remoteServer), gle);
                CloseHandle(hPipe);
                hPipe = INVALID_HANDLE_VALUE;
+               CloseHandle(hEvent);
                return false;
            }
        }

        while(true)
        {
            if(WAIT_OBJECT_0 == WaitForSingleObject(ol.hEvent, 0))
            {
                GetOverlappedResult(hPipe, &ol, &cbRead, FALSE);
                break;
            }
            if(gbStop)
+           {
                return false;
+               CloseHandle(hEvent);
+           }
            Sleep(100);
        }

        if(bFirstRead)
        {
            if(cbRead < sizeof(WORD))
            {
                gle = GetLastError();
                Log(StrFormat(L"Received too little data from %s.", remoteServer), gle);
                CloseHandle(hPipe);
                hPipe = INVALID_HANDLE_VALUE;
+               CloseHandle(hEvent);
                return false;
            }
            msgReturned.SetFromReceivedData(buffer, cbRead);
            bFirstRead = false;
        }
        else
            msgReturned.m_payload.insert(msgReturned.m_payload.end(), buffer, buffer + cbRead);
    } while( msgReturned.m_expectedLen != msgReturned.m_payload.size());

    CloseHandle(hEvent);
    return true; 
}