Fixes for SLA file parsing bugs uncovered during fuzzing

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

@@ -23,6 +23,7 @@
 #ifdef CPUI_STATISTICS
 #include <cmath>
 #endif
+#include <memory>
 
 namespace ghidra {
 
@@ -741,23 +742,24 @@ void Architecture::decodeDynamicRule(Decoder &decoder)
 ProtoModel *Architecture::decodeProto(Decoder &decoder)
 
 {
-  ProtoModel *res;
+  std::unique_ptr<ProtoModel> model;
   uint4 elemId = decoder.peekElement();
   if (elemId == ELEM_PROTOTYPE)
-    res = new ProtoModel(this);
+    model = std::unique_ptr<ProtoModel>(new ProtoModel(this));
   else if (elemId == ELEM_RESOLVEPROTOTYPE)
-    res = new ProtoModelMerged(this);
+    model = std::unique_ptr<ProtoModel>(new ProtoModelMerged(this));
   else
     throw LowlevelError("Expecting <prototype> or <resolveprototype> tag");
 
-  res->decode(decoder);
+  model->decode(decoder);
 
-  ProtoModel *other = getModel(res->getName());
+  ProtoModel *other = getModel(model->getName());
   if (other != (ProtoModel *)0) {
-    string errMsg = "Duplicate ProtoModel name: " + res->getName();
-    delete res;
+    string errMsg = "Duplicate ProtoModel name: " + model->getName();
     throw LowlevelError(errMsg);
   }
+
+  ProtoModel* res = model.release();
   protoModels[res->getName()] = res;
   return res;
 }

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

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 #include "funcdata.hh"
+#include <memory>
 
 namespace ghidra {
 
@@ -615,9 +616,9 @@ void Funcdata::decodeJumpTable(Decoder &decoder)
 {
   uint4 elemId = decoder.openElement(ELEM_JUMPTABLELIST);
   while(decoder.peekElement() != 0) {
-    JumpTable *jt = new JumpTable(glb);
+    std::unique_ptr<JumpTable> jt = std::unique_ptr<JumpTable>(new JumpTable(glb));
     jt->decode(decoder);
-    jumpvec.push_back(jt);
+    jumpvec.push_back(jt.release());
   }
   decoder.closeElement(elemId);
 }

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/modelrules.cc

@@ -15,6 +15,7 @@
  */
 #include "modelrules.hh"
 #include "funcdata.hh"
+#include <memory>
 
 namespace ghidra {
 
@@ -252,23 +253,23 @@ PrimitiveExtractor::PrimitiveExtractor(Datatype *dt,bool unionIllegal,int offset
 DatatypeFilter *DatatypeFilter::decodeFilter(Decoder &decoder)
 
 {
-  DatatypeFilter *filter;
+  std::unique_ptr<DatatypeFilter> filter;
   uint4 elemId = decoder.openElement(ELEM_DATATYPE);
   string nm = decoder.readString(ATTRIB_NAME);
   if (nm == "any") {
-    filter = new SizeRestrictedFilter();
+    filter = std::unique_ptr<DatatypeFilter>(new SizeRestrictedFilter());
   }
   else if (nm == "homogeneous-float-aggregate") {
-    filter = new HomogeneousAggregate(TYPE_FLOAT,4,0,0);
+    filter = std::unique_ptr<DatatypeFilter>(new HomogeneousAggregate(TYPE_FLOAT,4,0,0));
   }
   else {
     // If no other name matches, assume this is a metatype
     type_metatype meta = string2metatype(nm);
-    filter = new MetaTypeFilter(meta);
+    filter = std::unique_ptr<DatatypeFilter>(new MetaTypeFilter(meta));
   }
   filter->decode(decoder);
   decoder.closeElement(elemId);
-  return filter;
+  return filter.release();
 }
 
 /// Parse the given string as a comma or space separated list of decimal integers,
@@ -451,18 +452,18 @@ void HomogeneousAggregate::decode(Decoder &decoder)
 QualifierFilter *QualifierFilter::decodeFilter(Decoder &decoder)
 
 {
-  QualifierFilter *filter;
+  std::unique_ptr<QualifierFilter> filter;
   uint4 elemId = decoder.peekElement();
   if (elemId == ELEM_VARARGS)
-    filter = new VarargsFilter();
+    filter = std::unique_ptr<QualifierFilter>(new VarargsFilter());
   else if (elemId == ELEM_POSITION)
-    filter = new PositionMatchFilter(-1);
+    filter = std::unique_ptr<QualifierFilter>(new PositionMatchFilter(-1));
   else if (elemId == ELEM_DATATYPE_AT)
-    filter = new DatatypeMatchFilter();
+    filter = std::unique_ptr<QualifierFilter>(new DatatypeMatchFilter());
   else
     return (QualifierFilter *)0;
   filter->decode(decoder);
-  return filter;
+  return filter.release();
 }
 
 /// The AndFilter assumes ownership of all the filters in the array and the original vector is cleared
@@ -592,32 +593,32 @@ bool AssignAction::fillinOutputMap(ParamActive *active) const
 AssignAction *AssignAction::decodeAction(Decoder &decoder,const ParamListStandard *res)
 
 {
-  AssignAction *action;
+  std::unique_ptr<AssignAction> action;
   uint4 elemId = decoder.peekElement();
   if (elemId == ELEM_GOTO_STACK)
-    action = new GotoStack(res,0);
+    action = std::unique_ptr<AssignAction>(new GotoStack(res,0));
   else if (elemId == ELEM_JOIN) {
-    action = new MultiSlotAssign(res);
+    action = std::unique_ptr<AssignAction>(new MultiSlotAssign(res));
   }
   else if (elemId == ELEM_CONSUME) {
-    action = new ConsumeAs(TYPECLASS_GENERAL,res);
+    action = std::unique_ptr<AssignAction>(new ConsumeAs(TYPECLASS_GENERAL,res));
   }
   else if (elemId == ELEM_CONVERT_TO_PTR) {
-    action = new ConvertToPointer(res);
+    action = std::unique_ptr<AssignAction>(new ConvertToPointer(res));
   }
   else if (elemId == ELEM_HIDDEN_RETURN) {
-    action = new HiddenReturnAssign(res,hiddenret_specialreg);
+    action = std::unique_ptr<AssignAction>(new HiddenReturnAssign(res,hiddenret_specialreg));
   }
   else if (elemId == ELEM_JOIN_PER_PRIMITIVE) {
-    action = new MultiMemberAssign(TYPECLASS_GENERAL,false,res->isBigEndian(),res);
+    action = std::unique_ptr<AssignAction>(new MultiMemberAssign(TYPECLASS_GENERAL,false,res->isBigEndian(),res));
   }
   else if (elemId == ELEM_JOIN_DUAL_CLASS) {
-    action = new MultiSlotDualAssign(res);
+    action = std::unique_ptr<AssignAction>(new MultiSlotDualAssign(res));
   }
   else
     throw DecoderError("Expecting model rule action");
   action->decode(decoder);
-  return action;
+  return action.release();
 }
 
 /// \brief Read the next model rule precondition element from the stream
@@ -653,22 +654,22 @@ AssignAction *AssignAction::decodePrecondition(Decoder &decoder,const ParamListS
 AssignAction *AssignAction::decodeSideeffect(Decoder &decoder,const ParamListStandard *res)
 
 {
-  AssignAction *action;
+  std::unique_ptr<AssignAction> action;
   uint4 elemId = decoder.peekElement();
 
   if (elemId == ELEM_CONSUME_EXTRA) {
-    action = new ConsumeExtra(res);
+    action = std::unique_ptr<AssignAction>(new ConsumeExtra(res));
   }
   else if (elemId == ELEM_EXTRA_STACK) {
-    action = new ExtraStack(res);
+    action = std::unique_ptr<AssignAction>(new ExtraStack(res));
   }
   else if (elemId == ELEM_CONSUME_REMAINING) {
-	action = new ConsumeRemaining(res);
+	action = std::unique_ptr<AssignAction>(new ConsumeRemaining(res));
   }
   else
     throw DecoderError("Expecting model rule sideeffect");
   action->decode(decoder);
-  return action;
+  return action.release();
 }
 
 /// \brief Truncate a tiling by a given number of bytes

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

@@ -15,6 +15,7 @@
  */
 #include "override.hh"
 #include "funcdata.hh"
+#include <memory>
 
 namespace ghidra {
 
@@ -367,10 +368,10 @@ void Override::decode(Decoder &decoder,Architecture *glb)
     }
     else if (subId == ELEM_PROTOOVERRIDE) {
       Address callpoint = Address::decode(decoder);
-      FuncProto *fp = new FuncProto();
+      std::unique_ptr<FuncProto> fp(new FuncProto());
       fp->setInternal(glb->defaultfp,glb->types->getTypeVoid());
       fp->decode(decoder,glb);
-      insertProtoOverride(callpoint,fp);
+      insertProtoOverride(callpoint,fp.release());
     }
     else if (subId == ELEM_FORCEGOTO) {
       Address targetpc = Address::decode(decoder);

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

@@ -15,6 +15,7 @@
  */
 #include "semantics.hh"
 #include "translate.hh"
+#include <memory>
 
 namespace ghidra {
 
@@ -720,9 +721,9 @@ void OpTpl::decode(Decoder &decoder)
     output->decode(decoder);
   }
   while(decoder.peekElement() != 0) {
-    VarnodeTpl *vn = new VarnodeTpl();
+    std::unique_ptr<VarnodeTpl> vn(new VarnodeTpl());
     vn->decode(decoder);
-    input.push_back(vn);
+    input.push_back(vn.release());
   }
   decoder.closeElement(el);
 }
@@ -914,9 +915,9 @@ int4 ConstructTpl::decode(Decoder &decoder)
     result->decode(decoder);
   }
   while(decoder.peekElement() != 0) {
-    OpTpl *op = new OpTpl();
+    std::unique_ptr<OpTpl> op(new OpTpl());
     op->decode(decoder);
-    vec.push_back(op);
+    vec.push_back(op.release());
   }
   decoder.closeElement(el);
   return sectionid;

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

@@ -15,6 +15,7 @@
  */
 #include "slghpatexpress.hh"
 #include "sleighbase.hh"
+#include <memory>
 
 namespace ghidra {
 
@@ -503,10 +504,12 @@ 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");
 
-  res->decode(decoder,trans);
-  return res;
+  // Call PatternExpression::release on decoding failure
+  std::unique_ptr<PatternExpression, void(*)(PatternExpression *)> patexp(res, PatternExpression::release);
+  patexp->decode(decoder, trans);
+  return patexp.release();
 }
 
 static intb getInstructionBytes(ParserWalker &walker,int4 bytestart,int4 byteend,bool bigendian)
@@ -826,6 +829,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/slghpattern.cc

@@ -15,6 +15,7 @@
  */
 #include "slghpattern.hh"
 #include "slaformat.hh"
+#include <memory>
 
 namespace ghidra {
 
@@ -141,16 +142,16 @@ bool DisjointPattern::resolvesIntersect(const DisjointPattern *op1,const Disjoin
 DisjointPattern *DisjointPattern::decodeDisjoint(Decoder &decoder)
 
 {				// DisjointPattern factory
-  DisjointPattern *res;
+  std::unique_ptr<DisjointPattern> res;
   uint4 el = decoder.peekElement();
   if (el == sla::ELEM_INSTRUCT_PAT)
-    res = new InstructionPattern();
+    res = std::unique_ptr<InstructionPattern>(new InstructionPattern());
   else if (el == sla::ELEM_CONTEXT_PAT)
-    res = new ContextPattern();
+    res = std::unique_ptr<ContextPattern>(new ContextPattern());
   else
-    res = new CombinePattern();
+    res = std::unique_ptr<CombinePattern>(new CombinePattern());
   res->decode(decoder);
-  return res;
+  return res.release();
 }
 
 void PatternBlock::normalize(void)