Fix heap corruption on instructions that outgrow the parse tree

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

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;

pypcode/sleigh/sleigh.cc

@@ -453,7 +453,12 @@ void DisassemblyCache::initialize(int4 min,int4 hashsize)
   hashtable = new ParserContext *[hashsize];
   for(int4 i=0;i<minimumreuse;++i) {
     ParserContext *pos = new ParserContext(contextcache,translate);
-    pos->initialize(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];

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.