Skip to content

Fix segfault when a delay-slot instruction has no p-code - #288

Open
zardus wants to merge 1 commit into
masterfrom
feature/fix-pypcode-delayslot-uaf
Open

Fix segfault when a delay-slot instruction has no p-code#288
zardus wants to merge 1 commit into
masterfrom
feature/fix-pypcode-delayslot-uaf

Conversation

@zardus

@zardus zardus commented Aug 9, 2026

Copy link
Copy Markdown
Member

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Context.translate segfaults on a delay-slot branch whose delay slot has no p-code. SleighBuilder::delaySlot points the builder at a ParserWalker on its own stack frame and puts the previous one back only on normal exit, so the UnimplError raised for the empty delay slot leaves the builder holding a destroyed walker that Sleigh::oneInstruction then reads to describe the error.

delaySlot and appendCrossBuild now restore through a scope object, so an exception cannot outlive the walker it installed. The error names the branch rather than its delay slot, matching the instruction length it already carries. The vendored code still matches Ghidra master, so the defect is upstream too.

The regression translates a call followed by an unimplemented instruction on sparc and Toy; both segfault on master.

Validation: #288 (comment)

@zardus

zardus commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Validation record for head 2bbb34dc13c44db056ae0493b7a56f451058b030 against baseline 559aacdc9d363fd19477d9daa40721279cd99248.

  • Reproducer: python -c 'import pypcode; pypcode.Context("sparc:BE:32:default").translate(bytes.fromhex("63748596a7b8c9da"))' — SIGSEGV on baseline, UnimplError naming the call on head
  • Regression: python -m pytest tests/test_pypcode.py -k "delay_slot_unimpl or partial_delay_slot" — 2 passed on head; on a baseline build of pypcode/sleigh/sleigh.cc the process dies with SIGSEGV in test_delay_slot_unimpl_failure, exit 139
  • End to end: CFGFast over those eight bytes as a sparc:BE:32:default blob, angr 9.3.3.dev0 — SIGSEGV on baseline, completes on head
  • Focused: python -m pytest tests/ — 48 passed, 191 subtests passed
  • As the wheel job runs it: python -m unittest discover -s tests — Ran 48 tests, OK
  • Lint/type: pre-commit run --all-files — of 27 hooks, 25 passed and 2 skipped for having no files to check, none rewrote a file; black, ruff, mypy and pylint included
  • Docs: make -C docs html coverage — build succeeded, docstring coverage 100%
  • Differential: all 187 shipped languages, 64 pseudorandom 16-byte inputs each, one process per language — the 182 languages that survive both revisions produce identical p-code and identical exception text on all 11,648 results, with space-id operands normalized because they carry raw AddrSpace pointers that differ per build; sparc:BE:64:default segfaults on baseline and survives on head
  • The two builds differ only in pypcode/sleigh/sleigh.cc; the sleigh compiler, the .sla files and every other object are unchanged

Caveats, one line each:

  • appendCrossBuild is fixed for the same invariant but has no reproducer: inside a named p-code section every build() call checks its template for null first, so only a delay slot in a crossbuild target could raise UnimplError there, and Hexagon, the only shipped language using CROSSBUILD, has no delay slots.
  • The differential run also crashes on ARM, JVM and NDS32 from unrelated defects, identically on both revisions.
  • Measured on Linux x86_64 with GCC 15.2.0 and Python 3.12.13; Windows and macOS are covered only by this PR's CI.

@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.82%. Comparing base (559aacd) to head (2bbb34d).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #288   +/-   ##
=======================================
  Coverage   86.82%   86.82%           
=======================================
  Files           5        5           
  Lines         516      516           
  Branches       82       82           
=======================================
  Hits          448      448           
  Misses         26       26           
  Partials       42       42           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

SleighBuilder::delaySlot points the builder at a ParserWalker on its own
stack frame and puts the previous one back only on the normal exit path.
When the delay-slot instruction has no p-code section, PcodeBuilder::build
throws UnimplError, the walker is destroyed with the frame, and the
builder is left holding that address. Sleigh::oneInstruction catches the
exception and describes it through exactly that pointer, reading a
ParserContext out of stack the handler has already reused, so
Context.translate segfaults on any delay-slot branch followed by an
instruction with no semantics. Eight bytes of SPARC are enough:

    Context("sparc:BE:32:default").translate(bytes.fromhex("63748596a7b8c9da"))

SleighBuilder::appendCrossBuild saves and restores the walker the same
unguarded way around a build() that can throw.

Restore both through a scope object so an exception leaving either method
cannot outlive the walker it installed. The reported instruction is now
the branch rather than its delay slot, which matches the instruction
length UnimplError already carries.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@zardus
zardus force-pushed the feature/fix-pypcode-delayslot-uaf branch from 40c3774 to 2bbb34d Compare August 9, 2026 22:47
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.

1 participant