7
\$\begingroup\$

I've been working on this project to learn networking and concurrency as well as C++11 practices. I'm just looking for a general code review.

#define _WINSOCK_DEPRECATED_NO_WARNINGS

#include <WinSock2.h>
#include <iostream>
#include <string>
#include <vector>
#include <thread>
#include <mutex>

#pragma comment(lib, "ws2_32.lib")

using namespace std;

typedef enum
{
    NONE,
    T_SCAN
} Thread_Type;

typedef struct
{
    HANDLE      tHandle;
    Thread_Type tType;
    SOCKET      tSock;
} Single_Thread;

typedef struct
{
    SOCKET Sock;
    string IPAddress;
    short int port;
    short int threadNum;
    short int totalThreads;
    string *ptrIP; // Pointer To The IPAdress Above
} Scan_Job;

typedef struct
{
    short int port;
    bool isOpen;
    function<void(Scan_Job hScanJob)> Port_Function;
} Port_Struct;

vector<thread> quenceThreads;   // I Use this to push threads
Single_Thread hThreads[256];

// Generic Banner Grabber
// Works On Any Non SSL Protocol That Begins With A Send()
// Tested On: FTP
void Generic_Recv_Banner_Grabber(Scan_Job sJob)
{
    SOCKET Sock;
    char recvBuf[256];

    // Define Socket & Make Sure Its Valid
    Sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    if (Sock == INVALID_SOCKET)
    {
        // error handling code
    }

    // Setup Connection Struct
    sockaddr_in sockAddr;
    sockAddr.sin_family = AF_INET;
    sockAddr.sin_port = htons(sJob.port);
    sockAddr.sin_addr.S_un.S_addr = inet_addr(sJob.IPAddress.c_str());

    // Connect to the server
    if (connect(Sock, (sockaddr*)(&sockAddr), sizeof(sockAddr)) != 0)
    {
        cout << "Recieved Socket Error While Attempting To Connect To " << sJob.IPAddress << ":" << sJob.port << endl;
    }
    else
    {
        cout << "Successfully Connected To " << sJob.IPAddress << ":" << sJob.port << " To Do Banner Grab!" << endl;
        if (recv(Sock, recvBuf, sizeof(recvBuf), 0) != SOCKET_ERROR)
        {
            cout << "Data: " << recvBuf << endl;
        }
    }

    closesocket(Sock);
}

int Thread_Add(Thread_Type type)
{
    int i;

    for (i = 0; i < 256; i++)
    {
        if (hThreads[i].tHandle == nullptr)
        {
            break;
        }
    }

    if (i == 256)
    {
        i = -1;
    }
    else
    {
        hThreads[i].tType = type;
    }

    cout << "Adding Thread Number: " << i << endl;
    return(i);
}

void Thread_Clear(int num)
{
    hThreads[num].tHandle   = nullptr;
    hThreads[num].tType     = NONE; // Enum For 0

    closesocket(hThreads[num].tSock);
}

int Thread_Check(Thread_Type type)
{
    int i, k = 0;

    for (i = 0; i < 256; i++)
    {
        if (hThreads[i].tType == type)
        {
            k++;
        }
    }

    return(k);
}

HANDLE Thread_Start(LPTHREAD_START_ROUTINE function, LPVOID param, BOOL wait)
{
    DWORD  id = 0;
    HANDLE tHandle;

    tHandle = CreateThread(nullptr, 0, function, (LPVOID)param, 0, &id);

    if (wait)
    {
        WaitForSingleObject(tHandle, INFINITE);
        CloseHandle(tHandle);
    }
    else
    {
        Sleep(30); // Do This Better
    }

    return(tHandle);
}

int Thread_Kill(Thread_Type type)
{
    int i, k = 0;

    for (i = 0; i < 256; i++)
    {
        if (hThreads[i].tType == type)
        {
            TerminateThread(hThreads[i].tHandle, 0);
            Thread_Clear(i);
            k++;
        }
    }

    return(k);
}

// Increment IP Address
void AdvGetNextIPPointer(string *host)
{
    struct in_addr paddr;

    // Convert String To ULONG
    u_long addr1 = inet_addr(host->c_str());

    // Convert ULONG To Network Byte Order
    addr1 = ntohl(addr1);

    // Incremement By 1
    addr1 += 1;

    // Convert Network Byte Order Back To ULONG
    addr1 = htonl(addr1);

    // Convert ULONG Back To in_addr Struct
    paddr.S_un.S_addr = addr1;

    // Convert Back To String
    *host = inet_ntoa(paddr);
}

// This Function Checks If A Remote Port Is Open
bool PortCheck(SOCKET sock, string ip, int port)
{
    struct sockaddr_in  sin;
    unsigned long       blockcmd = 1;

    // Define Socket & Make Sure Its Valid
    sock = socket(AF_INET, SOCK_STREAM, 0);
    if (sock == INVALID_SOCKET)
    {
        cout << "Bad Socket, Was Winsock Initialized?" << endl;
        return(false);
    }

    // Setup Winsock Struct
    sin.sin_family              = AF_INET;
    sin.sin_addr.S_un.S_addr    = inet_addr(ip.c_str());    // Update this function to be safe
    sin.sin_port                = htons(port);

    // Set Socket To Non-Blocking
    ioctlsocket(sock, FIONBIO, &blockcmd);

    // Make Connection
    connect(sock, (struct sockaddr *)&sin, sizeof(sin));

    // Setup Timeout
    TIMEVAL tv;
    tv.tv_sec = 5; // Seconds
    tv.tv_usec = 0;

    // Setup Result Set
    FD_SET  rset;
    FD_ZERO(&rset);
    FD_SET(sock, &rset);

    // Move Pointer
    int i = select(0, 0, &rset, 0, &tv);

    // Close Socket
    closesocket(sock);

    // Return Result
    if (i <= 0) {
        return(false);
    }
    else {
        return(true);
    }
}

// This Is Our Individual Scan Thread
DWORD WINAPI Scan_Thread(LPVOID passedParams)
{
    mutex m;
    IN_ADDR hInetAddress;
    Scan_Job hExpScanJob;

    Scan_Job hScanJob = *static_cast<Scan_Job *>(passedParams);

    cout << "IP: " << hScanJob.IPAddress << " Threads: " << hScanJob.totalThreads << " This Thread: " << hScanJob.threadNum << endl;

    // Define What Ports Trigger What Functions Here
    Port_Struct hPorts[] =
    {
        { 21,   false,  Generic_Recv_Banner_Grabber },
        { 0,    false,  nullptr }
    };

    while (true)
    {
        // Lock Because We Are Altering A Shared Resource
        m.lock();
            AdvGetNextIPPointer(hScanJob.ptrIP); // Increment IP Address
        m.unlock();

        cout << "Checking " << *hScanJob.ptrIP << endl;

        hInetAddress.s_addr = inet_addr(hScanJob.ptrIP->c_str()); // Loading Scan IP Into Needed Struct

        // Set isOpen Flag To False On All Ports Before It Becomes A Meaningful Value
        for (int i = 0; hPorts[i].port != 0; i++)
        {
            hPorts[i].isOpen = false;
        }

        // Check Each Port Using PortCheck() Function
        for (int i = 0; hPorts[i].port != 0; i++)
        {
            hPorts[i].isOpen = PortCheck(hThreads[hScanJob.threadNum].tSock, *hScanJob.ptrIP, hPorts[i].port);
        }

        // Check The PortChecker() Results
        for (int i = 0; hPorts[i].port != 0; i++)
        {
            // Is Port Open?
            if (hPorts[i].isOpen)
            {
                m.lock();
                    cout << "Port Open: " << inet_ntoa(hInetAddress) << ":" << hPorts[i].port << endl;
                    hExpScanJob.IPAddress = *hScanJob.ptrIP;
                    hExpScanJob.port = hPorts[i].port;

                    quenceThreads.push_back(thread(hPorts[i].Port_Function, hExpScanJob));
                m.unlock();
            }
        }
    }

    Thread_Clear(hScanJob.threadNum);
    ExitThread(0);
}

// This Function Starts Our Scan Threads
void Start_Scan(Scan_Job hScanJob)
{
    int rThreads = 0;

    // Check If We Are Already Scanning
    if (Thread_Check(T_SCAN))
    {
        cout << "Scan Is Already Running" << endl;
        return;
    }

    // Create Our Desired Number Of Scan Threads
    for (int i = 0; i < hScanJob.totalThreads; i++)
    {
        hScanJob.threadNum = Thread_Add(T_SCAN);

        if (hScanJob.threadNum != -1)
        {
            if ((hThreads[hScanJob.threadNum].tHandle = Thread_Start(Scan_Thread, &hScanJob, FALSE)) != nullptr)
            {
                rThreads++;
            }
        }
    }
}

// Entry Point
int main()
{
    SOCKET Sock;
    WSADATA WsaDat;
    string testCommand;

    // Initialize Winsock
    if (WSAStartup(MAKEWORD(2, 2), &WsaDat) != 0)
    {
        cout << "Winsock Initialization Failed!" << endl;
        WSACleanup();
        return(0);
    }

    // Define Socket
    Sock = socket(AF_INET, SOCK_STREAM, 0);
    if (Sock == INVALID_SOCKET)
    {
        cout << "Bad Socket, Was Winsock Initialized?" << endl;
        return(0);
    }

    // Create Our Scan Job
    Scan_Job hScanJob;
    hScanJob.IPAddress      = "192.168.0.1";
    hScanJob.totalThreads   = 30;
    hScanJob.ptrIP          = &hScanJob.IPAddress;


    // Launch The Scan Thread
    thread tScanThread(Start_Scan, hScanJob);

    // This Holds Up The Program From Ending
    while (Thread_Check(T_SCAN))
    {
        // cout << "Scan Is Running" << endl;
    }

//  Thread_Kill(T_SCAN); // Kill The Scan Thread
    return(0);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! I hope you get some great answers! \$\endgroup\$ – Phrancis Sep 2 '15 at 3:38
  • \$\begingroup\$ You use CreateThread. Is it your intention to learn threads on Windows? Or your intention is to learn threads in C++11? I am asking since C++11 has thread library and starting VisualStudio 2012 C++11 thread library is implemented on Windows. \$\endgroup\$ – user24597 Sep 2 '15 at 7:39
3
\$\begingroup\$

There is a reason these warnings are there:

#define _WINSOCK_DEPRECATED_NO_WARNINGS

Whatever function is causing this error should be replaced with a modern equivalent.

Don't do this.

using namespace std;

There is a big description of why here: Why is “using namespace std;” considered bad practice?.

From this:

typedef struct
{
    HANDLE      tHandle;
    Thread_Type tType;
    SOCKET      tSock;
} Single_Thread;

I can guess you are using windows threads. Why not use the C++ threads? They are cross platform, and more importantly, they work like C++ threads should (the Windows threading library is C based and thus horrible to use).

Why would you want to retain the address of a member that you have locally?

string *ptrIP; // Pointer To The IPAdress Above

This is "C" code.

typedef enum
{
    NONE,
    T_SCAN
} Thread_Type;

Type names, structure names and enumeration names are all in the same namespace in C++. So there is no need to use a typedef here.

enum Thread_Type
{
    NONE,
    T_SCAN
};

struct Single_Thread
{
    HANDLE      tHandle;
    Thread_Type tType;
    SOCKET      tSock;
};

Now you are using both threading systems:

// Windows threads
tHandle = CreateThread(nullptr, 0, function, (LPVOID)param, 0, &id);

// C++ threads
vector<thread> quenceThreads;   // I Use this to push threads
quenceThreads.push_back(thread(hPorts[i].Port_Function, hExpScanJob));

Pick one (and pick the C++ version).

Not all services listening on a port are going to reply.

    cout << "Successfully Connected To " << sJob.IPAddress << ":" << sJob.port << " To Do Banner Grab!" << endl;
    if (recv(Sock, recvBuf, sizeof(recvBuf), 0) != SOCKET_ERROR)
    {
        cout << "Data: " << recvBuf << endl;
    }

This is just as likely to hang for ever as give you data. I believe that even HTTP connections are going to wait for you to make the first move. You have to send a message before they will reply (I could be wrong on that).

This:

// Return Result
if (i <= 0) {
    return(false);
}
else {
    return(true);
}

is an overly verbose way of saying

 return i > 0;

The number of threads is usually best set to a number close to the number of cores on the machine.

 hScanJob.totalThreads   = 30;

Unless you have some heavy metal here this number is way to big. You should create a couple of threads. Then reuse the threads. So create a job for each port then get your threads to pick up jobs from the job queue.

Admittedly creating a large number of jobs will protect you against time you errors. But thats because you are using threading and sockets incorrectly together. You should open all the sockets in one thread then use select to see who is listening then dispatch readers when you get a connection.

Search for the C10K problem

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.