EsotericSoftware / kryonet

TCP/UDP client/server library for Java, based on Kryo
BSD 3-Clause "New" or "Revised" License
1.82k stars 419 forks source link

Prone to BufferOverflow #7

Closed ghost closed 10 years ago

ghost commented 10 years ago

From Nephir...@gmail.com on August 10, 2010 00:30:30

http://code.google.com/p/kryonet/source/browse/trunk/kryonet/src/com/esotericsoftware/kryonet/TcpConnection.java#197 From there, and especially on line 199: writeBuffer.put(tempWriteBuffer); No check on Buffer overflow is done. This problem probably does not occur much since it uses 16000 byte buffers by default.

Original issue: http://code.google.com/p/kryonet/issues/detail?id=6

ghost commented 10 years ago

From nathan.s...@gmail.com on August 09, 2010 16:06:27

Thanks for taking a look. That piece of code is very core, so it is good to have eyes on it.

Here's a quick overview of that piece of code: objects are serialized to the "temp write buffer" which is then written to socket. Almost always this will succeed, but if not, the temp buffer is copied to the larger "write buffer" to be written later. This way the ideal scenario doesn't have any extra memory copies. Also this allows us to write the length of serialized object before the object without any extra memory copies.

I've added exception handling for the write buffer overflow. I rethrow a SerializationException, which will cause the connection to be closed. This is much better than a server crash, which was the previous behavior.

Thanks!

Status: Fixed
Owner: nathan.sweet