Skip to content

fix: block placement validation, handshake null safety, processQueue atomic polling, and error propagation - #164

Open
GamingOP69 wants to merge 6 commits into
Wind-Development:masterfrom
GamingOP69:fix/protocol-safety-and-handshake
Open

fix: block placement validation, handshake null safety, processQueue atomic polling, and error propagation#164
GamingOP69 wants to merge 6 commits into
Wind-Development:masterfrom
GamingOP69:fix/protocol-safety-and-handshake

Conversation

@GamingOP69

Copy link
Copy Markdown

Summary

Adds packet bounds validation for block placement, null guards in PlayerHandshakeEvent / HandshakeListener, atomic polling in processQueue, and prevents JVM Error suppression.

Problem & Root Cause

  • Malformed PacketPlayInBlockPlace packets containing invalid face directions (not in 0..5, 255) or negative Y coordinates were passed directly into block shift calculations, causing server crashes.
  • When proxy authentication plugins (such as TCPShield or BungeeGuard) forwarded empty or unexpected handshake headers, HandshakeListener threw NPE when reading socket addresses or profile properties.
  • MinecraftServer polled processQueue using .remove() after checking !isEmpty(), which is not thread-safe. Additionally, catching raw Throwable suppressed JVM critical errors (such as OutOfMemoryError or StackOverflowError), leaving the server in an unrecoverable corrupted state.
  • AntiCrash threw NPE on custom payload packets with null sub-channels.

Solution

  • Added explicit bounds checks in PlayerConnection.a(PacketPlayInBlockPlace) for null block position, null direction, and negative Y coordinates.
  • Added null safety checks in HandshakeListener and PlayerHandshakeEvent.
  • Switched processQueue polling to poll() with null check, and narrowed the catch block to Exception so critical JVM Error instances propagate correctly.
  • Added null validation in AntiCrash packet inspection.
  • Added unit tests: BlockPlaceValidationTest, PlayerHandshakeEventTest, PlayerHandshakeEventRegressionTest, and ProcessQueueSafetyRegressionTest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant