diff --git a/libIRDB/test/SConscript b/libIRDB/test/SConscript index 8cf229e14659817a22d9d7a72c717b29a6e9d1be..24c10568e47edf1e6dda13f3c6e4ac6bfac57a29 100644 --- a/libIRDB/test/SConscript +++ b/libIRDB/test/SConscript @@ -30,14 +30,17 @@ if 'build_tools' not in myenv or myenv['build_tools'] is None or int(myenv['buil pgm=myenv.Program("fill_in_indtargs.exe", ehframe+split_eh_frame+Split("fill_in_indtargs.cpp check_thunks.cpp"), LIBPATH=LIBPATH, LIBS=LIBS) install=myenv.Install("$SECURITY_TRANSFORMS_HOME/bin/", pgm) Default(install) + installed=installed+install pgm=myenv.Program("fill_in_cfg.exe", split_eh_frame+Split("fill_in_cfg.cpp"), LIBPATH=LIBPATH, LIBS=LIBS) install=myenv.Install("$SECURITY_TRANSFORMS_HOME/bin/", pgm) Default(install) + installed=installed+install pgm=myenv.Program("fix_calls.exe", ehframe+Split("fix_calls.cpp"), LIBPATH=LIBPATH, LIBS=LIBS) install=myenv.Install("$SECURITY_TRANSFORMS_HOME/bin/", pgm) Default(install) + installed=installed+install # most programs go to $sectrans/bin pgms='''clone @@ -50,6 +53,7 @@ if 'build_tools' not in myenv or myenv['build_tools'] is None or int(myenv['buil pgm=myenv.Program(target=i+".exe", source=Split(i+".cpp"), LIBPATH=LIBPATH, LIBS=LIBS) install=myenv.Install("$SECURITY_TRANSFORMS_HOME/bin/", pgm) Default(install) + installed=installed+install # ilr goes to $sectrans/plugins_install diff --git a/libIRDB/test/fix_calls.cpp b/libIRDB/test/fix_calls.cpp index 57d0414ff5f57015bed39e4a0c8d8f4878dc51f9..2510b4df5f0dceea272d30b3cfeeff0dfbbd59b9 100644 --- a/libIRDB/test/fix_calls.cpp +++ b/libIRDB/test/fix_calls.cpp @@ -290,49 +290,6 @@ bool call_needs_fix(Instruction_t* insn) return ret; } - /* now, search the function for stack references */ - -// is this even right with the assembler switch ? -#if 0 - /* determine what the stack ref. would look like */ - if(func->GetUseFramePointer()) - { - pattern="[ebp+0x04]"; - } - else - { - pattern="[esp+"+to_string(func->GetStackFrameSize())+"]"; - } - - - /* check each instruction */ - for( - std::set<Instruction_t*>::const_iterator it=func->GetInstructions().begin(); - it!=func->GetInstructions().end(); - ++it - ) - { - Instruction_t* itrinsn=*it; - /* if the disassembly contains the string mentioned */ - //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")) - { - virtual_offset_t addr = 0; - if (insn->GetAddress()) - addr = insn->GetAddress()->GetVirtualOffset(); - cout<<"Needs fix: Found pattern"<< " address=" - <<hex<<addr<<": "<<insn->getDisassembly()<<endl; - } - /* then we need to fix this callsite */ - return true; - } - } -#endif /* otherwise, we think it's safe */ return false; @@ -554,15 +511,9 @@ void fix_call(Instruction_t* insn, FileIR_t *firp, bool can_unpin) { /* record the possibly new indirect branch target if this call gets fixed */ Instruction_t* newindirtarg=insn->GetFallthrough(); - //bool has_rex=false; - - /* disassemble */ - //DISASM disasm; - DecodedInstruction_t disasm(insn); /* Disassemble the instruction */ - //int instr_len = disasm.length(); // Disassemble(insn,disasm); - + DecodedInstruction_t disasm(insn); /* if this instruction is an inserted call instruction than we don't need to * convert it for correctness' sake. @@ -575,10 +526,7 @@ void fix_call(Instruction_t* insn, FileIR_t *firp, bool can_unpin) */ if((insn->GetDataBits()[0]&0x40)==0x40) { -#if 0 - // has rex! - has_rex=true; -#endif + // ignore rex prefixes } else if( (insn->GetDataBits()[0]!=(char)0xff) && (insn->GetDataBits()[0]!=(char)0xe8) && @@ -753,6 +701,24 @@ template <class T> struct insn_less : binary_function <T,T,bool> { }; +// +// Mark ret_point as an unpinned IBT. +// +void mark_as_unpinned_ibt(FileIR_t* firp, Instruction_t* ret_point) +{ + if( ret_point == NULL ) return; + if( ret_point->GetIndirectBranchTargetAddress() != NULL ) return; + + auto newaddr = new AddressID_t; + assert(newaddr); + newaddr->SetFileID(ret_point->GetAddress()->GetFileID()); + newaddr->SetVirtualOffset(0); // unpinne + + firp->GetAddresses().insert(newaddr); + ret_point->SetIndirectBranchTargetAddress(newaddr); + +} + // // fix_all_calls - convert calls to push/jump pairs in the IR. if fix_all is true, all calls are converted, // else we attempt to detect the calls it is safe to convert. @@ -818,7 +784,8 @@ void fix_all_calls(FileIR_t* firp, bool print_stats, bool fix_all) else { if(getenv("VERBOSE_FIX_CALLS")) - cout<<"no fix needed for "<<insn->GetAddress()->GetVirtualOffset()<<":"<<insn->getDisassembly()<<endl; + cout<<"No fix needed, marking ret site IBT, for "<<insn->GetAddress()->GetVirtualOffset()<<":"<<insn->getDisassembly()<<endl; + mark_as_unpinned_ibt(firp, insn->GetFallthrough()); not_fixed_calls++; } } @@ -860,18 +827,6 @@ void fix_all_calls(FileIR_t* firp, bool print_stats, bool fix_all) } } -#if 0 -bool arg_has_relative(const ARGTYPE &arg) -{ - /* if it's relative memory, watch out! */ - if(arg.ArgType&MEMORY_TYPE) - if(arg.ArgType&RELATIVE_) - return true; - - return false; -} -#endif - // // fix_other_pcrel - add relocations to other instructions that have pcrel bits diff --git a/tools/selective_cfi/scfi_instr.cpp b/tools/selective_cfi/scfi_instr.cpp index 950c43fd892c6fdddc4671010a3b401f9c3ac750..70825a50476c6e45d92c2702e15f3aec2b0be19b 100644 --- a/tools/selective_cfi/scfi_instr.cpp +++ b/tools/selective_cfi/scfi_instr.cpp @@ -350,21 +350,6 @@ void SCFI_Instrument::AddJumpCFI(Instruction_t* insn) cout<<"Converting ' "<<insn->getDisassembly()<<"' to '"; Instruction_t* after=insertDataBitsBefore(firp,insn,pushbits); move_relocs(after,insn); -#ifdef CGC - // insert the pop/checking code. - cout<<insn->getDisassembly()<<"+jmp slowpath'"<<endl; - - string jmpBits=getJumpDataBits(); - after->SetDataBits(jmpBits); - after->SetComment(insn->getDisassembly()+" ; scfi"); - assert(do_common_slow_path); /* fixme: this defaults to the slow_cfi path. need to color accordingly */ - createNewRelocation(firp,after,"slow_cfi_path",0); - after->SetFallthrough(NULL); - after->SetTarget(after); - after->SetIBTargets(NULL); // lose information about ib targets. - insn->SetIBTargets(NULL); // lose information about ib targets. - return; -#else after->SetDataBits(getRetDataBits()); cout <<insn->getDisassembly()<<" + ret' "<<endl ; @@ -377,7 +362,6 @@ void SCFI_Instrument::AddJumpCFI(Instruction_t* insn) AddReturnCFI(after,v); // cout<<"Warning, JUMPS not CFI's yet"<<endl; return; -#endif } //TODO: Does not work with coloring @@ -421,6 +405,10 @@ void SCFI_Instrument::AddCallCFIWithExeNonce(Instruction_t* insn) call=insertDataBitsBefore(firp, insn, pushbits); insertAssemblyAfter(firp,insn,string("pop ")+reg); + // keep any relocs on the push instruction, as those may need updating. + insn->GetRelocations()=call->GetRelocations(); + call->GetRelocations().clear(); + stub=addNewAssembly(firp,stub,string("cmp ")+decoration+ " ["+reg+"-"+to_string(nonce_offset)+"], "+to_string(nonce)); jne=insertAssemblyAfter(firp,stub,"jne 0"); @@ -493,49 +481,6 @@ void SCFI_Instrument::AddReturnCFIForExeNonce(Instruction_t* insn, ColoredSlotVa insn=newafter; } -#ifdef CGC -#if 0 - ret -> jmp shared - - shared: - and [rsp], 0x7ffffffff - mov reg <- [ rsp ] - cmp [reg], exe_nonce_value - jne slow - ret - -#endif - - string jmpBits=getJumpDataBits(); - -#ifndef CGC - assert(0); // not ported to non-cgc mode -#endif - if(!ret_shared) - { - string clamp_str="and "+worddec+"["+rspreg+"], 0x7fffffff"; - Instruction_t* tmp=NULL; - ret_shared= -#ifdef CGC - tmp=addNewAssembly(firp,tmp,clamp_str); // FIXME??? - tmp=addNewAssembly(firp, tmp, "mov "+reg+", ["+rspreg+"]"); -#else - tmp=addNewAssembly(firp, "mov "+reg+", ["+rspreg+"]"); -#endif -// fixme: get value from ExecutableNonceString -- somewhat challening - tmp=addNewAssembly(firp, tmp, "cmp byte ["+reg+"], 0x90"); - tmp=addNewAssembly(firp, tmp, "jne 0"); - createNewRelocation(firp,tmp,"slow_cfi_path",0); - tmp->SetTarget(tmp); - tmp=addNewAssembly(firp, tmp, "ret"); - - } - - insn->SetDataBits(jmpBits); - insn->SetTarget(ret_shared); - - return; -#else //TODO: Fix possible TOCTOU race condition (see Abadi CFI paper) // Ret address can be changed between nonce check and ret insn execution (in theory) string decoration=""; @@ -578,7 +523,6 @@ void SCFI_Instrument::AddReturnCFIForExeNonce(Instruction_t* insn, ColoredSlotVa // leave the ret instruction (risk of successful race condition exploit << performance benefit) return; -#endif } @@ -652,20 +596,6 @@ void SCFI_Instrument::AddReturnCFI(Instruction_t* insn, ColoredSlotValue_t *v) -#ifdef CGC - // insert the pop/checking code. - Instruction_t* after=insn; - - string jmpBits=getJumpDataBits(); - - after->SetDataBits(jmpBits); - after->SetComment(insn->getDisassembly()+" ; scfi"); - createNewRelocation(firp,after,slow_cfi_path_reloc_string,0); - after->SetFallthrough(NULL); - after->SetTarget(after); - return; - -#else string decoration=""; int nonce_size=GetNonceSize(insn); @@ -712,7 +642,6 @@ void SCFI_Instrument::AddReturnCFI(Instruction_t* insn, ColoredSlotValue_t *v) cout<<"Setting slow path for: "<<slow_cfi_path_reloc_string<<endl; return; -#endif } static void display_histogram(std::ostream& out, std::string attr_label, std::map<int,int> & p_map) diff --git a/tools/selective_cfi/tests/test_fib.sh b/tools/selective_cfi/tests/test_fib.sh index d98a3663b00f704b3d6ea23b304ba725bbad97e5..24c0eb53da7a17dbbc1aa220bd4c3f1cc8747b92 100755 --- a/tools/selective_cfi/tests/test_fib.sh +++ b/tools/selective_cfi/tests/test_fib.sh @@ -2,18 +2,18 @@ do_cfi() { - $PS $1 $2 --backend zipr --step move_globals=on --step selective_cfi=on --step-option selective_cfi:--multimodule --step-option move_globals:--elftables-only --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:--elftables-only --step-option fix_calls:--fix-all --step-option zipr:"--add-sections false") } # Note: exe nonce cfi doesn't always run against non-exe nonce cfi modules do_cfi_exe_nonces() { - $PS $1 $2 --backend zipr --step move_globals=on --step selective_cfi=on --step-option selective_cfi:--multimodule --step-option move_globals:--elftables-only --step-option fix_calls:--no-fix-safefn --step-option selective_cfi:--exe-nonce-for-call --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:--elftables-only --step-option fix_calls:--no-fix-safefn --step-option selective_cfi:--exe-nonce-for-call --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:--elftables-only --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:--elftables-only --step-option fix_calls:--fix-all --step-option selective_cfi:--color --step-option zipr:"--add-sections false") }