Skip to content

Fix the signature scanner writing to a freed stack frame - #158

Merged
CryZe merged 1 commit into
LiveSplit:masterfrom
Janzert:signature-scan-dangling-buffer
Sep 5, 2026
Merged

Fix the signature scanner writing to a freed stack frame#158
CryZe merged 1 commit into
LiveSplit:masterfrom
Janzert:signature-scan-dangling-buffer

Conversation

@Janzert

@Janzert Janzert commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Signature::scan_iter zero-initialises a Buffer<N> as a local, takes a
&mut [u8] over it with slice::from_raw_parts_mut, and moves that slice
into the iter::from_fn closure it returns.

Only the slice is moved. The buffer stays a local of scan_iter, so its frame
is gone before the returned iterator is ever polled. Every poll then reads and
writes roughly 4 KiB through a pointer into a dead stack frame — on a scan that
Module::attach runs.

That is UB regardless of what it happens to do at runtime: the pointer outlives
the storage it was derived from.

How it was discovered

A test harness replays captured game memory through a chain of twenty delta
snapshots, which puts the caller of the scan about twenty frames deeper than
normal. The scan's 256-byte page read then landed inside a live frame, over
a return address. It only bites when whatever runs below the stale address is
deeper than whatever ran there before. That, and release builds inlining
scan_iter away entirely, is why it does not normally surface.

What decides whether it bites

opt-level, and nothing else I tried:

opt-level lto = false lto = true
0 reproduces reproduces
1-3 no no

At opt-level = 0 scan_iter is a real function whose frame is popped. From
opt-level = 1 it is inlined into its caller, so the buffer lands in the frame
that then drives the iterator and nothing dangles;
-C llvm-args=--inline-threshold=0 does not bring it back at 3. I also checked
one release wasm32-unknown-unknown build of an auto splitter: no standalone
scan_iter in the binary, and the frames that drive the scans reserve
4224–4256 bytes of shadow stack, which is the buffer sitting in the live frame.

So release builds are very likely fine today — by inlining, not by anything
guaranteeing it. Debug builds are not, and that is what cargo test uses.

The test

Second commit, tests/signature_scan_buffer.rs. The current Test (Host) job
runs it as it stands. No CI change needed, and nothing from the crate beyond
the public API.

The runtime side of a read is stubbed. It plants a canary in a local array,
then writes only into the buffer asr asked it to fill — it does not know where
that buffer is. Without the fix in the first commit filling the buffer the
scan handed over writes 256 bytes into the reader's own locals, so the buffer
is a pointer into a frame that has already been given back.

The canary is the whole assertion, because it is the only part that cannot
misfire: locals do not change unless something wrote into them, whichever way
the stack grows and wherever the frames land. Where the buffer sat is reported
with the failure but not asserted on — which sign means trouble depends on the
stack's direction and on how deep the freed frame had been.

So it can miss but not misfire, and for the reason above it only fails at
opt-level = 0. Both are written into the test.

The fix

Move the buffer into the closure — move already captures it once the outer
let buffer = ... rebinding is removed — and build the slice inside each call,
where the storage is live for as long as the pointer is used. One hunk in
src/signature.rs; no behaviour change, no API change.

Something else in the same three lines

Not changed here, and mentioned only so it is not a surprise: the tail carried
in front of each page is taken from the offset a full previous page would
have ended at. That holds for every page but two — the first page of a range
that does not start on a boundary, and the last page of one that does not end
on one.

  • A range ending part way into a page asks for more bytes than the page held and
    panics in copy_from_slice.
  • A range starting part way into a page carries bytes that were never read, and
    misses a signature across its first boundary silently.

Module and memory ranges are page-aligned at both ends, so they reach neither.
The scans of the form (some symbol, 0x100) aren't aligned at either end,
though, and those reach the panic: five sites in mono::Module::attach, one per
pointer-size/format arm. Module bases being page-aligned, that offset is
RVA & 0xfff — set by the shipped mono binary rather than varying per launch.
I measured 0x390 on Unity 6000.3 and 0x790 on 6000.5. With 16-byte function
alignment, 15 of the 256 slots in a page put the scan across a boundary, so
roughly 6% of mono builds; a bad one would panic for every splitter against
every game on that Unity version, every time, rather than failing
intermittently. The silent miss needs a range that begins part way into a page
and then runs a full page further, which asr itself never builds.

This is a pre-existing problem and independent of the dangling reference.
I have a fix and two regression tests and am happy to send them as a separate
pull request.

🤖 Code and comments initially written with Claude Code

@CryZe

CryZe commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

That's a really good finding, though, can you remove the tests? Like, there's some other pull requests that also add testing, and it kind of interferes with what you're doing here. Overall, for the testing strategy, I would like to figure out a better strategy long term. So for now, I would want you to just remove the tests.

@Janzert
Janzert force-pushed the signature-scan-dangling-buffer branch from 4a2fdd5 to b64df45 Compare September 5, 2026 15:38
`scan_iter` zero-initialises a `Buffer<N>` as a local, takes a `&mut [u8]`
over it via `slice::from_raw_parts_mut`, and moves that slice into the
`iter::from_fn` closure it returns. Only the slice is moved: the buffer
itself remains a local of `scan_iter`, so by the time the returned
iterator is first polled the storage the slice points at has been given
back, and every poll reads and writes roughly 4 KiB of a dead stack
frame.

Move the buffer into the closure and take the slice inside each call,
where its storage is live for as long as the pointer is used.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@CryZe
CryZe force-pushed the signature-scan-dangling-buffer branch from b64df45 to 620f56d Compare September 5, 2026 15:39
@CryZe
CryZe enabled auto-merge (squash) September 5, 2026 15:39
@Janzert

Janzert commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Sure, no problem. They were in the last commit by themselves so quite easy to drop.

@CryZe
CryZe merged commit 12375fc into LiveSplit:master Sep 5, 2026
7 checks passed
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.

2 participants