Fix GS4 query crash and validate login usernames#1849
Conversation
|
These should be seperate PRs, and the 2nd fix breaks stuff like geyser |
Check readableBytes before reading the query packet header to prevent IndexOutOfBoundsException log spam from malformed datagrams shorter than 7 bytes.
aff2bf4 to
91967d6
Compare
|
Done — removed the username validation. This PR is now just the GS4 query fix. |
|
Please update the PR description accordingly. And throwing the second part of this PR away could be a shame, if you think it's still relevant, I'd encourage you to open it under a second PR (but do see my comment on that PR.. this would likely be a breaking change so I doubt it'd be useful). Other than "testing" it by compiling & running unit tests, were you able to verify that this indeed fixes the referenced issue? The issue contains a MRE in the form of a command to run. One issue still: |
Summary
Two small defensive fixes for the proxy's network layer.
1. Guard GameSpyQueryHandler against short datagrams
GameSpyQueryHandler.channelRead0() reads magic bytes, type, and sessionId
without first checking readableBytes(). A malformed UDP datagram shorter
than 7 bytes causes IndexOutOfBoundsException on every packet, spamming
the log at WARNING level.
This adds a readableBytes() >= 7 guard at the start so short datagrams
are silently dropped instead.
2. Validate usernames in ServerLoginPacket
In offline mode, ServerLoginPacket.decode() accepts any UTF-8 content
as a username. This username becomes the offline UUID seed and is
forwarded to the backend as-is, enabling log injection and identity
confusion.
This adds a [A-Za-z0-9_]+ regex check (the vanilla Minecraft character
set), rejecting non-conforming usernames with a QuietDecoderException.
Testing
./gradlew :velocity-proxy:check passes all checks (compile, Checkstyle,
SpotBugs, tests).
Closes #1828
Closes #1827