Repository navigation
Conversation
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS Validation record for head This head is the change rebased onto master at Ghidra 12.1.4, plus a skip of
Register lists are abbreviated in the last column. Which glibc check fires, and whether the process aborts or faults, depends on heap layout and varies between builds. Caveats:
How these were runThe dataset rows come from
The 40 objects were run twice more per side with pypcode_native sha256: baseline The 40 objects
Earlier measurements, at baseline
Tree sizes from a build with a counter in
The operand counts come from the shipped specs:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #289 +/- ##
=======================================
Coverage 83.65% 83.65%
=======================================
Files 5 5
Lines 514 514
Branches 98 98
=======================================
Hits 430 430
Misses 26 26
Partials 58 58 ☔ View full report in Codecov by Harness. |
478e663 to
54d3ad5
Compare
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS A note on an interaction with #288. This branch makes a crash in This branch still fixes JVM, NDS32 and the PowerPC case in #292, none of which #288 touches. But with it merged, a delay-slot use-after-return will often not fault, which could hide a regression, so #288 should be evaluated on its own tree rather than on top of this one. |
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS Corpus frequency for this defect, from a sweep that has since completed. Of 746,966 objects analysed, 479 died on a signal. This defect accounts for 115 of them, 24% of every native crash and the second largest cause. It also covers the PowerPC All 115 are 32-to-128-byte synthetic blobs from a generated p-code collection. Not one is a real-world binary. So the rank and the 24% measure the fuzz corpus, not real-world exposure: by real-world objects the same run puts a delay-slot use-after-return (#288) at 220 and an out-of-memory family at 40 ahead of it. That does not make it less of a defect. One interaction worth knowing: this change makes #288's symptom disappear without fixing it. Growing |
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS
Before — three encodings return correct p-code and then abort in an unrelated allocator call, two abort inside pypcode master, 559aacdAfter — six decode and exit cleanly, and the unbounded JVM with this change, 54d3ad5 |
ParserContext holds the parse tree of the instruction being decoded in three
fixed arrays: a node array and a per-node operand array, both sized once by
ParserContext::initialize, and the walker's breadcrumb trail. allocateOperand
indexed all three without checking any of them, so an instruction whose tree
did not fit wrote past the end of two heap blocks and into the walker's own
context member. Depending on the instruction and the heap layout, the process
then died with a segfault or a glibc heap-check abort, either during
translation or after translation had returned.
Ordinary encodings reach every one of the three. PowerPC AltiVec spells
arithmetic out one operand per vector lane, so vaddubm declares 24 operands and
vadduhm and vsubuhm declare 27, against an allotment of 20. A register list
takes one Constructor per register, so ARM vldmia r0,{s0-s19} needs 76 nodes
against 75, and vldmia r0,{s0-s30} needs 109 and nests 36 deep against a
32-entry breadcrumb trail.
Give the node array room for 512 nodes and the breadcrumb trail room for 128,
have setConstructor size a node's operand array to the Constructor it is
attached to, and have allocateOperand raise BadDataError rather than allocate a
node the arrays cannot hold. BadDataError is already how oneInstruction reports
undecodable input, so a tree that genuinely has no bound of its own -- a JVM
lookupswitch or tableswitch, which nests one level per table entry and takes
the entry count from the instruction stream -- now reaches the caller as a
catchable exception. Refusing abandons a half-built tree in a cached
ParserContext, so the tests cover that the Context stays usable afterwards.
The tests run each instruction in a child interpreter, so they are skipped
under Pyodide, which cannot start one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
54d3ad5 to
327cfba
Compare
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS
Problem
Context.translatewrites past fixed-size parse-tree arrays on ordinary encodings. On PowerPC0x10000000,vaddubm v0,v0,v0, translation returns correct p-code and the process dies afterwards in an unrelated allocator call:Five more encodings across three shipped languages do the same —
0x10000040and0x10000440(vadduhm,vsubuhm), ARM14 0a 90 ecand1f 0a 90 ec(vldmiawith 20 and with 31 registers), NDS323b c5 37 45(lmwa.bim) — dying as SIGABRT or SIGSEGV depending on heap layout. In angr, CFGFast dies with SIGSEGV on 40 JVM objects from megabench, each at atableswitchorlookupswitch. They are generated 32- to 128-byte sequences, not class files, and are not public (SHA-256s in the validation record). An ordinary switch fails the same way: on master, atableswitchwith 9 or more cases, or alookupswitchwith 6 or more pairs (or none), kills the process.Root cause
Every cached parse tree gets a fixed allotment, 75 nodes of 20 operands each in
pypcode/sleigh/sleigh.cc, and a fixed path from the root inpypcode/sleigh/context.hh:ParserContext::allocateOperandindexes all three and checks none of them:ConstructState *opstate = &state[alloc++]; ... walker.point->resolve[i] = opstate; walker.breadcrumb[walker.depth++] += 1;Measured on an instrumented build, one instruction per process:
10 00 00 00vaddubm10 00 00 40vadduhm14 0a 90 ecvldmia, 20 registers1f 0a 90 ecvldmia, 31 registers3b c5 37 45lmwa.bimAltiVec arithmetic names one operand per vector lane, so
vaddubmruns off the 20-entry operand array. A register list is one Constructor per register, sovldmiaandlmwa.bimrun off the 75-node array, and at 31 registers off the 32-entry breadcrumb trail as well. A JVMlookupswitch(ab) ortableswitch(aa) recurses once per table entry with the count read from the instruction stream, so its tree has no bound at all.Fix
Size a node's operand array to the Constructor attached to it, in
ParserWalkerChange::setConstructor; raise the node allotment to 512 and the breadcrumb trail to 128, several times the largest tree measured above; and haveallocateOperandraiseBadDataErrorrather than overrun either. That is already howtranslateanddisassemblereport undecodable input (pypcode/pypcode_native.cpp:309), and angr maps it toIjk_NoDecode. All seven encodings then decode or refuse cleanly, the JVM one as:JVM switches still do not decode; refusing them is containment, not support.
This code is vendored unchanged from Ghidra 12.1.4, which has the same defect. Upstream master reworked the allocation in
5328fa2c6dbf(GP-6554), which 12.1.4 does not contain: it grows the node array on demand but keeps the 20-operand and 32-level limits, now checked and raised asLowlevelError, so it refusesvaddubm,vadduhm,vsubuhmand the 31-registervldmiainstead of decoding them.Testing
ParseTreeTestsdecodes each encoding in a child interpreter, because on master the overrun raises nothing and the process doing it cannot observe the damage;test_context_is_reusable_after_a_refusalchecks that a refused instruction leaves the cachedParserContextusable. The class is skipped under Pyodide, which master's CI tests since22fe108and which cannot start a child interpreter. Against a build of master,-k ParseTreeover this branch's tests reports8 failed, 2 passed, every failure a child killed by a signal (assert -11 == 0,assert -6 == 0); on this branch, 4 passed and 6 subtests passed.On the 40 JVM objects, the sweep's per-object CFGFast completes on this branch where master dies, in 3 of 3 runs each, and the 3284 other p-code objects in the same sweep keep identical status, function, block and edge counts.
Growing
ParserWalkeralso resizes every stack frame holding a walker, which makes a separate delay-slot use-after-return in sparc and MIPS stop faulting without repairing it. pypcode 288 is that fix and has to be judged on its own tree, not on top of this one.Validation: #289 (comment)
session: sharpen