Sleigh engine memory fixes

Ghidra/Features/Decompiler/src/decompile/cpp/marshal.cc

@@ -689,7 +689,10 @@ void PackedDecode::endIngest(int4 bufPos)
     }
     uint1 *buf = inStream.back().start;
     buf[bufPos] = ELEMENT_END;
+  } else {
+    throw DecoderError("Ended ingestion without any input");
   }
+
 }
 
 PackedDecode::~PackedDecode(void)
@@ -1006,6 +1009,9 @@ AddrSpace *PackedDecode::readSpace(void)
   AddrSpace *spc;
   if (typeCode == TYPECODE_ADDRESSSPACE) {
     res = readInteger(readLengthCode(typeByte));
+    if (res >= spcManager->numSpaces())
+      throw DecoderError("Invalid address space index");
+
     spc = spcManager->getSpace(res);
     if (spc == (AddrSpace *)0)
       throw DecoderError("Unknown address space index");

Ghidra/Features/Decompiler/src/decompile/cpp/marshal.hh

@@ -536,7 +536,7 @@ private:
   uint1 getByte(Position &pos) { return *pos.current; }	///< Get the byte at the current position, do not advance
   uint1 getBytePlus1(Position &pos);	///< Get the byte following the current byte, do not advance position
   uint1 getNextByte(Position &pos);	///< Get the byte at the current position and advance to the next byte
-  void advancePosition(Position &pos,int4 skip);	///< Advance the position by the given number of bytes
+  void advancePosition(Position &pos,uint4 skip);	///< Advance the position by the given number of bytes
   uint8 readInteger(int4 len);		///< Read an integer from the \e current position given its length in bytes
   uint4 readLengthCode(uint1 typeByte) { return ((uint4)typeByte & PackedFormat::LENGTHCODE_MASK); }	///< Extract length code from type byte
   void findMatchingAttribute(const AttributeId &attribId);	///< Find attribute matching the given id in open element
@@ -631,7 +631,7 @@ inline uint1 PackedDecode::getNextByte(Position &pos)
 /// An exception is thrown of position is advanced past the end of the stream
 /// \param pos is the position being advanced
 /// \param skip is the number of bytes to advance
-inline void PackedDecode::advancePosition(Position &pos,int4 skip)
+inline void PackedDecode::advancePosition(Position &pos,uint4 skip)
 
 {
   while(pos.end - pos.current <= skip) {

Ghidra/Features/Decompiler/src/decompile/cpp/semantics.hh

@@ -142,7 +142,7 @@ class OpTpl {
   OpCode opc;
   vector<VarnodeTpl *> input;
 public:
-  OpTpl(void) {}
+  OpTpl(void) : output(nullptr) {}
   OpTpl(OpCode oc) { opc = oc; output = (VarnodeTpl *)0; }
   ~OpTpl(void);
   VarnodeTpl *getOut(void) const { return output; }

Ghidra/Features/Decompiler/src/decompile/cpp/slghpatexpress.cc

@@ -503,7 +503,7 @@ PatternExpression *PatternExpression::decodeExpression(Decoder &decoder,Translat
   else if (el == sla::ELEM_NOT_EXP)
     res = new NotExpression();
   else
-    return (PatternExpression *)0;
+    throw DecoderError("Invalid pattern expression element");
   try {
     res->decode(decoder,trans);
   } catch(...) {
@@ -830,6 +830,9 @@ void OperandValue::decode(Decoder &decoder,Translate *trans)
   uintm ctid = decoder.readUnsignedInteger(sla::ATTRIB_CT);
   SleighBase *sleigh = (SleighBase *)trans;
   SubtableSymbol *tab = dynamic_cast<SubtableSymbol *>(sleigh->findSymbol(tabid));
+  if (ctid >= tab->getNumConstructors()) {
+    throw DecoderError("Invalid constructor id");
+  }
   ct = tab->getConstructor(ctid);
   decoder.closeElement(el);
 }

Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc

@@ -49,10 +49,12 @@ SymbolTable::~SymbolTable(void)
 {
   vector<SymbolScope *>::iterator iter;
   for(iter=table.begin();iter!=table.end();++iter)
-    delete *iter;
+    if (*iter)
+      delete *iter;
   vector<SleighSymbol *>::iterator siter;
   for(siter=symbollist.begin();siter!=symbollist.end();++siter)
-    delete *siter;
+    if (*siter)
+      delete *siter;
 }
 
 void SymbolTable::addScope(void)
@@ -175,8 +177,20 @@ void SymbolTable::decode(Decoder &decoder,SleighBase *trans)
   for(int4 i=0;i<table.size();++i) { // Decode the scopes
     int4 subel = decoder.openElement(sla::ELEM_SCOPE);
     uintm id = decoder.readUnsignedInteger(sla::ATTRIB_ID);
+    if (id >= table.size()) {
+      throw SleighError("Bad symbol scope id: exceeds symbol scope table size");
+    }
+
     uintm parent = decoder.readUnsignedInteger(sla::ATTRIB_PARENT);
+    if (parent >= table.size()) {
+      throw SleighError("Bad symbol scope parent id: exceeds symbol scope table size");
+    }
+
     SymbolScope *parscope = (parent==id) ? (SymbolScope *)0 : table[parent];
+    if (table[id]) {
+      throw SleighError("Bad symbol scope parent id: not unique");
+    }
+
     table[id] = new SymbolScope( parscope, id );
     decoder.closeElement(subel);
   }
@@ -202,41 +216,58 @@ void SymbolTable::decodeSymbolHeader(Decoder &decoder)
 
 {				// Put the shell of a symbol in the symbol table
 				// in order to allow recursion
-  SleighSymbol *sym;
+  unique_ptr<SleighSymbol> sym;
   uint4 el = decoder.peekElement();
   if (el == sla::ELEM_USEROP_HEAD)
-    sym = new UserOpSymbol();
+    sym.reset(new UserOpSymbol());
   else if (el == sla::ELEM_EPSILON_SYM_HEAD)
-    sym = new EpsilonSymbol();
+    sym.reset(new EpsilonSymbol());
   else if (el == sla::ELEM_VALUE_SYM_HEAD)
-    sym = new ValueSymbol();
+    sym.reset(new ValueSymbol());
   else if (el == sla::ELEM_VALUEMAP_SYM_HEAD)
-    sym = new ValueMapSymbol();
+    sym.reset(new ValueMapSymbol());
   else if (el == sla::ELEM_NAME_SYM_HEAD)
-    sym = new NameSymbol();
+    sym.reset(new NameSymbol());
   else if (el == sla::ELEM_VARNODE_SYM_HEAD)
-    sym = new VarnodeSymbol();
+    sym.reset(new VarnodeSymbol());
   else if (el == sla::ELEM_CONTEXT_SYM_HEAD)
-    sym = new ContextSymbol();
+    sym.reset(new ContextSymbol());
   else if (el == sla::ELEM_VARLIST_SYM_HEAD)
-    sym = new VarnodeListSymbol();
+    sym.reset(new VarnodeListSymbol());
   else if (el == sla::ELEM_OPERAND_SYM_HEAD)
-    sym = new OperandSymbol();
+    sym.reset(new OperandSymbol());
   else if (el == sla::ELEM_START_SYM_HEAD)
-    sym = new StartSymbol();
+    sym.reset(new StartSymbol());
   else if (el == sla::ELEM_END_SYM_HEAD)
-    sym = new EndSymbol();
+    sym.reset(new EndSymbol());
   else if (el == sla::ELEM_NEXT2_SYM_HEAD)
-    sym = new Next2Symbol();
+    sym.reset(new Next2Symbol());
   else if (el == sla::ELEM_SUBTABLE_SYM_HEAD)
-    sym = new SubtableSymbol();
+    sym.reset(new SubtableSymbol());
   else
     throw SleighError("Bad symbol xml");
-  unique_ptr<SleighSymbol> usym(sym);
-  usym->decodeHeader(decoder);	// Restore basic elements of symbol
-  usym.release();
-  symbollist[sym->id] = sym;	// Put the basic symbol in the table
-  table[sym->scopeid]->addSymbol(sym); // to allow recursion
+
+  sym->decodeHeader(decoder);	// Restore basic elements of symbol
+
+  if (sym->id >= symbollist.size()) {
+    throw SleighError("Bad symbol id: exceeds symbollist size");
+  }
+
+  if (symbollist[sym->id] != (SleighSymbol *)0) {
+    throw SleighError("Bad symbol id: not unique");
+  }
+
+  if (sym->scopeid >= table.size()) {
+    throw SleighError("Bad symbol scope id: too large");
+  }
+
+  if (table[sym->scopeid] == (SymbolScope *)0) {
+    throw SleighError("Bad symbol scope id: undefined");
+  }
+
+  SleighSymbol *res = sym.release();
+  symbollist[res->id] = res;	// Put the basic symbol in the table
+  table[res->scopeid]->addSymbol(res); // to allow recursion
 }
 
 void SymbolTable::purge(void)
@@ -502,6 +533,9 @@ void ValueSymbol::encodeHeader(Encoder &encoder) const
 void ValueSymbol::decode(Decoder &decoder,SleighBase *trans)
 
 {
+  if (patval)
+    throw DecoderError("Already decoded symbol");
+
   patval = (PatternValue *) PatternExpression::decodeExpression(decoder,trans);
   patval->layClaim();
   decoder.closeElement(sla::ELEM_VALUE_SYM.getId());
@@ -583,6 +617,9 @@ void ValueMapSymbol::encodeHeader(Encoder &encoder) const
 void ValueMapSymbol::decode(Decoder &decoder,SleighBase *trans)
 
 {
+  if (patval)
+    throw DecoderError("Already decoded symbol");
+
   patval = (PatternValue *) PatternExpression::decodeExpression(decoder,trans);
   patval->layClaim();
   while(decoder.peekElement() != 0) {
@@ -662,6 +699,9 @@ void NameSymbol::encodeHeader(Encoder &encoder) const
 void NameSymbol::decode(Decoder &decoder,SleighBase *trans)
 
 {
+  if (patval)
+    throw DecoderError("Already decoded symbol");
+
   patval = (PatternValue *) PatternExpression::decodeExpression(decoder,trans);
   patval->layClaim();
   while(decoder.peekElement() != 0) {
@@ -796,6 +836,10 @@ void ContextSymbol::decode(Decoder &decoder,SleighBase *trans)
   if (lowMissing || highMissing) {
     throw DecoderError("Missing high/low attributes");
   }
+
+  if (patval)
+    throw DecoderError("Already decoded symbol");
+
   patval = (PatternValue *) PatternExpression::decodeExpression(decoder,trans);
   patval->layClaim();
   decoder.closeElement(sla::ELEM_CONTEXT_SYM.getId());
@@ -900,6 +944,9 @@ void VarnodeListSymbol::encodeHeader(Encoder &encoder) const
 void VarnodeListSymbol::decode(Decoder &decoder,SleighBase *trans)
 
 {
+  if (patval)
+    throw DecoderError("Already decoded symbol");
+
   patval = (PatternValue *) PatternExpression::decodeExpression(decoder,trans);
   patval->layClaim();
   while(decoder.peekElement() != 0) {
@@ -947,7 +994,9 @@ void OperandSymbol::defineOperand(TripleSymbol *tri)
 OperandSymbol::~OperandSymbol(void)
 
 {
-  PatternExpression::release(localexp);
+  if (localexp != (PatternExpression *)0)
+    PatternExpression::release(localexp);
+
   if (defexp != (PatternExpression *)0)
     PatternExpression::release(defexp);
 }
@@ -1042,6 +1091,9 @@ void OperandSymbol::encodeHeader(Encoder &encoder) const
 void OperandSymbol::decode(Decoder &decoder,SleighBase *trans)
 
 {
+  if (defexp || localexp)
+    throw DecoderError("Already decoded symbol");
+
   defexp = (PatternExpression *)0;
   triple = (TripleSymbol *)0;
   flags = 0;
@@ -2336,6 +2388,9 @@ void DecisionNode::decode(Decoder &decoder,DecisionNode *par,SubtableSymbol *sub
     if (subel == sla::ELEM_PAIR) {
       decoder.openElement();
       uintm id = decoder.readSignedInteger(sla::ATTRIB_ID);
+      if (id >= sub->getNumConstructors()) {
+        throw DecoderError("Invalid constructor id");
+      }
       Constructor *ct = sub->getConstructor(id);
       DisjointPattern *pat = DisjointPattern::decodeDisjoint(decoder);
       list.push_back(pair<DisjointPattern *,Constructor *>(pat,ct));

Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.hh

@@ -326,7 +326,7 @@ private:
   void setVariableLength(void) { flags |= variable_len; }
   bool isVariableLength(void) const { return ((flags&variable_len)!=0); }
 public:
-  OperandSymbol(void) {}	// For use with decode
+  OperandSymbol(void) : localexp(nullptr), defexp(nullptr) {}	// For use with decode
   OperandSymbol(const string &nm,int4 index,Constructor *ct);
   uint4 getRelativeOffset(void) const { return reloffset; }
   int4 getOffsetBase(void) const { return offsetbase; }
@@ -448,8 +448,8 @@ class ContextOp : public ContextChange {
   int4 shift;			// Number of bits to shift value into place
 public:
   ContextOp(int4 startbit,int4 endbit,PatternExpression *pe);
-  ContextOp(void) {}		// For use with decode
-  virtual ~ContextOp(void) { PatternExpression::release(patexp); }
+  ContextOp(void) : patexp(nullptr) {}		// For use with decode
+  virtual ~ContextOp(void) { if (patexp) PatternExpression::release(patexp); }
   virtual void validate(void) const;
   virtual void encode(Encoder &encoder) const;
   virtual void decode(Decoder &decoder,SleighBase *trans);