C++

Forum

 
Timed delay for each new connection to my server

Edelweise (29)
Hello guys. I have this Open Source game server that I recently started working on. Unexpected, the server gets populated real quick and some angry users tries to TCP ddos my server when they die, since if they crash the server, when it's restarted it will take the whole game back about 2 minutes.

As far as I know they use simple programs as LOIC for example which will send thousands of new TCP connections to the server. The server itself can't process that much connections and crashes.

My idea is to initialize a time waiting function that will set 5 seconds delay to each new connection to my server.

Here's the code for accepting new connection: https://pastebin.com/rNjDFrHd

Thank you for your time!
kbw (9488)
The normal approach is to put something in front of the server, and not modify the server.

So, for example, PF on OpenBSD is an excellect starting point for this sort of thing. You just can't outwit spammers within your application, partly because the abstraction sockets library.
Edelweise (29)
I have another idea for this one. Can you help me with it please ?
Before connection is accepted, client will send dummy packet(byte).
Then I will do something like this in the server:

1
2
3
4
5
6
7
 
char buf[1024];
ZeroMemory(buf, 1024);
int bytesReceived = recv(user_s, buf, 1024, 0);
if (bytesReceived == 0)
 {
 break;
 }


So how about that ?
coder777 (8439)
What you can do is save the ip address in the client data. If the ip addtress appears twice ignore it.

Do you know why the server crashes?
Edelweise (29)
I already have code for limiting the connections per IP, but it doesn't solve the problem. The ddos software make the connections so quick and by the time that server detect and limit the next connection if MAX_CONNECTIONS_PER_IP are reached it already crashes. When I run server via debugger and it crashes because of ddos the debugger says: An Access Violation Seagment Fault Raised in your program. In this code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
 
void split(char *dest, char *message, char char_spliter, int *initial)
{
        char temp[80];
        int i, a = 0;

    	for(i = *initial; i < 100100100; i++)
        {
	        if (message[i] == 0) // check to see if at end of string
	                break;

        	if (message[i] != char_spliter) //check to see if at the split mark
	        {
	        	dest[a] = message[i];
                        a++;
        	}
                else
                        break;
        }

        dest[a] = '\0';          //"cap" return string
	if (message[i] == 0) //return point to continue search
		*initial = i;
	else
		*initial = ++i;
        return;			//return
}


And to be more specific, the dest[a] = message[i]; line.
coder777 (8439)
This looks very error prone. What is this magic number 100100100 good for?

It will crash when a (or i) is out of bounds. I would suggest at least something like this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
 
void split(char *dest, int dst_size, char *message, int msg_size, char char_spliter, int *initial)
{
        char temp[80];
        int i, a = 0;

    	for(i = *initial; (i < msg_size) && (a < (dst_size - 1)); i++)
        {
	        if (message[i] == 0) // check to see if at end of string
	                break;

        	if (message[i] != char_spliter) //check to see if at the split mark
	        {
	        	dest[a] = message[i];
                        a++;
        	}
                else
                        break;
        }

        dest[a] = '\0';          //"cap" return string
	if (message[i] == 0) //return point to continue search
		*initial = i;
	else if(i < msg_size)
		*initial = ++i;
        return;			//return
}
Last edited on
Edelweise (29)
If I use your code, I get a lot of errors, for example:

1
2
3
 
point=0;
split(line_section[0], cur_line, ';', &point);
split(line_section[1], cur_line, ';', &point);


Invalid conversation from char* to int
Too few arguments to function void split(etc, etc...)

I would like to use it, but as I said I did not make this game, somebody else coded it and I don't yet know how the code works. Thus I am not that experienced in C/C++ anyways.

So will you please help me do something a little bit more simple like sending dummy bytes from client to server upon initial connection and then tell server to "request" this number of bytes and if they are not sended to server (e.g connection is not via real client), close the connection. ?
Last edited on
mbozzi (3911)
If I use your code, I get a lot of errors

Obviously if you modify an interface you also need to modify all the places which use the interface.
Last edited on
coder777 (8439)
For instance like so:

split(line_section[0], sizeof(line_section[0]), cur_line, sizeof(cur_line), ';', &point);

Note that is only true if line_section[0]/cur_line are not dynamically created. You need to pass the size of the buffer in order to make the function safe.

So will you please help me do something a little bit more simple like sending dummy bytes from client to server upon initial connection and then tell server to "request" this number of bytes and if they are not sended to server (e.g connection is not via real client), close the connection. ?
Actually that would be not that simple. The problem would be the receive function. It would be an additional protection against invalid clients but it would not prevent the crash.

Line 105 makes your code vulnerable against [d]dos attacks. I can show how to change that. Would you be able to do so?
Edelweise (29)
Line 105 makes your code vulnerable against [d]dos attacks. I can show how to change that. Would you be able to do so?

Thanks for your time dealing with this and yes, I will be able to change the code if you show me how. I understand I have to pass the new msg_size and dst_size variables to the function calls, but I don't seem to understand how.

Actually that would be not that simple. The problem would be the receive function. It would be an additional protection against invalid clients but it would not prevent the crash.

I have contacted the original developer of the game. He said he's not dealing with the code anymore, but he had similar problem before. He said:

all i did was send "hi iam client" from the client to the server before each new connection is accepted which will result in a 13 bytes, then in server look to recv these 13 bytes and if not, close the connection

That way you will only allow connection through a real client only, he said.

EDIT: I added your code and changed all the lines that I received errors on for eg:
split(temp_str[0],sizeof(temp_str[0]),message,sizeof(message),',',&point);

It required me to change a lot of code. A lot of things are dependent on that split function like login,password,charname - information that is sended from client to server and back are using this split function too. When I added your code and I tried to dos the server it didn't crashed! It was fine and running ok I was see that packet names that I am sending to server with the dos tool, but the server didn't crash! However, other things are screwed up like it doesn't accept user and password (says incorrect password) and it doesn't read the databases, since they are for e.g a in-game bot:
0,0,botguild,botname,8,5,200,2000,etc,etc ...

So basically yes, your code works just fine, but the whole server is screwed up :/

EDIT 2: I fixed it. I used your code as another void and added that split method only for the sockets and it worked! Server is not crashing and doing fine. Thank you very much for your time to deal with my problem man! I owe u a big one :>

EDIT 3: As I said, it works and server didn't crash, but still vulnerable to TCP dos attack. It will send so much packets that the server will get unresposible to other traffic. Now I really need to do something to prevent this. I think that time delay of 1 second between each new connection will do the job.

EDIT 4: I am stupid ! The server is displaying a message [ printf("usr-rcv-%d\n", etc); ] The someone is dossing he send packet name (e.g THIS IS DDOS) and server display that message 30000000 times in the cmd window and that's why it goes unresponsible! I removed the message and now it's fine, voila!
Last edited on
coder777 (8439)
So basically yes, your code works just fine, but the whole server is screwed up :/
You must not use sizeof(...) when the buffer provided is dynamically allocated. In this case sizeof(...) will return the size of the pointer. You need to use the size when it was allocated.

The problem wiht line 105 is that it is called with the last connected not matter whether the actual connection was successful. This may lead to a crash (due to the exhaust of system resorces) in case of a [d]dos attack
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
 
    while(1)
    {
bool is_connect = false;

        //accept an user?
        c_temp_addrlen = sizeof(temp_in);
        temp_id = accept(user_s,(struct sockaddr*)  &temp_in, &c_temp_addrlen);
 
        if(temp_id > -1 && active_connections(&(temp_in.sin_addr)) >= MAX_CONNECTIONS_PER_IP)
        {
            printf("disallowed connection, max per ip reached:%d:%s\n", MAX_CONNECTIONS_PER_IP, inet_ntoa(temp_in.sin_addr));
            uni_close_socket(temp_id);
            temp_id = -1;
        }
 
        //if accept fails, try again
        if(temp_id > -1) //got one so lets put him in
        {
            //find an empty slot
            for(i=0;i<MAX_USER_CON;i++)
                if (client_user[i].connected == 0)
            {
                u_long on_mode = 1;
                //async it
#ifdef _WIN32
                ioctlsocket(temp_id, FIONBIO, &on_mode);
#else
                ioctl(temp_id, FIONBIO, &on_mode);
#endif
                //log it off just incase
                user_logoff(i);
 
                //set stuff
                client_user[i].c_id = temp_id;
                client_user[i].c_in = temp_in;
 
                //clean up
                client_user[i].recv_amt = 0;
                client_user[i].recv_data[0] = 0;
                client_user[i].sending_mutex = 0;
 
                //make sure of this
                user_con_id[i] = -1;
 
                //set con_top if required
                if (i > user_con_top) user_con_top = i;
 
                //official
                client_user[i].connected = 1;
 
                //testing
 
                //send shit
                //char recvbuf[34];
                //int buflen = 34;
                //int iresult = recv(user_s, recvbuf, buflen, 0);
 
                con_user_connect(i);
                //printf("asdasd: %d\n");
                printf("usr-connection %d accepted from %s\n",i,inet_ntoa(client_user[i].c_in.sin_addr));

is_connect = true; 

                break;
            }
            //if no slots found then close the good fool
            if(i==MAX_USER_CON)
                uni_close_socket(temp_id);
        }

            //if no slots found then close the good fool
            if(i==MAX_SERVER_CON)
                uni_close_socket(temp_id);
        }
 
if(is_connect)
{
        //now do all of the packet stuff
        for(i=0;i<=user_con_top;i++)
            if(client_user[i].connected)
                handle_socket_receive(&(client_user[i]), i, process_packet_user, user_logoff);
 
        //do main loop too..
        main_loop();
 
        //little pause..
        uni_pause(10);
}
else
{
if(temp_id > -1)
                uni_close_socket(temp_id);
uni_pause(1000); // 1 Second
}
    }

That way you will only allow connection through a real client only, he said.
This can still be done though.
Edelweise (29)
Again, thank you for your time and all the help. I am currently using your code and the server performs just fine, but they found another way to [d]dos my server (why JESUS???) .. It has something to do with the force logout (e.g shut down client via task manager). Again, I asked the original dev about this and he said that it can be fixed by adding delay when user force logout. I can provide you with the functions for logging out. Can you help me do this: If is a forced logout, wait "X" seconds before closing the connection. Also, by that time user shouldn't be able to login.
Last edited on
kbw (9488)
I told you ...

You have to put a firewall in front of your server.
Edelweise (29)
I told you ...

You have to put a firewall in front of your server.


At first the server was on my machine, paid $99 for a premium anti-ddos firewall (didn't worked).
Tried moving it to VPS - didn't worked ...
Took it back to my machine, paid another $120 for a premium firewall - again, it didn't worked
Then now thanks to @coder777 the server is able to handle [d]dos attack.
I think if I manage to fix the logging out procedure they won't be able to do anything anymore.

EDIT: I have a function for user creation limit. It detects when a user was created and the creator IP then it compare upon new creation (if current_ip == this_ip) wait some time (which is for this current user only).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
 
int user_creation_limit ( unsigned long the_addr )
{
	int u;
	int max_time = 0;
	int time_difference;

	//find whoever was the most recent guy to be created with this ip
	for ( u=0;u<=user_max;u++ )
		if ( user[u].the_s_addr_creator == the_addr && user[u].time_created > max_time )
			max_time = user[u].time_created;

	//find how long its been since the most recent user was created
	time_difference = time ( 0 ) - max_time;

	if ( time_difference < 0 ) //time create after current time?
	{
		printf ( "warning!! user created at a time that exists in the future!\n" );
		return 0;
	}
	else if ( time_difference < CREATION_LIMIT_SECONDS )
	{
		return CREATION_LIMIT_SECONDS - time_difference;
	}
	else
	{
		return 0;
	}
}


Then in the create user function:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
void create_user ( int con, char *login, char *password, char *username, char *email )
{
	int i, k, point = 0, len;
	char temp_str[2][51], rtn_str[200];
	int time_until_creation, time_hours, time_minutes, time_seconds;

	time_until_creation = user_creation_limit ( client_user[con].c_in.sin_addr.s_addr );

	if ( time_until_creation > 0 )
	{
		time_hours = time_until_creation / 3600;
		time_minutes = ( time_until_creation / 60 ) - ( time_hours * 60 );
		time_seconds = time_until_creation % 60;

		sprintf ( rtn_str,"2,Please wait another %d hours, %d minutes, and %d seconds before attempting to create another user.", time_hours, time_minutes, time_seconds );
		send_con_user ( con,rtn_str );
		return;
	}

etc, etc, etc .. continue with the creation...


The problem is that for this particular check I have user[u].time_created and I don't have user[u].time_loggedout

Last edited on
kbw (9488)
a premium anti-ddos firewall (didn't worked).
I understand your difficulties. However, attackers typically play around with TCP flags and such. You can process that in a C/C++ server using sockets because the sockets library is a state machine for known good cases of packets. There is no mechanism to deal with what attackers can do.

Firewalls, such as Linux IPFilters, manipulate packets directly. And that's what you need, anything else is a waste of time, and adds unncessary complexity to possibly already complex servers.

From the ZeroMemory call and such, I take it you're running Windows. I can't offer any advice with that. But you'll always be vulnerable without a decent firewall.

Have you considered moving the game server to Linux and using some that technology to protect you?
Last edited on
Edelweise (29)
@kbw I think that moving to linux won't solve my current issue, why ? Well I will tell you what is going on right now.

1. A person opens 30 game clients (all legit, all accepted)
2. Probably using a simple batch command like taskkill /f /im game.exe to insta-close all the clients.
3. Server logs out 30 clients at the same time and get stuck for about 30-60 seconds. It doesn't crash, it just got stuck for a time

This isn't actually a dos attack, it's more like a code-side problem. IPTables won't solve the issue here since it's just a game-leaving action ( closing socket ).

Funny thing is that this only stuck the server if the user logged out while in space (it's a space mmo game). If they are landed at planet, it doesn't stuck at all (very strange).

Anyway I did a little trick: I added Sleep(2000); before each connection is closed while user is in space and it didn't get stuck. However, I can't use Sleep(2000); because it globally sleeps the whole function and I need to find a way to add that delay for the current user (ip) only.
Last edited on
coder777 (8439)
Are there still problems you need help with?
Edelweise (29)
Are there still problems you need help with?


I would like to add that "X" seconds delay before actually close the user socket as described above. Thank you!
Topic archived. No new replies allowed.