Skip to content

Commit

Permalink
disconnect instead of crashing, if server sends invalid packet
Browse files Browse the repository at this point in the history
  • Loading branch information
UnknownShadow200 committed Aug 14, 2018
1 parent dd140ea commit 79efdcf
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 28 deletions.
2 changes: 2 additions & 0 deletions ClassicalSharp/Network/Enums.cs
Expand Up @@ -50,6 +50,8 @@ public static class Opcode {
public const byte CpeSetEntityProperty = 42;
public const byte CpeTwoWayPing = 43;
public const byte CpeSetInventoryOrder = 44;

public const byte Count = 45;
}
}

Expand Down
33 changes: 14 additions & 19 deletions ClassicalSharp/Network/NetworkProcessor.cs
Expand Up @@ -160,12 +160,20 @@ public sealed class NetworkProcessor : IServerConnection {
continue;
}

if (opcode > maxHandledPacket) {
throw new InvalidOperationException("Invalid opcode: " + opcode);
if (opcode >= handlers.Length) {
game.Disconnect("Disconnected!", "Server sent invalid packet " + opcode + "!"); return;
}

if ((reader.size - reader.index) < packetSizes[opcode]) break;
ReadPacket(opcode);

reader.Skip(1); // remove opcode
lastOpcode = opcode;
lastPacket = DateTime.UtcNow;

Action handler = handlers[opcode];
if (handler == null) {
game.Disconnect("Disconnected!", "Server sent invalid packet " + opcode + "!"); return;
}
handler();
}

reader.RemoveProcessed();
Expand All @@ -186,7 +194,6 @@ public sealed class NetworkProcessor : IServerConnection {
public void Set(byte opcode, Action handler, int packetSize) {
handlers[opcode] = handler;
packetSizes[opcode] = (ushort)packetSize;
maxHandledPacket = Math.Max(opcode, maxHandledPacket);
}

public void SendPacket() {
Expand All @@ -203,17 +210,6 @@ public sealed class NetworkProcessor : IServerConnection {
}
}

void ReadPacket(byte opcode) {
reader.Skip(1); // remove opcode
lastOpcode = opcode;
Action handler = handlers[opcode];
lastPacket = DateTime.UtcNow;

if (handler == null) {
throw new InvalidOperationException("Unsupported opcode: " + opcode);
}
handler();
}

public override void Reset(Game game) {
UsingExtPlayerList = false;
Expand Down Expand Up @@ -244,9 +240,8 @@ public sealed class NetworkProcessor : IServerConnection {
writer.ExtendedPositions = false; writer.ExtendedBlocks = false;
}

internal Action[] handlers = new Action[256];
internal ushort[] packetSizes = new ushort[256];
int maxHandledPacket = 0;
internal Action[] handlers = new Action[Opcode.Count];
internal ushort[] packetSizes = new ushort[Opcode.Count];

void BlockChanged(object sender, BlockChangedEventArgs e) {
Vector3I p = e.Coords;
Expand Down
21 changes: 13 additions & 8 deletions src/Client/ServerConnection.c
Expand Up @@ -248,7 +248,6 @@ struct Stream net_writeStream;
UInt8 net_readBuffer[4096 * 5];
UInt8 net_writeBuffer[131];

Int32 net_maxHandledPacket;
bool net_writeFailed;
Int32 net_ticks;
DateTime net_lastPacket;
Expand Down Expand Up @@ -276,7 +275,7 @@ static void MPConnection_FinishConnect(void) {
Event_RaiseReal(&WorldEvents_Loading, 0.0f);

String streamName = String_FromConst("network socket");
Stream_ReadonlyMemory(&net_readStream, net_readBuffer, sizeof(net_readBuffer), &streamName);
Stream_ReadonlyMemory(&net_readStream, net_readBuffer, sizeof(net_readBuffer), &streamName);
Stream_WriteonlyMemory(&net_writeStream, net_writeBuffer, sizeof(net_writeBuffer), &streamName);

net_readStream.Meta.Mem.Left = 0; /* initally no memory to read */
Expand Down Expand Up @@ -435,15 +434,23 @@ static void MPConnection_Tick(struct ScheduledTask* task) {
continue;
}

if (opcode > net_maxHandledPacket) { ErrorHandler_Fail("Invalid opcode"); }
if (opcode >= OPCODE_COUNT) {
String title = String_FromConst("Disconnected");
String msg = String_FromConst("Server sent invalid packet!");
Game_Disconnect(&title, &msg); return;
}
if (net_readStream.Meta.Mem.Left < Net_PacketSizes[opcode]) break;

Stream_Skip(&net_readStream, 1); /* remove opcode */
net_lastOpcode = opcode;
Net_Handler handler = Net_Handlers[opcode];
DateTime_CurrentUTC(&net_lastPacket);

if (!handler) { ErrorHandler_Fail("Unsupported opcode"); }
Net_Handler handler = Net_Handlers[opcode];
if (!handler) {
String title = String_FromConst("Disconnected");
String msg = String_FromConst("Server sent invalid packet!");
Game_Disconnect(&title, &msg); return;
}
handler(&net_readStream);
}

Expand All @@ -470,7 +477,6 @@ static void MPConnection_Tick(struct ScheduledTask* task) {
void Net_Set(UInt8 opcode, Net_Handler handler, UInt16 packetSize) {
Net_Handlers[opcode] = handler;
Net_PacketSizes[opcode] = packetSize;
net_maxHandledPacket = max(opcode, net_maxHandledPacket);
}

void Net_SendPacket(void) {
Expand All @@ -485,7 +491,7 @@ void Net_SendPacket(void) {
}
}

net_writeStream.Meta.Mem.Cur = net_writeStream.Meta.Mem.Base;
net_writeStream.Meta.Mem.Cur = net_writeStream.Meta.Mem.Base;
net_writeStream.Meta.Mem.Left = net_writeStream.Meta.Mem.Length;
}

Expand Down Expand Up @@ -524,7 +530,6 @@ static void MPConnection_Reset(void) {
}

net_writeFailed = false;
net_maxHandledPacket = 0;
Handlers_Reset();
ServerConnection_Free();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Client/ServerConnection.h
Expand Up @@ -53,6 +53,8 @@ enum OPCODE_ {
OPCODE_CPE_SET_ENTITY_PROPERTY,
OPCODE_CPE_TWO_WAY_PING,
OPCODE_CPE_SET_INVENTORY_ORDER,

OPCODE_COUNT,
};

struct PickedPos;
Expand Down Expand Up @@ -90,7 +92,6 @@ void ServerConnection_InitMultiplayer(void);
void ServerConnection_MakeComponent(struct IGameComponent* comp);

typedef void (*Net_Handler)(struct Stream* stream);
#define OPCODE_COUNT 45
UInt16 Net_PacketSizes[OPCODE_COUNT];
Net_Handler Net_Handlers[OPCODE_COUNT];
void Net_Set(UInt8 opcode, Net_Handler handler, UInt16 size);
Expand Down

0 comments on commit 79efdcf

Please sign in to comment.