Decode cost accounting

[dktapps]

It's a well-known issue that arrays in packets are subject to attack. However, the proposed solutions this far have been crude and would easily run into problems with clients, as they usually involve hardcoding list size limits like #216.

I considered setting up configurable limits using loop IDs, but this turned out to be an enormous clusterfuck because there are hundreds of places where lists appear in the protocol, and it's far too much of a burden to try and establish sensible static limits for them all, even if they are configurable.

After mulling this over I realized that it's not directly the size of lists that's problematic (especially considering e.g. lists inside lists, NBT, etc). More so, it's that the number of operations needed to decode the data in the packet blows up.

With this in mind, I propose a cost-based system whereby:

• Atomic operations like getInt, getByte, getByteArray have a cost of 1
• getVarInt has a cost of 1-5 depending on the number of bytes
• getString has a cost of 2-6 (1-5 for its length, plus 1 for byte array following)

This way, PocketMine can set up a dynamic cost buffer for packet processing, similar to how we currently do flexible packets-per-tick limits. This means a session would have a certain budget of packet decode operations allowed, and would be kicked from the server if it causes the server to spend a disproportionate amount of cycles working on its packets.

Advantages

• One limit to rule them all - it could also replace packet count limits too, since it should cost the same to decode several small packets as one big one
• Simple to implement - only changes to BinaryStream and PM core would be needed, and the limit would kick in by throwing BinaryDataException, which would be maximally compatible with existing code
• Allows responding to dynamic events, e.g. allowing us to tolerate a small number of packets with big lists if necessary without just accepting them all the time

Problems

• This system could become problematic if it applied to item NBT decoding, since that could blow up the session's cost budget through no fault of the player (e.g. shulker boxes can have huge NBT).

Why not just use time-based accounting?

Good question. Sure, we could just assign a time budget too, but there are a couple of problems with this:

• Decoding the same packet can take different lengths of real time depending on machine conditions, like scheduling and CPU frequency scaling
• Slower machines wouldn't be able to use the same limits as faster ones, so we'd need some dynamic way to calculate limits

Basing it on the number of operations should be consistent across servers.