Should I catch this case and just close the connection?
Yes. As soon as something is not right, close the connection. Also, log this to some counter, perhaps correlated with client version reported on connection, so that you can tell if you have a bug that causes users to get disconnected because the client sends bad data :-)
Also, put a stringent max size on EVERYTHING. A maximum size on the message type. A maximum size on files downloaded or uploaded. A maximum size on strings. A maximum item count in arrays. And make those limits be fairly limited. This will avoid someone doing things like sending a "valid" packet that claims to have 2,000,000,000 items in it, and thus tying up your sever just receiving and dealing with that packet.
Another popular attack is sending malformed inner data. Say you have a "login" packet that looks something like: name:string, password:string, clientversion:varint.
Now, if strings are zero terminated, then someone can send a valid outer packet (type: login, length: whatever) but then not include a zero byte in the payload. Your decoding function may at that point run past the end of the packet, and read random memory. Don't do that! (Better is to length-prefix strings, and automatically cut them off at 255 bytes max.)
Even if you fix the string, someone might send a badly encoded varint. For example, let's say that varints are encoded with the high bit set when there's a continuation byte, and the high bit clear for the last byte; each lower 7 bits are data, little-endian. For example, the value 0xAA55 would be sent as:
(0xAA55 & 0x7f) | 0x80, ((0xAA55 >> 7) & 0x7f) | 0x80, ((0xAA55 >> 14) & 0x7f)
Now, someone could send a packet that contains the value 0x80 over and over again, causing your decoding logic to never break out of the varint decoding loop. Always establish a maximum length for a varint, or any data type, on the wire.
Once you have the serialization/de-serialization dealt with, the semantics of the packets matter. Don't treat something as "known good." For example, if an entity ID is in the packet, look up that entity ID in a hash table each time you see it. If it's not there, disconnect the player. If a property ID is in the packet, make sure it's valid, and if it's not, disconnect the player. If a floating point value is in the packet, make sure it's not NaN or Inf or denormal -- if it is, disconnect the player.
Edited by hplus0603, 01 April 2013 - 08:37 PM.