LEGO® Universe Authentication Server

Please note that all credit for code goes to humanoid24 and I have only helped the bare minimum required to be able to post it here.

I am working with some friends to replicate a server for the game LEGO® Universe which was shut down two years ago. This code is included in the LUNIServer v3 here.

The server does what it's supposed to do, but I'm wondering if anything could be improved. If you have any tips to make this more efficient (not necessarily smaller), please post them. I'll need to change stuff before the server grows larger, making a major change hard to do.

```
/*
Lego Universe Authentification Server WIP
Code done by humanoid24 (aka Triver) as of 01.12.2013

This source code requires RakNet version 3.25 as
an external dependency to work with the luni client.
*/
//#define USE_ENCRYPTION // is actually not required for a connection (but may be useful once login works because of user password)

#include "serverLoop.h"
#include
//#if defined USE_ENCRYPTION
#include "RSACrypt.h"
//#endif

void loadConfig(CONNECT_INFO* cfg) // do a very simple and dirty config loading
{
memset(cfg->redirectIp, 0, sizeof(cfg->redirectIp));
std::ifstream in(".\\config.ini", std::ios::in);
if(in.is_open())
{
std::string temp((std::istreambuf_iterator(in)), std::istreambuf_iterator());
in.close();
size_t pos = temp.find("redirect_ip=", 0)+strlen("redirect_ip=");
size_t end = temp.find_first_of('\n', pos);
if(end > pos)
{
std::string value = temp.substr(pos, end-pos);
if(value.size() redirectIp))
memcpy(cfg->redirectIp, value.c_str(), value.size());
else
printf("specified host exceeds limit of 15!\n");
pos = temp.find("redirect_port=", end)+strlen("redirect_port=");
end = temp.find_first_of('\n', pos);
if(end > pos)
{
cfg->redirectPort = atoi(temp.substr(pos, e

I've only skimmed a little bit of your code, but here's what I found so far:

Always make sure std::string methods do not return std::string::npos.

size_t pos = temp.find("redirect_ip=", 0)+strlen("redirect_ip=");

This can easily lead to undefined behavior. size_t is an unsigned integral type. On some systems, std::string::npos is defined as 0xffffffff. You are guaranteed an overflow if this is the case on your system and your call to find fails.

size_t end = temp.find_first_of('\n', pos);
    if(end > pos)
    {
        std::string value = temp.substr(pos, end-pos);

If end == std::string::npos, then you're going to be reading way out of bounds.

if(value.compare("true") == 0 || value.compare("1") == 0)

There is no reason to use compare here. An std::string's operator== is overloaded to check for equality. This would be better:

if(value == "true" || value == "1")

Your entire structure of nested if statements is ugly to me. I would rather use exceptions instead or return early.

Be consistent with your API. In loadConfig, you use C++-style streams, but in InitSecurity, you use C-style file descriptors. Unless you've profiled your code and decided C++-style streams are too slow, I would stick to those. If you have profiled your code and decided that C-style descriptors give you an adequate boost, then you should document that detail in a comment.

StackExchange Code Review Q#39608, answer score: 3