diff --git a/libIRDB/include/decode/decode.hpp b/libIRDB/include/decode/decode.hpp index 8dd3ff32f4ce72ca160824d80828eda4238c0d9b..331618fc5f4f1cb68dd32e99b29e5430ea1fda67 100644 --- a/libIRDB/include/decode/decode.hpp +++ b/libIRDB/include/decode/decode.hpp @@ -2,6 +2,7 @@ #define libirdb_decode_hpp #include <stdint.h> +#include <vector> namespace libIRDB { @@ -27,18 +28,21 @@ class DecodedInstruction_t bool valid() const; uint32_t length() const; bool isBranch() const; + bool isCall() const; bool isUnconditionalBranch() const; bool isConditionalBranch() const; bool isReturn() const; string getMnemonic() const; virtual_offset_t getImmediate() const; virtual_offset_t getAddress() const; - - - + bool setsStackPointer() const; + uint32_t getPrefixCount() const; + virtual_offset_t getMemoryDisplacementOffset(const DecodedOperand_t& t) const; // 0-based. first operand is numbered 0. + bool hasOperand(const int op_num) const; DecodedOperand_t getOperand(const int op_num) const; + vector<DecodedOperand_t> getOperands() const; private: diff --git a/libIRDB/include/decode/operand.hpp b/libIRDB/include/decode/operand.hpp index 797c0e0878a5fa48733abae795dd17cf2ab77fd7..f7faa935a35c1f3286f31b49c731939d913cbe63 100644 --- a/libIRDB/include/decode/operand.hpp +++ b/libIRDB/include/decode/operand.hpp @@ -29,6 +29,8 @@ class DecodedOperand_t virtual_offset_t getMemoryDisplacement() const; bool isPcrel() const; uint32_t getScaleValue() const; + uint32_t getMemoryDisplacementEncodingSize() const; + diff --git a/libIRDB/src/decode/decode.cpp b/libIRDB/src/decode/decode.cpp index a21f2d647cb13a94ed22aa3f8e6b14d616741262..29524477eac604b839da9a0b1e2d0a05fb8a3c29 100644 --- a/libIRDB/src/decode/decode.cpp +++ b/libIRDB/src/decode/decode.cpp @@ -6,6 +6,20 @@ using namespace libIRDB; using namespace std; +// static functions +static inline bool my_SetsStackPointer(const ARGTYPE* arg) +{ + if((arg->AccessMode & WRITE ) == 0) + return false; + int access_type=arg->ArgType; + + if(access_type==REGISTER_TYPE + GENERAL_REG +REG4) + return true; + return false; + +} + +// constructors, destructors, operators. DecodedInstruction_t::DecodedInstruction_t(const Instruction_t* i) { @@ -54,6 +68,8 @@ DecodedInstruction_t& DecodedInstruction_t::operator=(const DecodedInstruction_t return *this; } +// private methods + void DecodedInstruction_t::Disassemble(const virtual_offset_t start_addr, const void *data, uint32_t max_len) { DISASM* d=static_cast<DISASM*>(disasm_data); @@ -67,6 +83,8 @@ void DecodedInstruction_t::Disassemble(const virtual_offset_t start_addr, const } +// public methods + string DecodedInstruction_t::getDisassembly() const { assert(valid()); @@ -92,6 +110,12 @@ bool DecodedInstruction_t::isBranch() const return d->Instruction.BranchType!=0; } +bool DecodedInstruction_t::isCall() const +{ + DISASM* d=static_cast<DISASM*>(disasm_data); + return d->Instruction.BranchType==CallType; +} + bool DecodedInstruction_t::isUnconditionalBranch() const { DISASM* d=static_cast<DISASM*>(disasm_data); @@ -111,9 +135,31 @@ bool DecodedInstruction_t::isReturn() const } +bool DecodedInstruction_t::hasOperand(const int op_num) const +{ + DISASM* d=static_cast<DISASM*>(disasm_data); + switch(op_num+1) + { + case 1: + return d->Argument1.ArgType!=NO_ARGUMENT; + case 2: + return d->Argument2.ArgType!=NO_ARGUMENT; + case 3: + return d->Argument3.ArgType!=NO_ARGUMENT; + case 4: + return d->Argument4.ArgType!=NO_ARGUMENT; + default: + return false; + } +} + + // 0-based. first operand is numbered 0. DecodedOperand_t DecodedInstruction_t::getOperand(const int op_num) const { + if(!hasOperand(op_num)) + throw std::out_of_range("op_num"); + DISASM* d=static_cast<DISASM*>(disasm_data); switch(op_num+1) { @@ -125,11 +171,22 @@ DecodedOperand_t DecodedInstruction_t::getOperand(const int op_num) const return DecodedOperand_t(&d->Argument3); case 4: return DecodedOperand_t(&d->Argument4); - default: - throw std::out_of_range("op_num"); } } +vector<DecodedOperand_t> DecodedInstruction_t::getOperands() const +{ + auto ret_val=vector<DecodedOperand_t>(); + for(auto i=0;i<4;i++) + { + if(hasOperand(i)) + ret_val.push_back(getOperand(i)); + else + break; + } + return ret_val; +} + string DecodedInstruction_t::getMnemonic() const { @@ -152,3 +209,39 @@ virtual_offset_t DecodedInstruction_t::getAddress() const DISASM* d=static_cast<DISASM*>(disasm_data); return d->Instruction.AddrValue; } + + +bool DecodedInstruction_t::setsStackPointer() const +{ + DISASM* d=static_cast<DISASM*>(disasm_data); + + if(strstr(d->Instruction.Mnemonic, "push")!=NULL) + return true; + if(strstr(d->Instruction.Mnemonic, "pop")!=NULL) + return true; + if(strstr(d->Instruction.Mnemonic, "call")!=NULL) + return true; + if(d->Instruction.ImplicitModifiedRegs==REGISTER_TYPE+GENERAL_REG+REG4) + return true; + + if(my_SetsStackPointer(&d->Argument1)) return true; + if(my_SetsStackPointer(&d->Argument2)) return true; + if(my_SetsStackPointer(&d->Argument3)) return true; + if(my_SetsStackPointer(&d->Argument4)) return true; + + return false; +} + +uint32_t DecodedInstruction_t::getPrefixCount() const +{ + DISASM* d=static_cast<DISASM*>(disasm_data); + return d->Prefix.Number; +} + +virtual_offset_t DecodedInstruction_t::getMemoryDisplacementOffset(const DecodedOperand_t& t) const +{ + DISASM* d=static_cast<DISASM*>(disasm_data); + ARGTYPE* the_arg=static_cast<ARGTYPE*>(t.arg_data); + return the_arg->Memory.DisplacementAddr-d->EIP; +} + diff --git a/libIRDB/src/decode/operand.cpp b/libIRDB/src/decode/operand.cpp index b2d3c81251681e96ecbe7fe9a5cd146cc118f7f3..2d8118b745b9ca1805535ac55dd4b32d124ed7b7 100644 --- a/libIRDB/src/decode/operand.cpp +++ b/libIRDB/src/decode/operand.cpp @@ -166,3 +166,9 @@ bool DecodedOperand_t::isPcrel() const ARGTYPE *t=static_cast<ARGTYPE*>(arg_data); return (t->ArgType&RELATIVE_)==RELATIVE_; } + +uint32_t DecodedOperand_t::getMemoryDisplacementEncodingSize() const +{ + ARGTYPE *t=static_cast<ARGTYPE*>(arg_data); + return t->Memory.DisplacementSize; +} diff --git a/libIRDB/test/fill_in_indtargs.cpp b/libIRDB/test/fill_in_indtargs.cpp index 869a9b0a6302e2c1260c23bbbf8d0bf7a4a6779d..6df80cfdd40f6187ce8a01db7fdf04bb7527bb8a 100644 --- a/libIRDB/test/fill_in_indtargs.cpp +++ b/libIRDB/test/fill_in_indtargs.cpp @@ -442,14 +442,15 @@ void get_instruction_targets(FileIR_t *firp, EXEIO::exeio* elfiop, const set<vir } /* otherwise, any immediate is a possible branch target */ possible_target(disasm.getImmediate() /* Instruction.Immediat*/ ,0, prov); - const auto op0=disasm.getOperand(0); - const auto op1=disasm.getOperand(1); - const auto op2=disasm.getOperand(2); - const auto op3=disasm.getOperand(3); - handle_argument(&op0, insn, prov); - handle_argument(&op1, insn, prov); - handle_argument(&op2, insn, prov); - handle_argument(&op3, insn, prov); + + for(auto i=0;i<4;i++) + { + if(disasm.hasOperand(i)) + { + const auto op=disasm.getOperand(i); + handle_argument(&op, insn, prov); + } + } } } @@ -1264,7 +1265,11 @@ Note: Here the operands of the add are reversed, so lookup code was not finding table_index_str += ")"; const auto cmp_str = string("cmp ") + disasm.getOperand(0).getString(); //Argument1.ArgMnemonic; - const auto cmp_str2 = string("cmp ") + disasm.getOperand(1).getString(); //Argument2.ArgMnemonic; + + // this was completely broken because argument2 had a null mnemonic, which we found out because getOperand(1) threw an exception. + // i suspect it's attempting to find a compare of operand1 on the RHS of a compare, but i need better regex foo to get that. + // for now, repeat what was working. + const auto cmp_str2 = string("cmp "); // + disasm.getOperand(1).getString(); //Argument2.ArgMnemonic; if(!backup_until(table_index_str.c_str(), I7, I8)) return; @@ -2295,7 +2300,7 @@ void setup_icfs(FileIR_t* firp, EXEIO::exeio* elfiop) } -void unpin_elf_tables(FileIR_t *firp, int do_unpin_opt) +void unpin_elf_tables(FileIR_t *firp, int64_t do_unpin_opt) { map<string,int> unpin_counts; map<string,int> missed_unpins; @@ -2530,7 +2535,7 @@ DataScoop_t* find_scoop(FileIR_t *firp, const virtual_offset_t &vo) int type3_unpins=0; int type3_pins=0; -void unpin_type3_switchtable(FileIR_t* firp,Instruction_t* insn,DataScoop_t* scoop, int do_unpin_opt) +void unpin_type3_switchtable(FileIR_t* firp,Instruction_t* insn,DataScoop_t* scoop, int64_t do_unpin_opt) { assert(firp && insn && scoop); @@ -2697,7 +2702,7 @@ void print_unpins(FileIR_t *firp) } -void unpin_well_analyzed_ibts(FileIR_t *firp, int do_unpin_opt) +void unpin_well_analyzed_ibts(FileIR_t *firp, int64_t do_unpin_opt) { unpin_elf_tables(firp, do_unpin_opt); unpin_switches(firp, do_unpin_opt); @@ -2709,7 +2714,7 @@ void unpin_well_analyzed_ibts(FileIR_t *firp, int do_unpin_opt) /* * fill_in_indtargs - main driver routine for */ -void fill_in_indtargs(FileIR_t* firp, exeio* elfiop, std::list<virtual_offset_t> forced_pins, int do_unpin_opt) +void fill_in_indtargs(FileIR_t* firp, exeio* elfiop, std::list<virtual_offset_t> forced_pins, int64_t do_unpin_opt) { set<virtual_offset_t> thunk_bases; find_all_module_starts(firp,thunk_bases); @@ -2782,7 +2787,7 @@ void fill_in_indtargs(FileIR_t* firp, exeio* elfiop, std::list<virtual_offset_t> setup_icfs(firp, elfiop); // do unpinning of well analyzed ibts. - if(do_unpin_opt!=-1) + if(do_unpin_opt!=(int64_t)-1) unpin_well_analyzed_ibts(firp, do_unpin_opt); } @@ -2792,7 +2797,7 @@ main(int argc, char* argv[]) { auto argc_iter = (int)2; auto split_eh_frame_opt=true; - auto do_unpin_opt=-1; + auto do_unpin_opt=numeric_limits<int64_t>::max() ; auto forced_pins=std::list<virtual_offset_t> (); if(argc<2) diff --git a/libIRDB/test/fix_calls.cpp b/libIRDB/test/fix_calls.cpp index 4d34cf9850d7da90d7d922f6556a0cef36b9b21e..4b062d0e936c8add18614e1ae71b52514d0fc884 100644 --- a/libIRDB/test/fix_calls.cpp +++ b/libIRDB/test/fix_calls.cpp @@ -33,7 +33,8 @@ #include <exeio.h> #include "fill_in_indtargs.hpp" -#include <bea_deprecated.hpp> +#include <libIRDB-decode.hpp> +//#include <bea_deprecated.hpp> using namespace libIRDB; @@ -41,6 +42,7 @@ using namespace std; using namespace EXEIO; +#define ALLOF(a) begin(a),end(a) class Range_t { @@ -105,10 +107,11 @@ bool check_entry(bool &found, ControlFlowGraph_t* cfg) ++it ) { - DISASM disasm; Instruction_t* insn=*it; - Disassemble(insn,disasm); - if(SetsStackPointer(&disasm)) { + //DISASM disasm; + //Disassemble(insn,disasm); + DecodedInstruction_t disasm(insn); + if(disasm.setsStackPointer()) { return false; } else { if(getenv("VERBOSE_FIX_CALLS")) @@ -121,7 +124,7 @@ bool check_entry(bool &found, ControlFlowGraph_t* cfg) } } - if(strstr(disasm.CompleteInstr, "[esp]")) + if(strstr(disasm.getDisassembly().c_str()/* disasm.CompleteInstr*/, "[esp]")) { found=true; if(getenv("VERBOSE_FIX_CALLS")) @@ -155,7 +158,7 @@ bool call_needs_fix(Instruction_t* insn) Instruction_t *target=insn->GetTarget(); Instruction_t *fallthru=insn->GetFallthrough(); - DISASM disasm; + //DISASM disasm; string pattern; @@ -203,8 +206,9 @@ bool call_needs_fix(Instruction_t* insn) if(!target) { /* call 0's aren't to real locations */ - Disassemble(insn,disasm); - if(strcmp(disasm.CompleteInstr, "call 0x00000000")==0) + // Disassemble(insn,disasm); + DecodedInstruction_t disasm(insn); + if(strcmp(disasm.getDisassembly().c_str()/*CompleteInstr*/, "call 0x00000000")==0) { return false; } @@ -306,9 +310,10 @@ bool call_needs_fix(Instruction_t* insn) { Instruction_t* itrinsn=*it; /* if the disassembly contains the string mentioned */ - DISASM disasm; - Disassemble(itrinsn,disasm); - if(strstr(disasm.CompleteInstr, pattern.c_str())!=NULL) + //DISASM disasm; + //Disassemble(itrinsn,disasm); + DecodedInstruction_t disasm(itrinsn); + if(strstr(disasm.getDisassembly().c_str() /*disasm.CompleteInstr*/, pattern.c_str())!=NULL) { found_pattern++; if(getenv("VERBOSE_FIX_CALLS")) @@ -460,10 +465,12 @@ string adjust_esp_offset(string newbits, int offset) static void convert_to_jump(Instruction_t* insn, int offset) { string newbits=insn->GetDataBits(); - DISASM d; - Disassemble(insn,d); + //DISASM d; + //Disassemble(insn,d); + DecodedInstruction_t d(insn); + /* this case is odd, handle it specially (and more easily to understand) */ - if(strcmp(d.CompleteInstr, "call qword [rsp]")==0) + if(strcmp(d.getDisassembly().c_str()/*.CompleteInstr*/, "call qword [rsp]")==0) { char buf[100]; sprintf(buf,"jmp qword [rsp+%d]", offset); @@ -473,7 +480,7 @@ static void convert_to_jump(Instruction_t* insn, int offset) /* skip over any prefixes */ - int op_index=d.Prefix.Number; + int op_index=d.getPrefixCount(); // d.Prefix.Number; // on the opcode. assume no prefix here switch((unsigned char)newbits[op_index]) @@ -545,10 +552,11 @@ void fix_call(Instruction_t* insn, FileIR_t *firp, bool can_unpin) bool has_rex=false; /* disassemble */ - DISASM disasm; + //DISASM disasm; + DecodedInstruction_t disasm(insn); /* Disassemble the instruction */ - int instr_len = Disassemble(insn,disasm); + int instr_len = disasm.length(); // Disassemble(insn,disasm); /* if this instruction is an inserted call instruction than we don't need to @@ -576,7 +584,7 @@ void fix_call(Instruction_t* insn, FileIR_t *firp, bool can_unpin) if(getenv("VERBOSE_FIX_CALLS")) { - cout<<"Doing a fix_call on "<<std::hex<<insn->GetAddress()->GetVirtualOffset()<< " which is "<<disasm.CompleteInstr<<endl; + cout<<"Doing a fix_call on "<<std::hex<<insn->GetAddress()->GetVirtualOffset()<< " which is "<<disasm.getDisassembly() /*.CompleteInstr*/<<endl; } @@ -696,20 +704,12 @@ void fix_call(Instruction_t* insn, FileIR_t *firp, bool can_unpin) bool is_call(Instruction_t* insn) { - DISASM disasm; - memset(&disasm, 0, sizeof(DISASM)); -#if 0 - disasm.Options = NasmSyntax + PrefixedNumeral; - disasm.Archi = 32; - disasm.EIP = (UIntPtr)insn->GetDataBits().c_str(); - disasm.VirtualAddr = insn->GetAddress()->GetVirtualOffset(); -#endif - /* Disassemble the instruction */ - int instr_len = Disassemble(insn,disasm); + DecodedInstruction_t disasm(insn); + //int instr_len = disasm.length(); // Disassemble(insn,disasm); - return (disasm.Instruction.BranchType==CallType); + return disasm.isCall(); // (disasm.Instruction.BranchType==CallType); } bool can_skip_safe_function(Instruction_t *call_insn) @@ -891,9 +891,13 @@ bool arg_has_relative(const ARGTYPE &arg) // void fix_other_pcrel(FileIR_t* firp, Instruction_t *insn, UIntPtr virt_offset) { - DISASM disasm; - Disassemble(insn,disasm); - int is_rel= arg_has_relative(disasm.Argument1) || arg_has_relative(disasm.Argument2) || arg_has_relative(disasm.Argument3); + //DISASM disasm; + //Disassemble(insn,disasm); + DecodedInstruction_t disasm(insn); + const auto &operands=disasm.getOperands(); + const auto relop_it=find_if(ALLOF(operands),[](const DecodedOperand_t& op) + { return op.isMemory() && op.isPcrel() ; } ); + const bool is_rel= relop_it!=operands.end(); /* if this has already been fixed, we can skip it */ if(virt_offset==0 || virt_offset==-1) @@ -901,26 +905,19 @@ void fix_other_pcrel(FileIR_t* firp, Instruction_t *insn, UIntPtr virt_offset) if(is_rel) { - ARGTYPE* the_arg=NULL; - if(arg_has_relative(disasm.Argument1)) - the_arg=&disasm.Argument1; - if(arg_has_relative(disasm.Argument2)) - the_arg=&disasm.Argument2; - if(arg_has_relative(disasm.Argument3)) - the_arg=&disasm.Argument3; - assert(the_arg); + const auto the_arg=*relop_it; - int offset=the_arg->Memory.DisplacementAddr-disasm.EIP; + int offset=disasm.getMemoryDisplacementOffset(the_arg); /*the_arg->Memory.DisplacementAddr-disasm.EIP*/; assert(offset>=0 && offset <=15); - int size=the_arg->Memory.DisplacementSize; + int size=the_arg.getMemoryDisplacementEncodingSize(); // the_arg->Memory.DisplacementSize; assert(size==1 || size==2 || size==4 || size==8); if(getenv("VERBOSE_FIX_CALLS")) { - cout<<"Found insn with pcrel memory operand: "<<disasm.CompleteInstr - <<" Displacement="<<std::hex<<the_arg->Memory.Displacement<<std::dec - <<" size="<<the_arg->Memory.DisplacementSize<<" Offset="<<offset; + cout<<"Found insn with pcrel memory operand: "<<disasm.getDisassembly()/*.CompleteInstr */ + <<" Displacement="<<std::hex<<the_arg.getMemoryDisplacement() /*the_arg->Memory.Displacement*/<<std::dec + <<" size="<<the_arg.getMemoryDisplacementEncodingSize() /*the_arg->Memory.DisplacementSize*/<<" Offset="<<offset; } /* convert [rip_pc+displacement] addresssing mode into [rip_0+displacement] where rip_pc is the actual PC of the insn, @@ -934,7 +931,7 @@ void fix_other_pcrel(FileIR_t* firp, Instruction_t *insn, UIntPtr virt_offset) memcpy(cstr,data.c_str(), data.length()); void *offsetptr=&cstr[offset]; - UIntPtr disp=the_arg->Memory.Displacement; + UIntPtr disp=the_arg.getMemoryDisplacement(); // ->Memory.Displacement; UIntPtr oldpc=virt_offset; UIntPtr newdisp=disp+oldpc; @@ -967,9 +964,9 @@ void fix_other_pcrel(FileIR_t* firp, Instruction_t *insn, UIntPtr virt_offset) insn->GetRelocations().insert(reloc); firp->GetRelocations().insert(reloc); - Disassemble(insn,disasm); + disasm=DecodedInstruction_t(insn); if(getenv("VERBOSE_FIX_CALLS")) - cout<<" Converted to: "<<disasm.CompleteInstr<<endl; + cout<<" Converted to: "<<disasm.getDisassembly() /*CompleteInstr*/<<endl; } } diff --git a/tools/selective_cfi/SConscript b/tools/selective_cfi/SConscript index 8f1007abc1696539ebeb7d387a920e5edf5ad9ac..8c95d4e0cbb3e9dda55ce48881416e9e6355bf13 100644 --- a/tools/selective_cfi/SConscript +++ b/tools/selective_cfi/SConscript @@ -23,7 +23,7 @@ myenv.Append(CXXFLAGS = " -std=c++11 -Wall ") pgm="selective_cfi.exe" LIBPATH="$SECURITY_TRANSFORMS_HOME/lib" -LIBS=Split( env.subst('$BASE_IRDB_LIBS')+ " IRDB-core IRDB-cfg IRDB-util transform rewrite MEDSannotation pqxx pq") +LIBS=Split( env.subst('$BASE_IRDB_LIBS')+ " IRDB-core IRDB-cfg IRDB-util IRDB-decode transform rewrite MEDSannotation pqxx pq") myenv=myenv.Clone(CPPPATH=Split(cpppath)) pgm=myenv.Program(pgm, files, LIBPATH=LIBPATH, LIBS=LIBS) install=myenv.Install("$SECURITY_TRANSFORMS_HOME/plugins_install/", pgm) diff --git a/tools/selective_cfi/scfi_instr.cpp b/tools/selective_cfi/scfi_instr.cpp index c5768eed85abd944d699da9ea4d765780985fbde..acaf0c63fe0af61e64d1a19efe4174c614cb8013 100644 --- a/tools/selective_cfi/scfi_instr.cpp +++ b/tools/selective_cfi/scfi_instr.cpp @@ -30,7 +30,8 @@ #include <elf.h> #include "elfio/elfio.hpp" #include "elfio/elfio_dump.hpp" -#include <bea_deprecated.hpp> +//#include <bea_deprecated.hpp> +#include <libIRDB-decode.hpp> @@ -358,16 +359,17 @@ static string change_to_push(Instruction_t *insn) { string newbits=insn->GetDataBits(); - DISASM d; - Disassemble(insn,d); + //DISASM d; + //Disassemble(insn,d); + const auto d=DecodedInstruction_t(insn); int opcode_offset=0; // FIXME: assumes REX is only prefix on jmp insn. // does not assume rex exists. - if(d.Prefix.REX.state == InUsePrefix) - opcode_offset=1; + //if(d.Prefix.REX.state == InUsePrefix) + opcode_offset=d.getPrefixCount(); unsigned char modregrm = (newbits[1+opcode_offset]); modregrm &= 0xc7; @@ -534,12 +536,14 @@ void SCFI_Instrument::AddReturnCFIForExeNonce(Instruction_t* insn, ColoredSlotVa worddec="qword"; // 64-bit reg. - DISASM d; - Disassemble(insn,d); - if(d.Argument1.ArgType!=NO_ARGUMENT) + //DISASM d; + //Disassemble(insn,d); + const auto d=DecodedInstruction_t(insn); + + if(d.hasOperand(0)) // d.Argument1.ArgType!=NO_ARGUMENT) { - unsigned int sp_adjust=d.Instruction.Immediat-firp->GetArchitectureBitWidth()/8; - cout<<"Found relatively rare ret_with_pop insn: "<<d.CompleteInstr<<endl; + unsigned int sp_adjust=d.getImmediate() /*d.Instruction.Immediat*/-firp->GetArchitectureBitWidth()/8; + cout<<"Found relatively rare ret_with_pop insn: "<<d.getDisassembly() /*CompleteInstr*/<<endl; char buf[30]; sprintf(buf, "pop %s [%s+%d]", worddec.c_str(), rspreg.c_str(), sp_adjust); Instruction_t* newafter=insertAssemblyBefore(firp,insn,buf); @@ -627,12 +631,13 @@ void SCFI_Instrument::AddReturnCFI(Instruction_t* insn, ColoredSlotValue_t *v) worddec="qword"; // 64-bit reg. - DISASM d; - Disassemble(insn,d); - if(d.Argument1.ArgType!=NO_ARGUMENT) + //DISASM d; + //Disassemble(insn,d); + const auto d=DecodedInstruction_t(insn); + if(d.hasOperand(1)) // d.Argument1.ArgType!=NO_ARGUMENT) { - unsigned int sp_adjust=d.Instruction.Immediat-firp->GetArchitectureBitWidth()/8; - cout<<"Found relatively rare ret_with_pop insn: "<<d.CompleteInstr<<endl; + unsigned int sp_adjust=d.getImmediate() /* Instruction.Immediat*/-firp->GetArchitectureBitWidth()/8; + cout<<"Found relatively rare ret_with_pop insn: "<<d.getDisassembly()<<endl; char buf[30]; sprintf(buf, "pop %s [%s+%d]", worddec.c_str(), rspreg.c_str(), sp_adjust); Instruction_t* newafter=insertAssemblyBefore(firp,insn,buf); @@ -754,11 +759,13 @@ static void display_histogram(std::ostream& out, std::string attr_label, std::ma bool SCFI_Instrument::is_plt_style_jmp(Instruction_t* insn) { - DISASM d; - Disassemble(insn,d); - if((d.Argument1.ArgType&MEMORY_TYPE)==MEMORY_TYPE) + //DISASM d; + //Disassemble(insn,d); + const auto d=DecodedInstruction_t(insn); + if(d.getOperand(0).isMemory()) // (d.Argument1.ArgType&MEMORY_TYPE)==MEMORY_TYPE) { - if(d.Argument1.Memory.BaseRegister == 0 && d.Argument1.Memory.IndexRegister == 0) + //if(d.Argument1.Memory.BaseRegister == 0 && d.Argument1.Memory.IndexRegister == 0) + if(!d.getOperand(0).hasBaseRegister() && !d.getOperand(0).hasIndexRegister() ) return true; return false; } @@ -807,8 +814,9 @@ bool SCFI_Instrument::instrument_jumps() ++it) { Instruction_t* insn=*it; - DISASM d; - Disassemble(insn,d); + //DISASM d; + //Disassemble(insn,d); + const auto d=DecodedInstruction_t(insn); // we always have to protect the zestcfi dispatcher, that we just added. @@ -823,7 +831,7 @@ bool SCFI_Instrument::instrument_jumps() if(insn->GetBaseID()==BaseObj_t::NOT_IN_DATABASE) continue; - if(string(d.Instruction.Mnemonic)==string("call ") && (protect_safefn && !do_exe_nonce_for_call)) + if(d.isCall() /*string(d.Instruction.Mnemonic)==string("call ")*/ && (protect_safefn && !do_exe_nonce_for_call)) { cerr<<"Fatal Error: Found call instruction!"<<endl; cerr<<"FIX_CALLS_FIX_ALL_CALLS=1 should be set in the environment, or"<<endl; @@ -848,116 +856,124 @@ bool SCFI_Instrument::instrument_jumps() cerr<<"Looking at: "<<insn->getDisassembly()<< " but no associated function" << endl; - switch(d.Instruction.BranchType) + //switch(d.Instruction.BranchType) + //{ + //case JmpType: + if(d.isUnconditionalBranch()) { - case JmpType: + //if((d.Argument1.ArgType&CONSTANT_TYPE)!=CONSTANT_TYPE) + if(!d.getOperand(0).isConstant()) { - if((d.Argument1.ArgType&CONSTANT_TYPE)!=CONSTANT_TYPE) + bool is_fixed_call=is_jmp_a_fixed_call(insn); + bool is_plt_style=is_plt_style_jmp(insn); + bool is_any_call_style = (is_fixed_call || is_plt_style); + if(do_jumps && !is_any_call_style) { - bool is_fixed_call=is_jmp_a_fixed_call(insn); - bool is_plt_style=is_plt_style_jmp(insn); - bool is_any_call_style = (is_fixed_call || is_plt_style); - if(do_jumps && !is_any_call_style) + if (insn->GetIBTargets() && insn->GetIBTargets()->IsComplete()) { - if (insn->GetIBTargets() && insn->GetIBTargets()->IsComplete()) - { - cfi_branch_jmp_complete++; - jmps[insn->GetIBTargets()->size()]++; - } - - cfi_checks++; - cfi_branch_jmp_checks++; - - AddJumpCFI(insn); + cfi_branch_jmp_complete++; + jmps[insn->GetIBTargets()->size()]++; } - else if(do_calls && is_any_call_style) - { - if (insn->GetIBTargets() && insn->GetIBTargets()->IsComplete()) - { - cfi_branch_call_complete++; - calls[insn->GetIBTargets()->size()]++; - } - cfi_checks++; - cfi_branch_call_checks++; - - AddJumpCFI(insn); - } - else - { - cout<<"Eliding protection for "<<insn->getDisassembly()<<std::boolalpha - <<" is_fixed_call="<<is_fixed_call - <<" is_plt_style="<<is_plt_style - <<" is_any_call_style="<<is_any_call_style - <<" do_jumps="<<do_jumps - <<" do_calls="<<do_calls<<endl; - } - } - - break; - } - case CallType: - { + cfi_checks++; + cfi_branch_jmp_checks++; - // should only see calls if we are not CFI'ing safe functions - // be sure to use with: --no-fix-safefn in fixcalls - // (1) --no-fix-safefn in fixcalls leaves call as call (instead of push/jmp) - // (2) and here, we don't plop down a nonce - // see (3) below where we don't instrument returns for safe functions - if (!protect_safefn) + AddJumpCFI(insn); + } + else if(do_calls && is_any_call_style) { - bool isDirectCall = (d.Argument1.ArgType&CONSTANT_TYPE)==CONSTANT_TYPE; - - if (safefn || (isDirectCall && isCallToSafeFunction(insn))) + if (insn->GetIBTargets() && insn->GetIBTargets()->IsComplete()) { - cfi_safefn_call_skipped++; - continue; + cfi_branch_call_complete++; + calls[insn->GetIBTargets()->size()]++; } - } - AddExecutableNonce(insn); // for all calls - if((d.Argument1.ArgType&CONSTANT_TYPE)!=CONSTANT_TYPE) - { - // for indirect calls. - AddCallCFIWithExeNonce(insn); + cfi_checks++; + cfi_branch_call_checks++; + + AddJumpCFI(insn); + } + else + { + cout<<"Eliding protection for "<<insn->getDisassembly()<<std::boolalpha + <<" is_fixed_call="<<is_fixed_call + <<" is_plt_style="<<is_plt_style + <<" is_any_call_style="<<is_any_call_style + <<" do_jumps="<<do_jumps + <<" do_calls="<<do_calls<<endl; } - break; } - case RetType: + + break; + } + // case CallType: + else if(d.isCall()) + { + + + // should only see calls if we are not CFI'ing safe functions + // be sure to use with: --no-fix-safefn in fixcalls + // (1) --no-fix-safefn in fixcalls leaves call as call (instead of push/jmp) + // (2) and here, we don't plop down a nonce + // see (3) below where we don't instrument returns for safe functions + + // bool isDirectCall = (d.Argument1.ArgType&CONSTANT_TYPE)==CONSTANT_TYPE; + bool isDirectCall = d.getOperand(0).isConstant(); + if (!protect_safefn) { - if (insn->GetFunction()) - cerr << "found ret type protect_safefn: " << protect_safefn << " safefn: " << safefn << " function: " << insn->GetFunction()->GetName() << endl; - else - cerr << "found ret type protect_safefn: " << protect_safefn << " safefn: " << safefn << " no functions associated with instruction!! wtf???" << endl; - if (insn->GetIBTargets() && insn->GetIBTargets()->IsComplete()) - { - cfi_branch_ret_complete++; - rets[insn->GetIBTargets()->size()]++; - } - // (3) and here, we don't instrument returns for safe function - if (!protect_safefn && safefn) + if (safefn || (isDirectCall && isCallToSafeFunction(insn))) { - cerr << "Skip ret instructions in function: " << insn->GetFunction()->GetName() << endl; - cfi_safefn_ret_skipped++; + cfi_safefn_call_skipped++; continue; } + } - cfi_checks++; - cfi_branch_ret_checks++; - - if(do_exe_nonce_for_call) - AddReturnCFIForExeNonce(insn); - else - AddReturnCFI(insn); - break; - + AddExecutableNonce(insn); // for all calls + if(!isDirectCall) // (d.Argument1.ArgType&CONSTANT_TYPE)!=CONSTANT_TYPE) + { + // for indirect calls. + AddCallCFIWithExeNonce(insn); } - default: + break; + } + // case RetType: + else if (d.isReturn()) + { + if (insn->GetFunction()) + cerr << "found ret type protect_safefn: " << protect_safefn << " safefn: " << safefn << " function: " << insn->GetFunction()->GetName() << endl; + else + cerr << "found ret type protect_safefn: " << protect_safefn << " safefn: " << safefn << " no functions associated with instruction!! wtf???" << endl; + if (insn->GetIBTargets() && insn->GetIBTargets()->IsComplete()) + { + cfi_branch_ret_complete++; + rets[insn->GetIBTargets()->size()]++; + } + + // (3) and here, we don't instrument returns for safe function + if (!protect_safefn && safefn) { - break; + cerr << "Skip ret instructions in function: " << insn->GetFunction()->GetName() << endl; + cfi_safefn_ret_skipped++; + continue; } + + cfi_checks++; + cfi_branch_ret_checks++; + + if(do_exe_nonce_for_call) + AddReturnCFIForExeNonce(insn); + else + AddReturnCFI(insn); + break; + + } + else // default: + { + // break; + // do nothing } + //} } cout<<"# ATTRIBUTE cfi_jmp_checks="<<std::dec<<cfi_branch_jmp_checks<<endl; diff --git a/tools/selective_cfi/tests/test_dude.sh b/tools/selective_cfi/tests/test_dude.sh index 22380b2197a175d0c37c8af3d5b9614ad36e5272..f073062257a60efa9d2b94092eb20f36b6a9f4be 100755 --- a/tools/selective_cfi/tests/test_dude.sh +++ b/tools/selective_cfi/tests/test_dude.sh @@ -2,12 +2,12 @@ do_cfi() { - $PS $1 $2 --backend zipr --step move_globals=on --step selective_cfi=on --step-option selective_cfi:--multimodule --step-option move_globals:--cfi --step-option fix_calls:--fix-all --step-option zipr:"--add-sections false" + (set -x ; $PS $1 $2 --backend zipr --step move_globals=on --step selective_cfi=on --step-option selective_cfi:--multimodule --step-option move_globals:--cfi --step-option fix_calls:--fix-all --step-option zipr:"--add-sections false" ) } do_coloring_cfi() { - $PS $1 $2 --backend zipr --step move_globals=on --step selective_cfi=on --step-option selective_cfi:--multimodule --step-option move_globals:--cfi --step-option fix_calls:--fix-all --step-option selective_cfi:--color --step-option zipr:"--add-sections false" + (set -x ; $PS $1 $2 --backend zipr --step move_globals=on --step selective_cfi=on --step-option selective_cfi:--multimodule --step-option move_globals:--cfi --step-option fix_calls:--fix-all --step-option selective_cfi:--color --step-option zipr:"--add-sections false" ) }