diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/marshal.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/marshal.cc index f1a43d865a..9d768cb706 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/marshal.cc +++ b/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"); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/marshal.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/marshal.hh index 3b0ec0e4cc..9817f7b013 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/marshal.hh +++ b/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) { diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/semantics.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/semantics.hh index b53b18797d..2a768a7bef 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/semantics.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/semantics.hh @@ -142,7 +142,7 @@ class OpTpl { OpCode opc; vector input; public: - OpTpl(void) {} + OpTpl(void) : output(nullptr) {} OpTpl(OpCode oc) { opc = oc; output = (VarnodeTpl *)0; } ~OpTpl(void); VarnodeTpl *getOut(void) const { return output; } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/slghpatexpress.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/slghpatexpress.cc index 03bc815c9a..a724f3013f 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/slghpatexpress.cc +++ b/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(sleigh->findSymbol(tabid)); + if (ctid >= tab->getNumConstructors()) { + throw DecoderError("Invalid constructor id"); + } ct = tab->getConstructor(ctid); decoder.closeElement(el); } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc index f77a4a0aa3..d864b25d18 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc @@ -49,10 +49,12 @@ SymbolTable::~SymbolTable(void) { vector::iterator iter; for(iter=table.begin();iter!=table.end();++iter) - delete *iter; + if (*iter) + delete *iter; vector::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()) { + 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 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 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(pat,ct)); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.hh index 5e8b4d3dfd..eda7965f5c 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.hh +++ b/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);