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);
}
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Sep 2, 2015 at 3:04
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Welcome to Code Review! I hope you get some great answers! \$\endgroup\$ Commented Sep 2, 2015 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\$ Commented Sep 2, 2015 at 7:39

1 Answer 1

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

answered Sep 2, 2015 at 14:49
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.