Allow certain IP addresses to run a C++ program

I coded an executable program (.exe) that I only want run either from my home computer, our main server, or people in our development team.

I have coded logic that will only allow the program to be run from certain IP addresses.

The .txt file that's referenced in the getIPlist function looks like this:

[123.456.78.90] = Main Server
[77.34.555.392] = My computer at home
[333.455.3.3] = Assistant coder


And the HTTP address that the parseAllowableIPs function points to is a PHP file that is simply coded:




This code is working great (except for a bit of memory not being freed), so a review or some efficiency tips are welcome.

`bool compareIPs(std::string IPlist, std::string userIP)
{
const char *U = userIP.c_str();
char str[20];

const char *goodList[25] = { '\0' };

std::string uIP = "";

while (*U)
{
if (atoi(U))
{
uIP += itoa(atoi(U), str, 10);
while (U && U != '.')
*U++;

if (U && U == '.')
uIP += ".";
}

if (*U)
*U++;
}

const char *L = IPlist.c_str();
int count = 0;
std::string thisIP = "";

while (*L)
{
switch (*L)
{
case '[':
*L++;

while (L && L != ']')
{
thisIP += *L;
*L++;
}

goodList[count] = _strdup(thisIP.c_str());
thisIP = "";
count++;
break;
default:
break;
}

if (L && L != '[')
*L++;
}

std::string comp = "";

// Now check to see if the user's IP matches any in the goodList[]
for (int a = count; a >= 0; a--)
{
if (!goodList[a])
continue;

comp = goodList[a];

if (!uIP.compare(comp))
{
// I need to free() what was _strdup()'d
// but this causes the progam to crash.

//free(&good

It took me a while to figure out what the code was trying to do. I guess that http://www.myWebServer.com/ip/index.php serves a purpose similar to http://www.whatismyip.com, echoing the HTTP client's IP address. That is better than just enumerating the IP addresses on the client machine's interfaces, as it ensures that you pick an address that is actually routable.

Like all DRM schemes, this one is crackable too. One simple workaround is to use a transparent HTTP proxy that intercepts the call-home message and fakes a response. Using HTTPS with server certificate validation would help prevent that man-in-the-middle attack.

The parseAllowableIPs() function is misnamed. It's not just parsing — it's also fetching, reading, and making an allow/deny decision. A more appropriate name would be isClientIPAuthorized().

compareIPs() is doing more than its fair share. Instead of returning a std::string, getIPlist() should also split the text into a std::vector. There should also be a getMyIP() function. Then, compareIPs() would be a simple verification of whether an item is in a list.

In summary, the outline should be more like:

bool isClientIPAuthorized()
{
    std::string me = getMyIP();
    std::vector allowed = getAllowableIPs();
    return std::find(allowed.begin(), allowed.end(), me) != allowed.end();
}

int main()
{
    //... blah blah blah

    // If not an authorized IP, just exit the game.
    if (!isClientIPAuthorized())
    {
        MySQL__disconnect();
        exit(588924);
    }
}

However, I'd go a step further. The dotted-quad parsing code in compareIPs() has some minor issues, such as superfluous pointer dereferencing. (Note that it would be better to write U++; rather than *U++;, even though the effect is the same.) It would be better to avoid reinventing the wheel, and using a standard IP address parsing routine. (It's such a common task, a few minutes with Google and MSDN is worth it.) There's the Winsock function inet_aton(), which parses a IPv4 dotted-quad address into a long, and the newer InetPton() that also handle IPv6. So, getMyIP() and getAllowableIPs() should probably return long and std::vector, respectively, instead of strings.

StackExchange Code Review Q#74879, answer score: 2