Download website source through Socks4 using Winsock

Question

I just have started to introduce myself into network programming using C++. So I started with Winsock. The code I made is compiled with MinGW and works perfectly!

As a beginner, the main purpose of code was to do what I wanted. Now it's time to go to next level!

The program will download a webpage source using a given socket and the website address/ip and port. After the connection to socks4 is established, the program sends a packet telling to associate with destination server. Now a "GET" request is send to sock and it return the response from server.

So far so good and we have the following code: main.cpp

#include <winsock.h>
#include <string>
#include <iostream>

#include "util.hpp"

using namespace std;

int main(void)
{   
    //Socks4 info
    u_short sockPort = 1080;
    std::string sockIp = "xx.xx.xx.xx";

    //Destination info
    u_short destPort = 80;
    std::string destIPorURL = "checkip.dyndns.com";

    WSADATA wsaData;
    if (WSAStartup(MAKEWORD(2,0), &wsaData)==0)
    {
        if (LOBYTE(wsaData.wVersion) < 2)
        {
            cout << "WSA Version error!";
            return -1;
        }
    }
    else
    {
        cout << "WSA Startup Failed";
        return -1;
    }

    ///////////////////////////////////////////////////////////////////////////////////////////
    //Init socket for socks4
    cout << "Initialize sock_addr for socks4 connection...";
    sockaddr_in sock;
    sock.sin_family = AF_INET;                      // host byte order
    sock.sin_port = htons( sockPort );              // short, network byte order
    if(!utils::getHostIP(sock.sin_addr.S_un.S_addr, sockIp)) // Write ip address in the right format
    {
        cout << "fail";
        return -1;
    }
    cout << "done" << endl;

    //Creating socket handler
    cout << "Creating socket handler...";
    SOCKET hSocketSock = INVALID_SOCKET;
    if( (hSocketSock = socket(AF_INET, SOCK_STREAM, 0)) == INVALID_SOCKET )
    {
        cout << "fail";
        return -1;
    }
    cout << "done" << endl;

    /////////////////////////////////////////////////////////////////////////////////////////////
    //Init socket for destination server
    cout << "Initialize sock_addr for destination server...";
    sockaddr_in dest;
    dest.sin_family = AF_INET;
    dest.sin_port = htons( destPort );
    if(!utils::getHostIP(dest.sin_addr.S_un.S_addr, destIPorURL)) // Write ip address in the right format
    {
        cout << "fail";
        return -1;
    }
    memset( &(dest.sin_zero), '\0', 8 );
    cout << "done" << endl;

    ////////////////////////////////////////////////////////////////////////////////////////////////
    //Time to make connection to socks
    cout << "Connecting to sock server...";
    if(connect(hSocketSock, reinterpret_cast<sockaddr *>(&sock), sizeof(sock)) != 0)
    {
        cout << "failed";
        return -1;
    }
    cout << "done" << endl;

    //We are connected now to our socket!
    //All we have to do is to send our desires.
    //From now, the things differ from SOCKS4 to SOCKS5.
    //So, the code below apply only for SOCKS4!

    //So, we'll start with SOCKS4
    //Documentation: http://www.openssh.com/txt/socks4.protocol
    //The packet we have to build
    //         +----+----+----+----+----+----+----+----+----+----+....+----+
    //         | VN | CD | DSTPORT |      DSTIP        | USERID       |NULL|
    //         +----+----+----+----+----+----+----+----+----+----+....+----+
    //# of bytes: 1    1      2              4           variable       1


    //This packet is meant to inform socks server who's the destination we want to communicate with.
    char *initPacket = new char[9]; //9 because we don't use auth.

    initPacket[0] = 4; //Sock version we use is 4
    initPacket[1] = 1; //Connect code
    memcpy(initPacket + 2, &dest.sin_port, 2); //Copy port into
    memcpy(initPacket + 4, &dest.sin_addr.S_un.S_addr, 4); //Copy ip address
    initPacket[8] = 0; //No username for auth provided

    //Sending our packet!
    cout << "Sending init packet to socks...";
    if(send(hSocketSock, initPacket, 9, 0) == SOCKET_ERROR )
    {
        cout << "fail";
        return -1;
    }
    cout << "done" << endl;

    //Don't need init packet anymore as we have send it.
    delete[] initPacket;

    //We want a replay. This will tell us if the sock is able to communicate with destination
    char replay[8];          //WHY 8? Because of table below :)
    memset(&replay, 0, 8);

    //Reading the response
    cout << "Reading reaponse from sock...";
    //if(recv(hSocketSock, replay, strlen((const char *)replay), 0) == SOCKET_ERROR)
    if(recv(hSocketSock, replay, 8, 0) == SOCKET_ERROR)
    {
        "fail";
        return -1;
    }
    cout << "done" << endl;

    //         Expected response format:
    //         +----+----+----+----+----+----+----+----+
    //         | VN | CD | DSTPORT |      DSTIP        |
    //         +----+----+----+----+----+----+----+----+
    //# of bytes: 1    1      2              4
    //    VN is the version of the reply code and should be 0. CD is the result
    //    code with one of the following values:
    //
    //    90: request granted
    //    91: request rejected or failed
    //    92: request rejected becasue SOCKS server cannot connect to identd on the client
    //    93: request rejected because the client program and identd report different user-ids.

    //So, we have to check if replay is ok :)
    cout << "Checking replay version code...";
    if(replay[0] != 0)
    {
        cout << "fail - " << (int)replay[0];
        return -1;
    }
    cout << "ok" << endl;

    //Returned code: 90 = access granted
    cout << "Checking replay returned code...";
    if(replay[1] != 90)
    {
        cout << "failed - " << (int)replay[1];
        return -1;
    }
    cout << (int)replay[1] << " - ok" << endl;

    //Those being said, if everithing is ok, we can use @hSocketSock handler to send/recv data.
    //Let's download the content of an webpage:
    std::string headers = "GET " + destIPorURL + " HTTP/1.0\r\nHost: " + utils::getHostFromUrl(destIPorURL) + "\r\n\r\n";

    //Send our request
    cout << endl << "Sending custom request...";
    int sendResult = send(hSocketSock, headers.c_str(), headers.length(), 0);
    if(sendResult == SOCKET_ERROR)
    {
        cout << sendResult <<  "-  failed";
        return -1;
    }
    cout << "done!" << endl;

    std::string fullResp = "";
    char buffer[128];

    cout << "Reading response from server...";
    while(true)
    {
        int retval = recv(hSocketSock, buffer, strlen((const char *)buffer), 0);
        if(retval == 0)
        {
            break;
        }
        else if(retval == SOCKET_ERROR)
        {
            cout << "failed";
            return -1;
        }
        else
        {
            buffer[retval] = 0;
            fullResp +=  buffer;
        }
    }
    cout << "done" << endl;
    cout << "What we have got:" << endl << fullResp;


    //Make clean!
    if(hSocketSock != INVALID_SOCKET)
    {
        closesocket(hSocketSock);
    }
    cout << endl;
    return 0;
}

As you may have seen, the util.hpp contain some namespaces with the following functions used in program (and other useful functions):

std::string getHostFromUrl(std::string &url);
bool getHostIP(unsigned long &ipAddr, std::string urlOrHostnameOrIp);

Don't need a review for this. However I will post content of util.cpp and util.hpp in case someone wants to test.

util.hpp:

#include <winsock2.h>
#include <string>
#include <algorithm>
#include <vector>

namespace utils
{
    std::string getHostFromUrl(std::string &url);
    bool getHostIP(unsigned long &ipAddr, std::string urlOrHostnameOrIp);

    namespace IPAddr
    {
        bool isValidIPv4(std::string &ip);
        std::string reverseIpAddress(std::string ip);
        std::string decimalToDottedIp(unsigned long ip);
        unsigned long stripToDecimal(std::string &ip);
    }

    namespace strings
    {
        std::vector<std::string> split(std::string &s, char delim);
        std::string removeSubstrs(std::string &source, std::string pattern);
    }
};

util.cpp

#include <stdexcept>
#include <iostream>
#include <sstream>
#include <stdio.h>

#include "util.hpp"

#define cout std::cout
#define endl std::endl

/////////////////////////////////////////////////////////////////////////////////////
//   _   _                                                         _   _ _
//  | \ | | __ _ _ __ ___   ___  ___ _ __   __ _  ___ ___    _   _| |_(_) |___
//  |  \| |/ _` | '_ ` _ \ / _ \/ __| '_ \ / _` |/ __/ _ \  | | | | __| | / __|
//  | |\  | (_| | | | | | |  __/\__ \ |_) | (_| | (_|  __/  | |_| | |_| | \__ \
//  |_| \_|\__,_|_| |_| |_|\___||___/ .__/ \__,_|\___\___|   \__,_|\__|_|_|___/
//                                  |_|
/////////////////////////////////////////////////////////////////////////////////////

bool utils::getHostIP(unsigned long &ipAddr, std::string url)
{
    HOSTENT *pHostent;
    std::string hostname = getHostFromUrl(url);

    if( utils::IPAddr::isValidIPv4(hostname) )
    {
        //IP Address must be reversed in order to be compatible with sockAddr.sin_addr.S_un.S_addr
        //example: 192.168.1.2 => 2.1.168.192
        hostname = utils::IPAddr::reverseIpAddress(hostname);
        ipAddr =  utils::IPAddr::stripToDecimal(hostname);
        return true;
    }

    if (!(pHostent = gethostbyname(hostname.c_str())))
    {
        return false;
    }

    if (pHostent->h_addr_list && pHostent->h_addr_list[0])
    {
        ipAddr = *reinterpret_cast<unsigned long *>(pHostent->h_addr_list[0]);
        return true;
    }
    return false;
}

std::string utils::getHostFromUrl(std::string &url)
{
    std::string urlcopy = url;

    urlcopy = utils::strings::removeSubstrs(urlcopy, "http://");
    urlcopy = utils::strings::removeSubstrs(urlcopy, "www.");
    urlcopy = utils::strings::removeSubstrs(urlcopy, "https://");
    urlcopy = urlcopy.substr(0, urlcopy.find("/"));

    return urlcopy;
}

//   ___  ____        _        _      _
// | _ _||  _ \      / \    __| |  __| | _ __  ___  ___  ___
//   | | | |_) |    / _ \  / _` | / _` || '__|/ _ \/ __|/ __|
//   | | |  __/    / ___ \| (_| || (_| || |  |  __/\__ \\__ \
//  |___||_|      /_/   \_\\__,_| \__,_||_|   \___||___/|___/

bool utils::IPAddr::isValidIPv4(std::string &ipv4)
{
    const std::string address = ipv4;

    std::vector<std::string> arr;
    int k = 0;
    arr.push_back(std::string());
    for (std::string::const_iterator i = address.begin(); i != address.end(); ++i)
    {
        if (*i == '.')
        {
            ++k;
            arr.push_back(std::string());
            if (k == 4)
            {
                return false;
            }
            continue;
        }
        if (*i >= '0' && *i <= '9')
        {
            arr[k] += *i;
        }
        else
        {
            return false;
        }
        if (arr[k].size() > 3)
        {
            return false;
        }
    }

    if (k != 3)
    {
        return false;
    }
    for (int i = 0; i != 4; ++i)
    {
        const char* nPtr = arr[i].c_str();
        char* endPtr = 0;
        const unsigned long a = ::strtoul(nPtr, &endPtr, 10);
        if (nPtr == endPtr)
        {
            return false;
        }
        if (a > 255)
        {
            return false;
        }
    }
    return true;
}

std::string utils::IPAddr::reverseIpAddress(std::string ip)
{
    std::vector<std::string> octeti = utils::strings::split(ip, '.');
    return (octeti[3] + "." + octeti[2] + "." + octeti[1] + "." + octeti[0]);
}

unsigned long utils::IPAddr::stripToDecimal(std::string &ip)
{
    unsigned long a,b,c,d,base10IP;
    sscanf(ip.c_str(), "%lu.%lu.%lu.%lu", &a, &b, &c, &d);

    // Do calculations to convert IP to base 10
    a *= 16777216;
    b *= 65536;
    c *= 256;
    base10IP = a + b + c + d;

    return base10IP;
}

std::string utils::IPAddr::decimalToDottedIp(unsigned long ipAddr)
{
    unsigned short a, b, c, d;
    std::ostringstream os ;
    std::string ip = "";

    a = (ipAddr & (0xff << 24)) >> 24;
    b = (ipAddr & (0xff << 16)) >> 16;
    c = (ipAddr & (0xff << 8)) >> 8;
    d = ipAddr & 0xff;

    os << d << "." << c << "." << b << "." << a;
    ip = os.str();

    return ip;
}

//   ____   _          _
//  / ___| | |_  _ __ (_) _ __    __ _  ___
//  \___ \ | __|| '__|| || '_ \  / _` |/ __|
//   ___) || |_ | |   | || | | || (_| |\__ \
//  |____/  \__||_|   |_||_| |_| \__, ||___/
//                               |___/

std::vector<std::string> utils::strings::split(std::string &s, char delim)
{
    std::vector<std::string> elems;

    std::stringstream ss;
    ss.str(s);
    std::string item;
    while (std::getline(ss, item, delim))
    {
        elems.push_back(item);
    }
    return elems;
}

std::string utils::strings::removeSubstrs(std::string &input, std::string pattern)
{
    std::string source = input;
    std::string::size_type n = pattern.length();

    for (std::string::size_type i = source.find(pattern); i != std::string::npos; i = source.find(pattern))
    {
        source.erase(i, n);
    }
    return source;
}

The thing is that I have some plans for some projects which uses sockets and I want to get a good start!

I think that the code I wrote is not so easy to understand. For me it's easy but for the others my not!

Hope it is not to soon to say that but I want to write some professional code. So, be as critical as possible!

PS: Not sure if it helps, but the output looks like the following:

Answer 1

Bug:

char buffer[128];

This buffer is uninitialized (i.e. may contain any values)...

    int retval = recv(hSocketSock, buffer, strlen((const char *)buffer), 0);

So calling strlen on it is a really bad idea! We might get a size of zero, or run past the end of the buffer entirely!!! You need to pass in the length of the buffer minus one, to leave space for the null character you write later.

---

using namespace std;

Avoid doing this. It leads to name collisions and other issues.

---

int main(void)

Putting void in the parameter list is a hold-over from C. For C++ we can just leave the list empty.

---

//Socks4 info
u_short sockPort = 1080;
std::string sockIp = "xx.xx.xx.xx";

//Destination info
u_short destPort = 80;
std::string destIPorURL = "checkip.dyndns.com";

Variables should generally be declared where they are used. These are only needed much later, so declare them later.

---

WSADATA wsaData;
if (WSAStartup(MAKEWORD(2,0), &wsaData)==0)
{
    if (LOBYTE(wsaData.wVersion) < 2)
    {
        cout << "WSA Version error!";
        return -1;
    }
}
else
{
    cout << "WSA Startup Failed";
    return -1;
}

This should be a separate function to simplify main(). Winsock 2.2 is the current version, and is supported back to Windows 95 (with appropriate updates), so there is no reason not to use it.

---

sockaddr_in sock;
sock.sin_family = AF_INET;                      // host byte order
sock.sin_port = htons( sockPort );              // short, network byte order
if(!utils::getHostIP(sock.sin_addr.S_un.S_addr, sockIp)) // Write ip address in the right format
{
    cout << "fail";
    return -1;
}
cout << "done" << endl;

getaddrinfo exists! We don't need to do any manual string parsing, splitting, reversing, etc.

---

cout << "Creating socket handler...";
SOCKET hSocketSock = INVALID_SOCKET;
if( (hSocketSock = socket(AF_INET, SOCK_STREAM, 0)) == INVALID_SOCKET )
{
    cout << "fail";
    return -1;
}
cout << "done" << endl;

It's definitely worth writing a minimal Socket class so that we get correct automatic clean up of our socket when it goes out of scope.

---

char *initPacket = new char[9]; //9 because we don't use auth.

Avoid manual memory management where possible. We could surely use an array on the stack for 9 chars (preferably std::array<char, 9> instead of a C-style array). We could also define a struct with names and a serialize function, so that our code might look more like:

auto packet = s4_init
{
    .version = 4,
    .code = 1,
    .dst_port = 80,
    .dst_ip = ... // not sure off-hand what the correct type is be here...
    .user_id = 0,
};

auto send_buffer = serialize(packet);
auto bytes_sent = socket.send(send_buffer.begin(), send_buffer.size()));

// ...

send returns the number of bytes sent, which might be smaller than the size of the buffer (i.e. not everything got sent in one go). So we need to check the return value more thoroughly instead of just looking for SOCKET_ERROR.

---

The recv code has similar issues. Note the spelling error replay vs reply. Again, it might look nicer as something roughly like:

auto recv_buffer = std::array<char, 8>();
auto bytes_recv = socket.receive(packet_buffer.begin(), packet_buffer.size());
// ... error checking
auto reply = deserialize_s4_reply(recv_buffer);

// ... now we can check reply.version, reply.code etc. instead of unnamed array fields.