PocketMine-MP: Player entities can still die and drop items in flaggedForDespawn state

Summary

When an entity dies, the entity is flagged for despawn, but remains in the World's entity table, meaning it's still accessible by doing World->getEntity($entityId) and other methods. The same is true of a player when quitting the server.

When a network packet arrives from a client to attack an entity, the handler fetches the entity using World->getEntity($entityId) without any checks if the entity is already marked for despawning. Depending on the timing, the entity in question might already be in the flagged-for-despawn state when the action is processed. This means that the death handler for the entity might be run multiple times, causing loot and XP to be dropped multiple times, among other potential side effects.

Reproducing steps

To reproduce this vulnerability, two clients (Player A and Player B) are required.

Prerequisites: - Player A (Victim): Must have the valuable items to be duplicated in their inventory and 1 HP (to ensure instant death). - Player B (Attacker): Must be equipped with a weapon capable of dealing at least 1 damage.

Steps: 1. Player A and Player B stand next to each other. 2. Player A initiates the disconnect sequence (e.g., clicking "Disconnect" or "Exit to Menu"). 3. Immediately after Player A triggers the disconnect (within a split-second window), Player B must attack and kill Player A. 4. Player A's character dies server-side, and their inventory drops on the ground. 5. Player B collects the dropped items. 6. Player A logs back into the server. 7. Result: Player A still possesses the original items in their inventory, while Player B holds the dropped copies.

Patches

The issue was fixed in https://github.com/pmmp/PocketMine-MP/commit/c0719b76b18f2508143134e79bc9f1aa39109683 by adding checks for flagged-for-despawn entities in several affected locations.

While a cleaner fix would be to have World's various entity accessing methods exclude flagged-for-despawn entities, this was deemed too risky for 5.x as it would require significant internal changes.

Workarounds

Plugins can mitigate this issue on older versions by handling EntityDamageByEntityEvent, checking if the victim entity is flagged for despawn, and if so, cancelling the event.