From 54d3ad52d932d7a7cb4e7287778d47a0cdd860a4 Mon Sep 17 00:00:00 2001 From: Yan Date: Sun, 9 Aug 2026 18:39:07 +0000 Subject: [PATCH] Bound the instruction parse tree instead of writing past it 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. Translation returned normally with correct p-code and the process died later, in an unrelated malloc or free, with a glibc abort or a segfault. 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. NDS32 register-list instructions reach 120 nodes. 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, 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. Over 20,000 random inputs against each of the 187 shipped languages, no instruction outside JVM reaches either limit, and the largest tree any of them builds is 120 nodes at 36 levels. Co-Authored-By: Claude Opus 5 --- pypcode/sleigh/context.cc | 21 +++++++ pypcode/sleigh/context.hh | 18 +++++- pypcode/sleigh/sleigh.cc | 7 ++- tests/test_pypcode.py | 113 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 4 deletions(-) diff --git a/pypcode/sleigh/context.cc b/pypcode/sleigh/context.cc index 1a0a35f3..f4fc61db 100644 --- a/pypcode/sleigh/context.cc +++ b/pypcode/sleigh/context.cc @@ -46,6 +46,15 @@ void ParserContext::initialize(int4 maxstate,int4 maxparam,AddrSpace *spc) base_state = &state[0]; } +void ParserContext::parseTreeOverflow(const string &msg) const + +{ // The node array and the walker's breadcrumb trail are allocated + // once and cannot grow, so an instruction needing a bigger parse + // tree than they hold is bad data. Out of line because + // BadDataError is not declared in context.hh + throw BadDataError(msg); +} + const Address &ParserContext::getN2addr(void) const { @@ -220,6 +229,18 @@ void ParserWalker::setOutOfBandState(Constructor *ct,int4 index,ConstructState * breadcrumb[0] = 0; } +void ParserWalkerChange::setConstructor(Constructor *c) + +{ // Give the node room for every operand the Constructor declares. + // A few Constructors, such as the PowerPC AltiVec ones that name + // one operand per vector lane, declare more than the allotment + // ParserContext::initialize hands out + point->ct = c; + int4 numoper = c->getNumOperands(); + if (numoper > (int4)point->resolve.size()) + point->resolve.resize(numoper); +} + void ParserWalkerChange::calcCurrentLength(int4 length,int4 numopers) { // Calculate the length of the current constructor diff --git a/pypcode/sleigh/context.hh b/pypcode/sleigh/context.hh index 79fbe1ef..be316e26 100644 --- a/pypcode/sleigh/context.hh +++ b/pypcode/sleigh/context.hh @@ -94,6 +94,7 @@ private: ConstructState *base_state; int4 alloc; // Number of ConstructState's allocated int4 delayslot; // delayslot depth + void parseTreeOverflow(const string &msg) const; // Reject an instruction that outgrows the parse tree public: ParserContext(ContextCache *ccache,Translate *trans); ~ParserContext(void) { if (context != (uintm *)0) delete [] context; } @@ -128,12 +129,19 @@ public: }; class ParserWalker { // A class for walking the ParserContext +public: + // Size of the breadcrumb trail, so one more than the deepest operand + // nesting an instruction may have. The deepest tree a shipped language + // builds is 36 levels, but a JVM lookupswitch nests once per table entry + // and has no bound of its own + enum { max_depth = 128 }; +private: const ParserContext *const_context; const ParserContext *cross_context; protected: ConstructState *point; // The current node being visited int4 depth; // Depth of the current node - int4 breadcrumb[32]; // Path of operands from root + int4 breadcrumb[max_depth]; // Path of operands from root public: ParserWalker(const ParserContext *c) { const_context = c; cross_context = (const ParserContext *)0; } ParserWalker(const ParserContext *c,const ParserContext *cross) { const_context = c; cross_context = cross; } @@ -175,7 +183,7 @@ public: ParserContext *getParserContext(void) { return context; } ConstructState *getPoint(void) { return point; } void setOffset(uint4 off) { point->offset = off; } - void setConstructor(Constructor *c) { point->ct = c; } + void setConstructor(Constructor *c); void setCurrentLength(int4 len) { point->length = len; } void calcCurrentLength(int4 length,int4 numopers); }; @@ -191,10 +199,14 @@ inline void ParserContext::deallocateState(ParserWalkerChange &walker) { } inline void ParserContext::allocateOperand(int4 i,ParserWalkerChange &walker) { + if (alloc >= (int4)state.size()) + parseTreeOverflow("Instruction parse tree is too large"); + if (walker.depth + 1 >= ParserWalker::max_depth) + parseTreeOverflow("Instruction parse tree is too deep"); ConstructState *opstate = &state[alloc++]; opstate->parent = walker.point; opstate->ct = (Constructor *)0; - walker.point->resolve[i] = opstate; + walker.point->resolve[i] = opstate; // Sized to hold every operand by setConstructor walker.breadcrumb[walker.depth++] += 1; walker.point = opstate; walker.breadcrumb[walker.depth] = 0; diff --git a/pypcode/sleigh/sleigh.cc b/pypcode/sleigh/sleigh.cc index 304a73b4..1bf44ea0 100644 --- a/pypcode/sleigh/sleigh.cc +++ b/pypcode/sleigh/sleigh.cc @@ -453,7 +453,12 @@ void DisassemblyCache::initialize(int4 min,int4 hashsize) hashtable = new ParserContext *[hashsize]; for(int4 i=0;iinitialize(75,20,constspace); + // A register list is spelled out as one Constructor per register in several languages, so the + // node count runs well past the handful an ordinary instruction needs: ARM vldmia with 31 + // single-precision registers takes 109 nodes, and the largest tree seen across the shipped + // languages is an NDS32 register-list instruction at 120. 512 keeps several times that in + // reserve, for about 140KB per cached parse tree. + pos->initialize(512,20,constspace); list[i] = pos; } ParserContext *pos = list[0]; diff --git a/tests/test_pypcode.py b/tests/test_pypcode.py index 4f258a87..b20a045d 100755 --- a/tests/test_pypcode.py +++ b/tests/test_pypcode.py @@ -3,6 +3,8 @@ import gc import logging +import subprocess +import sys from unittest import main, TestCase from unittest.mock import create_autospec from typing import cast @@ -319,6 +321,117 @@ def test_pretty_printing(self): assert "RAX = RAX ^ RAX" in str(tx) +# Translate one instruction, given a language id and hex-encoded bytes, and print either the +# instruction it decoded to or the name of the exception that was raised. +TRANSLATE_ONE = """ +import sys + +import pypcode + +ctx = pypcode.Context(sys.argv[1]) +code = bytes.fromhex(sys.argv[2]) +try: + ctx.translate(code, max_instructions=1) + ins = ctx.disassemble(code, max_instructions=1).instructions[0] + print(ins.mnem, ins.body) +except Exception as exc: + print(type(exc).__name__) +""" + +# Refuse one instruction, then go on using the same Context. Prints, one per line, the outcome of +# translating the refused bytes, then the good bytes at the refused address and at two further +# addresses sharing its parser cache slot, then the refused bytes again. +REFUSE_THEN_REUSE = """ +import sys + +import pypcode + +ctx = pypcode.Context(sys.argv[1]) +refused = bytes.fromhex(sys.argv[2]) +good = bytes.fromhex(sys.argv[3]) + +def attempt(code, address): + try: + ctx.translate(code, base_address=address, max_instructions=1) + return ctx.disassemble(code, base_address=address, max_instructions=1).instructions[0].mnem + except Exception as exc: + return type(exc).__name__ + +print(attempt(refused, 0x1000)) +for address in (0x1000, 0x1020, 0x1040): + print(attempt(good, address)) +print(attempt(refused, 0x1000)) +""" + + +class ParseTreeTests(TestCase): + """ + Tests for instructions with an unusually large Constructor tree + """ + + def run_in_subprocess(self, program: str, *args: str) -> subprocess.CompletedProcess[str]: + """ + Run `program` in a child interpreter and report how the child ended. + + An instruction that outgrows the parse tree writes past it without raising, so the damage + surfaces as a heap abort or a segfault that the process doing it cannot observe. Only a + child process can be inspected for it. + + The child runs with -P so that it imports the pypcode under test rather than whatever + happens to sit in the working directory, which is the source tree when the suite is run + from the repository root. + """ + return subprocess.run( + [sys.executable, "-P", "-c", program, *args], + capture_output=True, + text=True, + check=False, + ) + + def test_more_operands_than_allotted(self): + # AltiVec arithmetic is spelled out one operand per vector lane, so vaddubm declares 24 + # operands and vadduhm and vsubuhm declare 27, against an allotment of 20. + for code, mnem in [ + (b"\x10\x00\x00\x00", "vaddubm"), + (b"\x10\x00\x00\x40", "vadduhm"), + (b"\x10\x00\x04\x40", "vsubuhm"), + ]: + with self.subTest(mnem=mnem): + proc = self.run_in_subprocess(TRANSLATE_ONE, "PowerPC:BE:32:default", code.hex()) + assert proc.returncode == 0, proc.stderr + assert mnem in proc.stdout + + def test_more_nodes_than_allotted(self): + # A register list takes one Constructor per register, so an instruction naming enough of + # them outgrows the node array. vldmia with 31 registers also nests deeper than the + # walker's breadcrumb trail used to reach. + for langid, code, body in [ + ("ARM:LE:32:v7", b"\x14\x0a\x90\xec", "vldmia r0,{s0,s1,s2,"), # 20 registers, 76 nodes + ("ARM:LE:32:v7", b"\x1f\x0a\x90\xec", "s29,s30}"), # 31 registers, 109 nodes, 36 deep + ("NDS32:LE:32:default", b"\x3b\xc5\x37\x45", "lmwa.bim fp,"), # 76 nodes + ]: + with self.subTest(langid=langid, code=code.hex()): + proc = self.run_in_subprocess(TRANSLATE_ONE, langid, code.hex()) + assert proc.returncode == 0, proc.stderr + assert body in proc.stdout + + def test_parse_tree_limit_is_reported(self): + # A JVM lookupswitch consumes its table by recursing once per entry, with the entry count + # taken from the instruction stream, so its tree has no bound of its own. Refusing it has + # to reach the caller as an exception rather than as a write past the node array. + proc = self.run_in_subprocess(TRANSLATE_ONE, "JVM:BE:32:default", "ab") + assert proc.returncode == 0, proc.stderr + assert "BadDataError" in proc.stdout + + def test_context_is_reusable_after_a_refusal(self): + # Refusing abandons a half-built tree in a cached ParserContext, so the Context has to + # stay usable: the refused address and the rest of its cache slot still decode, and + # asking for the refused instruction again reports the error rather than the partial tree. + proc = self.run_in_subprocess(REFUSE_THEN_REUSE, "JVM:BE:32:default", "ab", "00") + assert proc.returncode == 0, proc.stderr + assert proc.stdout.split() == ["BadDataError", "nop", "nop", "nop", "BadDataError"] + + class PrintingTests(TestCase): """ Pretty printing tests.