Do not abort() on an oversized position packet - #258
Draft
IrPgFKS0 wants to merge 1 commit into
Draft
Conversation
ServerSend() called abort() -- a process-killing crash -- when a packet over 500 bytes contained "Zp". Two problems: ordinary runtime data can trigger it (an oversized position packet should be dropped, not crash the launcher), and find() matched "Zp" anywhere in the payload, so an unrelated packet that merely contained those two bytes could kill the launcher as well. Check the actual packet code prefix and drop the packet gracefully instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This sounds like an active issue happening on some servers. Can this be looked into? I dont see any big issues with this change. Disconnect issues have been a plague since forever! |
Collaborator
|
Yes, this will be addressed in an upcoming patch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ServerSend()insrc/Network/GlobalHandler.cppcallsabort()— a process-killing crash — when a packet over 500 bytes contains"Zp". Two problems:Data.find("Zp") != nposmatches those two bytes anywhere in the payload — so an unrelated packet (chat message, event payload, mod data) that merely contains the substringZpand is over 500 bytes kills the launcher too.Fix
Check the actual packet code (
Data[0] == 'Z' && Data[1] == 'p'— safe, the size check precedes it) and drop the oversized packet with a debug log instead of aborting.How this was found
Hit in practice on a LAN fork of BeamMP (an oversized position payload aborted the launcher mid-session); the graceful-drop version has been running in that fork for weeks of sessions.
Transparency
This fix comes from an AI-assisted fork: the bug was found and the patch written with the help of an AI coding tool (Claude), then tested by a human in real multiplayer sessions. Given this project's policy on AI-generated code, it's submitted as a draft for the maintainers to decide — happy to close it if that's not wanted, or for a maintainer to re-implement it independently.
🤖 Generated with Claude Code