Skip to content

Fix GS4 query crash and validate login usernames#1849

Open
nezxenka wants to merge 1 commit into
PaperMC:dev/4.0.0from
nezxenka:fix/validate-username-and-gs4-guard
Open

Fix GS4 query crash and validate login usernames#1849
nezxenka wants to merge 1 commit into
PaperMC:dev/4.0.0from
nezxenka:fix/validate-username-and-gs4-guard

Conversation

@nezxenka

Copy link
Copy Markdown

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

@electronicboy

Copy link
Copy Markdown
Member

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.
@nezxenka
nezxenka force-pushed the fix/validate-username-and-gs4-guard branch from aff2bf4 to 91967d6 Compare July 20, 2026 00:56
@nezxenka

Copy link
Copy Markdown
Author

Done — removed the username validation. This PR is now just the GS4 query fix.

@WouterGritter

WouterGritter commented Jul 20, 2026

Copy link
Copy Markdown
Member

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:
"7" is a weird magic number here. Is there any way we could calculate this value? Most likely not, but at the very least I'd like this to be a private static field with a short javadoc explaining this is the shorted expected message and which branch reads this shortest possible message (QUERY_TYPE_HANDSHAKE / QUERY_TYPE_STAT).

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

Labels

None yet

Projects

None yet

3 participants