Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

restrict packet size to 4K in server #3535

Merged
merged 1 commit into from

3 participants

@chrisforbes
Owner
  • Sending a negative length no longer crashes the server
  • Sending very large lengths can't force us to buffer stupid amounts of data

The offending client just gets kicked if they do this.

@chrisforbes chrisforbes restrict packet size to 4K in server
- Sending a negative length no longer crashes the server
- Sending very large lengths can't force us to buffer stupid amounts of data

The offending client just gets kicked if they do this.
edb08d6
@pchote pchote merged commit 9746b53 into OpenRA:bleed
@chrisforbes
Owner

Will need some playtesting to make sure you can't legitimately bust the limit.

@Mailaender Mailaender commented on the diff
OpenRA.Game/Server/Connection.cs
@@ -86,6 +87,13 @@ public void ReadData(Server server)
ExpectLength = BitConverter.ToInt32(bytes, 0) - 4;
Frame = BitConverter.ToInt32(bytes, 4);
State = ReceiveState.Data;
+
+ if (ExpectLength < 0 || ExpectLength > MaxOrderLength)
+ {
+ server.DropClient(this);
+ Log.Write("server", "Dropping client {0} for excessive order length = {1}", PlayerIndex, ExpectLength);
@Mailaender Owner

@Ripsn was hosting and kicked himself when right-click spamming infantry units:

Dropping client 0 for excessive order length = 8811

This needs to be reverted as it is game breaking with the current order code.

@chrisforbes Owner
@Mailaender Owner

We simply reverted it in #3552 as that was faster.

@pchote Owner
pchote added a note

That was just a quick hotfix so we could have a playable build.
I've restored this with a 128k limit in #3561.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 8, 2013
  1. @chrisforbes

    restrict packet size to 4K in server

    chrisforbes authored
    - Sending a negative length no longer crashes the server
    - Sending very large lengths can't force us to buffer stupid amounts of data
    
    The offending client just gets kicked if they do this.
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 1 deletion.
  1. +9 −1 OpenRA.Game/Server/Connection.cs
View
10 OpenRA.Game/Server/Connection.cs
@@ -25,6 +25,7 @@ public class Connection
public int ExpectLength = 8;
public int Frame = 0;
public int MostRecentFrame = 0;
+ public const int MaxOrderLength = 4096;
/* client data */
public int PlayerIndex;
@@ -65,7 +66,7 @@ bool ReadDataInner(Server server)
if (e.SocketErrorCode == SocketError.WouldBlock) break;
server.DropClient(this);
- Log.Write("server", "Dropping client {0} because reading the data failed: {1}", this.PlayerIndex.ToString(), e);
+ Log.Write("server", "Dropping client {0} because reading the data failed: {1}", PlayerIndex, e);
return false;
}
}
@@ -86,6 +87,13 @@ public void ReadData(Server server)
ExpectLength = BitConverter.ToInt32(bytes, 0) - 4;
Frame = BitConverter.ToInt32(bytes, 4);
State = ReceiveState.Data;
+
+ if (ExpectLength < 0 || ExpectLength > MaxOrderLength)
+ {
+ server.DropClient(this);
+ Log.Write("server", "Dropping client {0} for excessive order length = {1}", PlayerIndex, ExpectLength);
@Mailaender Owner

@Ripsn was hosting and kicked himself when right-click spamming infantry units:

Dropping client 0 for excessive order length = 8811

This needs to be reverted as it is game breaking with the current order code.

@chrisforbes Owner
@Mailaender Owner

We simply reverted it in #3552 as that was faster.

@pchote Owner
pchote added a note

That was just a quick hotfix so we could have a playable build.
I've restored this with a 128k limit in #3561.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return;
+ }
} break;
case ReceiveState.Data:
Something went wrong with that request. Please try again.