fix: ServiceTag %ls buffer overread in Profile UI (stack corruption)#6
Open
kirklandsig wants to merge 1 commit into
Open
fix: ServiceTag %ls buffer overread in Profile UI (stack corruption)#6kirklandsig wants to merge 1 commit into
kirklandsig wants to merge 1 commit into
Conversation
ServiceTag is a fixed wchar_t[4] with no null terminator. sprintf's %ls walks until it finds a null word, so a profile whose tag uses all 4 characters (common for real player profiles) reads past the array and can overflow the 1024-byte stack buffer - this runs every frame the Profile section is rendered. Also clears the field before writing edits back so shortening a tag can't leave stale trailing characters. Co-Authored-By: Claude <noreply@anthropic.com>
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
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.
Hey Jack — kirklandsig here. While hunting a long-standing crash in my fork (users crashing on Apply Profile → Save Profile), I traced it to this line, and your
mastercarries the same code, so sending the fix upstream.The bug
ServiceTagis a fixedwchar_t[4]with no null terminator — a legitimate 4-character tag ("ODST", "117X", …) fills every slot.CUserProfile::ImGuiContextdoes:%lswalks memory until it hits a null word, so with a full 4-char tag it reads past the array into adjacent struct memory, and — if the following bytes happen to be non-zero for long enough — overflows the 1024-byte stack buffer. This executes every frame the Profile section is rendered.It's sneaky because dev-created profiles start zero-filled (safe); the dangerous data arrives when a real player profile with a full tag is copied in. In my fork that's the "Apply Profile" flow, which matched the crash reports exactly: unreproducible locally, deterministic for users with 4-char service tags.
The fix
wchar_t[5]copy before printing (bounded, no behavior change for shorter tags).wmemsetthe field before writing an edit back, so shortening a tag can't leave stale trailing characters that recreate the unterminated state.Kept dependency-free/inline so it drops straight into your tree. In my fork this shipped in v1.4.5-experimental (over there it's factored into shared
Stringhelpers; happy to PR that variant instead if you prefer). Compile-verified in my fork whereImGuiContextis byte-identical around this block — I haven't built your vcpkg/SDL2 setup, so a quick build check on your side would be appreciated.Thanks for porting my earlier changes into your beta, by the way — figured I'd return the favor.
🤖 Generated with Claude Code