Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DoS vulnerabilities causing unresponsive servers #1

Merged
merged 2 commits into from
Oct 8, 2016

Conversation

dktapps
Copy link
Member

@dktapps dktapps commented Oct 7, 2016

DoSing a server with empty packets will currently cause the server to become unresponsive for hours, not allowing anyone to connect. The occasional client may connect successfully, but lose connection within minutes.

EDIT: Further investigation revealed that the same bug also occurs with UNCONNECTED_PING and UNCONNECTED_PONG. These vulnerabilities have also been patched.

Explanation

The SessionManager reads packets from sockets every 20th of a second. readPacket() returns false for packets with a zero-length buffer, causing no more packets to be processed on that tick.
100,000 packets -> 100,000 ticks -> 83 minutes of downtime.

This patch fixes that by returning true for empty packets, allowing packets to continue being processed on the same tick after empty packets are processed.

Changes

  • Removed a useless empty packet check (strlen > 0 makes that redundant)
  • Add empty packet check in a different place

Tests

Tested with Packet Generator with 2000 packets @ 50 times, both before and after patch.

  • Before patch: server does not respond for over an hour
  • After patch: server does not respond for ~1 second (5000 packets/tick limit)

//ignored
}elseif(($packet = $this->getPacketFromPool($pid)) !== null){
$packet->buffer = $buffer;
$this->getSession($source, $port)->handlePacket($packet);
Copy link
Member

@SOF3 SOF3 Oct 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have time to review thoroughly.

Looks OK if you are sure you're calling this method from the proper thread?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

@dktapps dktapps changed the title Fix server unresponsive when DoSed with empty packets Fix server unresponsive when DoSed with empty, UNCONNECTED_PING or UNCONNECTED_PONG packets Oct 8, 2016
@dktapps dktapps changed the title Fix server unresponsive when DoSed with empty, UNCONNECTED_PING or UNCONNECTED_PONG packets Fix DoS vulnerabilities causing unresponsive servers Oct 8, 2016
@dktapps dktapps merged commit aaef8c6 into master Oct 8, 2016
@dktapps dktapps deleted the empty-packet-dos branch October 8, 2016 10:16
dktapps added a commit to iTXTech/Genisys that referenced this pull request Oct 8, 2016
 All users are encouraged to upgrade
immediately. See pmmp/RakLib#1 for details.
dktapps added a commit to ClearSkyTeam/ClearSky that referenced this pull request Oct 8, 2016
See pmmp/RakLib#1 for details. All users are
encouraged to upgrade immediately.
@legoboy0215
Copy link

So in theory, you could take down a server with just an app? That is scary...

@dktapps
Copy link
Member Author

dktapps commented Oct 8, 2016

@legoboy0215 It's no theory. People have been doing this for a long time.

@legoboy0215
Copy link

legoboy0215 commented Oct 8, 2016

Ok, Mind = Blown. Some DoS we can't prevent, some we can.

@SOF3
Copy link
Member

SOF3 commented Oct 8, 2016

Why so much trouble? You just need to DDoS robot clients to a 100-slot server from 100 different IPs. Now let's see if they have the patience and courage to ban all these IPs.

Robot-client-based DoS is easier to prevent, but it is still deadly if you have no admins online.

@ghost
Copy link

ghost commented Oct 10, 2016

Nah just make a custom player like @Falkirk's Spectre plugin

@SOF3
Copy link
Member

SOF3 commented Oct 11, 2016

@KairusDarkSeeker for the last time, Specter only spawns players server-side. It works internally, just like you create another user on your own computer. Other people can't create a specter on your server. Therefore it has absolutely no relationship with DoS, unless you have a strange favour of DoS'ing yourself.

@ghost
Copy link

ghost commented Oct 11, 2016

@SOF3 okay

@legoboy0215
Copy link

This is getting real funny and off-topic.

@HowToPeaZYT HowToPeaZYT mentioned this pull request Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants