Skip to content

Commit

Permalink
fix: DoS vector in kcp accept (#469)
Browse files Browse the repository at this point in the history
* fix: DoS vector in accept

Previously,  if a client sends a hello and nothing else,
the server waits to complete the handshake before accepting other connections.
This opens a very easy to exploit DoS vector.

Now,  the server fires a separate task for completing the handshake for each client.
So if a client does not cooperate with the handshake, other clients can still be accepted

* use else instead of return
  • Loading branch information
paulpach committed Nov 3, 2020
1 parent cecf5ad commit 6964bc6
Showing 1 changed file with 25 additions and 13 deletions.
38 changes: 25 additions & 13 deletions Assets/Mirror/Runtime/Transport/Kcp/KcpTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,33 @@ void RawInput(EndPoint endpoint, byte[] data, int msgLength)
if (channel != Channel.Reliable)
return;

// add it to a queue
connection = new KcpServerConnection(socket, endpoint, delayMode);
acceptedConnections.Writer.TryWrite(connection);
connectedClients.Add(endpoint as IPEndPoint, connection);
connection.Disconnected += () =>
{
connectedClients.Remove(endpoint as IPEndPoint);
};
// fire a separate task for handshake
// that way if the client does not cooperate
// we can continue with other clients
ServerHandshake(endpoint, data, msgLength).Forget();
}
else
{
connection.RawInput(data, msgLength);
}
}

private async UniTaskVoid ServerHandshake(EndPoint endpoint, byte[] data, int msgLength)
{
var connection = new KcpServerConnection(socket, endpoint, delayMode);
connectedClients.Add(endpoint as IPEndPoint, connection);

connection.Disconnected += () =>
{
connectedClients.Remove(endpoint as IPEndPoint);
};

connection.RawInput(data, msgLength);

await connection.HandshakeAsync();

// once handshake is completed, then the connection has been accepted
acceptedConnections.Writer.TryWrite(connection);
}

private readonly HashSet<HashCash> used = new HashSet<HashCash>();
Expand Down Expand Up @@ -153,11 +169,7 @@ public override async UniTask<IConnection> AcceptAsync()

try
{
KcpServerConnection connection = await acceptedConnections.Reader.ReadAsync();

await connection.HandshakeAsync();

return connection;
return await acceptedConnections.Reader.ReadAsync();
}
catch (ChannelClosedException)
{
Expand Down

0 comments on commit 6964bc6

Please sign in to comment.