Mojira Archive
MC-2840

Chat concurrency issues

Asynchronous chat (chat being executed from a second thread) lends itself to a few theoretical exploits.

1) The variable used for chat spam is not thread-aware (no synchronized locks, nor volatility). It is decremented once per tick, and incremented by twenty per message. Since the accesses are not thread-aware, a client may chat spam with the limiter subject to race conditions. See this fix, applied against Bukkit's decompiled code, https://github.com/Wolvereness/CraftBukkit-Bleeding/commit/fcc4b855acf2356f1694a56726950d5b7c56d104

2) Clients that get disconnected during chat messages (chat too long, illegal characters, or spam) will 'disconnect' the client, accessing single-threaded collections and internals from a secondary thread. The client should instead get disconnected the same way they would for an IO exception and the server will detect the disconnected client on next tick. See this fix, applied against Bukkit's decompiled code, https://github.com/Wolvereness/CraftBukkit-Bleeding/commit/46c84ee90c39731a3f0e9780feba7ff984b27c82

3) Sending the finalized chat packet to all clients is again, subject to race conditions, namely NPEs without iterator, as well as CMEs when using an iterator. This one can cause a client to be kicked when chatting at the same time another player logs out. To fix this, the internal list of players could be a CopyOnWriteArrayList, with an iterator when sending packet to all clients, or only use an iterator for asynchronous chat.

Invalid

Wesley Wolfe

2012-11-12, 08:15 AM

2018-03-27, 06:54 PM

2014-10-24, 11:09 PM

1

4

Confirmed

Minecraft 1.4.2 - Minecraft 1.6Minecraft 1.4.2, Minecraft 1.4.4, Minecraft 1.4.5, Minecraft 1.4.7, Minecraft 1.5, Minecraft 1.6

Minecraft 13w41a