From 0414ee3ce2f5b2137d7f0d48d4e3fb87692b3da0 Mon Sep 17 00:00:00 2001 From: Crabtux Date: Thu, 14 Mar 2024 14:26:22 +0800 Subject: [PATCH 1/8] Add PcodeFixupPreprocessor --- CMakeLists.txt | 2 ++ src/PcodeFixupPreprocessor.cpp | 11 +++++++++++ src/PcodeFixupPreprocessor.h | 15 +++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 src/PcodeFixupPreprocessor.cpp create mode 100644 src/PcodeFixupPreprocessor.h diff --git a/CMakeLists.txt b/CMakeLists.txt index fa6558fa..3d2ed9ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -57,6 +57,8 @@ set(CORE_SOURCE src/RzCoreMutex.cpp src/PrettyXmlEncode.h src/PrettyXmlEncode.cpp + src/PcodeFixupPreprocessor.h + src/PcodeFixupPreprocessor.cpp src/rz_ghidra.h src/rz_ghidra_internal.h) diff --git a/src/PcodeFixupPreprocessor.cpp b/src/PcodeFixupPreprocessor.cpp new file mode 100644 index 00000000..311abf32 --- /dev/null +++ b/src/PcodeFixupPreprocessor.cpp @@ -0,0 +1,11 @@ +// SPDX-FileCopyrightText: 2024 Crabtux +// SPDX-License-Identifier: LGPL-3.0-or-later + +#include "PcodeFixupPreprocessor.h" + +using namespace ghidra; + +void PcodeFixupPreprocessor::fixupSharedReturnCall(RizinArchitecture &arch, RzCore *core) +{ + return; +} diff --git a/src/PcodeFixupPreprocessor.h b/src/PcodeFixupPreprocessor.h new file mode 100644 index 00000000..701d016f --- /dev/null +++ b/src/PcodeFixupPreprocessor.h @@ -0,0 +1,15 @@ +// SPDX-FileCopyrightText: 2024 Crabtux +// SPDX-License-Identifier: LGPL-3.0-or-later + +#ifndef PCODE_PREPROCESSOR_H +#define PCODE_PREPROCESSOR_H + +#include "RizinArchitecture.h" + +class PcodeFixupPreprocessor +{ + public: + static void fixupSharedReturnCall(RizinArchitecture &arch, RzCore *core); +}; + +#endif \ No newline at end of file From 29179e9808caac0c40656cee1650f01eeca1c4a0 Mon Sep 17 00:00:00 2001 From: Crabtux Date: Thu, 14 Mar 2024 16:22:21 +0800 Subject: [PATCH 2/8] Modify core_ghidra.cpp to use PcodeFixupPreprocessor --- src/core_ghidra.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/core_ghidra.cpp b/src/core_ghidra.cpp index 00d50860..8369d564 100644 --- a/src/core_ghidra.cpp +++ b/src/core_ghidra.cpp @@ -8,6 +8,7 @@ #include "CodeXMLParse.h" #include "ArchMap.h" #include "PrettyXmlEncode.h" +#include "PcodeFixupPreprocessor.h" #include "rz_ghidra.h" #include "rz_ghidra_internal.h" @@ -134,6 +135,10 @@ static void Decompile(RzCore *core, ut64 addr, DecompileMode mode, std::stringst arch.readonlypropagate = cfg_var_ropropagate.GetBool(core->config); arch.setRawPtr(cfg_var_rawptr.GetBool(core->config)); arch.init(store); + + // Must be called after arch.init(), but before decompiling the function + PcodeFixupPreprocessor::fixupSharedReturnCall(arch, core); + Funcdata *func = arch.symboltab->getGlobalScope()->findFunction(Address(arch.getDefaultCodeSpace(), function->addr)); arch.print->setOutputStream(&out_stream); arch.setPrintLanguage("rizin-c-language"); From 5a009fb29ffe03041d1a7b371029bfe892e379bf Mon Sep 17 00:00:00 2001 From: Crabtux Date: Fri, 15 Mar 2024 12:18:57 +0800 Subject: [PATCH 3/8] Implement fixupSharedReturnJumpToRelocs --- src/PcodeFixupPreprocessor.cpp | 28 ++++++++++++++++++++++++++-- src/PcodeFixupPreprocessor.h | 6 ++++-- src/core_ghidra.cpp | 8 ++++---- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/PcodeFixupPreprocessor.cpp b/src/PcodeFixupPreprocessor.cpp index 311abf32..ebc1b208 100644 --- a/src/PcodeFixupPreprocessor.cpp +++ b/src/PcodeFixupPreprocessor.cpp @@ -3,9 +3,33 @@ #include "PcodeFixupPreprocessor.h" +#include +#include +#include + using namespace ghidra; -void PcodeFixupPreprocessor::fixupSharedReturnCall(RizinArchitecture &arch, RzCore *core) +void PcodeFixupPreprocessor::fixupSharedReturnJumpToRelocs(RzAnalysisFunction *function, Funcdata *func, RzCore *core, RizinArchitecture &arch) { - return; + RzList *refs = rz_analysis_function_get_xrefs_from(function); + + RzListIter *it; + RzAnalysisXRef *xref; + + // C++ has more strict type checking than C, which stops us from implicitly casting void * to RzListIter *. + // Expand the rz_list_foreach macro manually. + if (refs) for (it = refs->head; it && (xref = (RzAnalysisXRef *)it->elem, 1); it = it->next) + { + // To ensure the instruction is a `jmp` instruction + if (xref->type != RZ_ANALYSIS_XREF_TYPE_CODE) + continue; + + // If the target location is outside of the current function, and it is a imported function, then do the patch. + // FIXME: Use librz to confirm the target location is an imported function. + RzAnalysisFunction *targetFunction = rz_analysis_get_fcn_in(core->analysis, xref->to, RZ_ANALYSIS_FCN_TYPE_NULL); + if (targetFunction == NULL) + { + func->getOverride().insertFlowOverride(Address(arch.getDefaultCodeSpace(), xref->from), Override::CALL_RETURN); + } + } } diff --git a/src/PcodeFixupPreprocessor.h b/src/PcodeFixupPreprocessor.h index 701d016f..be4f1b34 100644 --- a/src/PcodeFixupPreprocessor.h +++ b/src/PcodeFixupPreprocessor.h @@ -6,10 +6,12 @@ #include "RizinArchitecture.h" +#include + class PcodeFixupPreprocessor { - public: - static void fixupSharedReturnCall(RizinArchitecture &arch, RzCore *core); + public: + static void fixupSharedReturnJumpToRelocs(RzAnalysisFunction *function, ghidra::Funcdata *func, RzCore *core, RizinArchitecture &arch); }; #endif \ No newline at end of file diff --git a/src/core_ghidra.cpp b/src/core_ghidra.cpp index 8369d564..cdaf958c 100644 --- a/src/core_ghidra.cpp +++ b/src/core_ghidra.cpp @@ -135,16 +135,16 @@ static void Decompile(RzCore *core, ut64 addr, DecompileMode mode, std::stringst arch.readonlypropagate = cfg_var_ropropagate.GetBool(core->config); arch.setRawPtr(cfg_var_rawptr.GetBool(core->config)); arch.init(store); - - // Must be called after arch.init(), but before decompiling the function - PcodeFixupPreprocessor::fixupSharedReturnCall(arch, core); - Funcdata *func = arch.symboltab->getGlobalScope()->findFunction(Address(arch.getDefaultCodeSpace(), function->addr)); arch.print->setOutputStream(&out_stream); arch.setPrintLanguage("rizin-c-language"); ApplyPrintCConfig(core->config, dynamic_cast(arch.print)); if(!func) throw LowlevelError("No function in Scope"); + + // Must be called after arch.init(), but before decompiling the function + PcodeFixupPreprocessor::fixupSharedReturnJumpToRelocs(function, func, core, arch); + arch.getCore()->sleepBegin(); auto action = arch.allacts.getCurrent(); int res; From 0177794e2a04f5dc7428bdc40daa8a35cb84a6e8 Mon Sep 17 00:00:00 2001 From: Crabtux Date: Fri, 15 Mar 2024 14:45:57 +0800 Subject: [PATCH 4/8] Add test for fixupSharedReturnJumpToRelocs --- test/bins/Makefile | 5 ++++- test/bins/sharedReturn.c | 13 +++++++++++++ test/bins/sharedReturn.o | Bin 0 -> 1512 bytes test/db/extras/ghidra | 26 ++++++++++++++++++++++++-- 4 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 test/bins/sharedReturn.c create mode 100644 test/bins/sharedReturn.o diff --git a/test/bins/Makefile b/test/bins/Makefile index 50eef138..ba130e5a 100644 --- a/test/bins/Makefile +++ b/test/bins/Makefile @@ -3,7 +3,7 @@ all: .PHONY: clean clean: - rm -fv dectest32 dectest64 rec + rm -fv dectest32 dectest64 rec sharedReturn.o dectest32: dectest.c types.h gcc -m32 -fno-pic -no-pie -fno-omit-frame-pointer -o dectest32 -O0 dectest.c @@ -22,3 +22,6 @@ rec: rec.c types_rec.h strings: strings.c gcc -m64 -fno-pic -no-pie -fno-omit-frame-pointer -o strings -O0 strings.c + +sharedReturn.o: sharedReturn.c + gcc -m64 -fno-pic -no-pie -fno-omit-frame-pointer -fno-inline -o sharedReturn.o -O2 -c sharedReturn.c diff --git a/test/bins/sharedReturn.c b/test/bins/sharedReturn.c new file mode 100644 index 00000000..b7ec2a11 --- /dev/null +++ b/test/bins/sharedReturn.c @@ -0,0 +1,13 @@ +#include + +int getNum(void); +int calc(int a, int b); + +int main(void){ + int c = getNum(); + + if (c > 2) + return calc(c, 2); + else + return 0; +} \ No newline at end of file diff --git a/test/bins/sharedReturn.o b/test/bins/sharedReturn.o new file mode 100644 index 0000000000000000000000000000000000000000..9abf334e9bb2970c60ac79fb958a8151e3390e8d GIT binary patch literal 1512 zcmbtSO-md>5Um+EYT^g0MhFR27W zOcy%ZzXU&Ti^!p@%lk|j)9D_s9o(esGW8j zlr@K15sG4MU8e;qZYGIIQI161iF#Z-jNkzvnY;!Op0DJj=@pZ%{&P#1R$;H0*6LKTC?#(3TU9R!_ zlKz>776!DZUw_4dh`l(!{s4)h`}@X?%01EjJ;WvTf)|}lXZ7fVcF}h*j-F=3uctlz d*`bmypFdAY&KSN!rZ6h1@3HXsPT3Fq`acrdZ5;pr literal 0 HcmV?d00001 diff --git a/test/db/extras/ghidra b/test/db/extras/ghidra index b5544a46..5b74d1dc 100644 --- a/test/db/extras/ghidra +++ b/test/db/extras/ghidra @@ -3119,8 +3119,6 @@ EXPECT=< Date: Fri, 15 Mar 2024 18:00:39 +0800 Subject: [PATCH 5/8] Restrict PcodeFixupPreprocessor to x86 only --- src/core_ghidra.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core_ghidra.cpp b/src/core_ghidra.cpp index cdaf958c..b7929904 100644 --- a/src/core_ghidra.cpp +++ b/src/core_ghidra.cpp @@ -142,8 +142,10 @@ static void Decompile(RzCore *core, ut64 addr, DecompileMode mode, std::stringst if(!func) throw LowlevelError("No function in Scope"); - // Must be called after arch.init(), but before decompiling the function - PcodeFixupPreprocessor::fixupSharedReturnJumpToRelocs(function, func, core, arch); + // Other archs are not tested + if (strcmp(core->analysis->arch_target->arch, "x86") == 0) + // Must be called after arch.init(), but before decompiling the function + PcodeFixupPreprocessor::fixupSharedReturnJumpToRelocs(function, func, core, arch); arch.getCore()->sleepBegin(); auto action = arch.allacts.getCurrent(); From 430e13874920534185930d109a7d24477e2ac394 Mon Sep 17 00:00:00 2001 From: Crabtux Date: Fri, 15 Mar 2024 18:37:02 +0800 Subject: [PATCH 6/8] Use librz APIs to detect relocs --- src/PcodeFixupPreprocessor.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/PcodeFixupPreprocessor.cpp b/src/PcodeFixupPreprocessor.cpp index ebc1b208..c798c39c 100644 --- a/src/PcodeFixupPreprocessor.cpp +++ b/src/PcodeFixupPreprocessor.cpp @@ -24,10 +24,9 @@ void PcodeFixupPreprocessor::fixupSharedReturnJumpToRelocs(RzAnalysisFunction *f if (xref->type != RZ_ANALYSIS_XREF_TYPE_CODE) continue; - // If the target location is outside of the current function, and it is a imported function, then do the patch. - // FIXME: Use librz to confirm the target location is an imported function. - RzAnalysisFunction *targetFunction = rz_analysis_get_fcn_in(core->analysis, xref->to, RZ_ANALYSIS_FCN_TYPE_NULL); - if (targetFunction == NULL) + // If the target location is a imported function, then do the patch. + RzBinReloc *reloc = rz_core_get_reloc_to(core, xref->to); + if (reloc != nullptr && reloc->import != nullptr) { func->getOverride().insertFlowOverride(Address(arch.getDefaultCodeSpace(), xref->from), Override::CALL_RETURN); } From 7dc95ed2f8b5050856052c2d9fbab91d365d865e Mon Sep 17 00:00:00 2001 From: Crabtux Date: Fri, 15 Mar 2024 18:56:43 +0800 Subject: [PATCH 7/8] Fix show noreturn test --- test/db/extras/ghidra | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/db/extras/ghidra b/test/db/extras/ghidra index 5b74d1dc..ae6ba9ae 100644 --- a/test/db/extras/ghidra +++ b/test/db/extras/ghidra @@ -3119,6 +3119,8 @@ EXPECT=< Date: Sat, 16 Mar 2024 11:56:24 +0800 Subject: [PATCH 8/8] Reuse rz_list_foreach_cpp --- src/PcodeFixupPreprocessor.cpp | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/PcodeFixupPreprocessor.cpp b/src/PcodeFixupPreprocessor.cpp index c798c39c..e28ff671 100644 --- a/src/PcodeFixupPreprocessor.cpp +++ b/src/PcodeFixupPreprocessor.cpp @@ -1,6 +1,8 @@ // SPDX-FileCopyrightText: 2024 Crabtux // SPDX-License-Identifier: LGPL-3.0-or-later +#include "RizinLoadImage.h" +#include "RizinUtils.h" #include "PcodeFixupPreprocessor.h" #include @@ -11,24 +13,15 @@ using namespace ghidra; void PcodeFixupPreprocessor::fixupSharedReturnJumpToRelocs(RzAnalysisFunction *function, Funcdata *func, RzCore *core, RizinArchitecture &arch) { - RzList *refs = rz_analysis_function_get_xrefs_from(function); - - RzListIter *it; - RzAnalysisXRef *xref; - - // C++ has more strict type checking than C, which stops us from implicitly casting void * to RzListIter *. - // Expand the rz_list_foreach macro manually. - if (refs) for (it = refs->head; it && (xref = (RzAnalysisXRef *)it->elem, 1); it = it->next) - { + RzList *xrefs = rz_analysis_function_get_xrefs_from(function); + rz_list_foreach_cpp(xrefs, [&](RzAnalysisXRef *xref){ // To ensure the instruction is a `jmp` instruction - if (xref->type != RZ_ANALYSIS_XREF_TYPE_CODE) - continue; - - // If the target location is a imported function, then do the patch. - RzBinReloc *reloc = rz_core_get_reloc_to(core, xref->to); - if (reloc != nullptr && reloc->import != nullptr) + if (xref->type == RZ_ANALYSIS_XREF_TYPE_CODE) { - func->getOverride().insertFlowOverride(Address(arch.getDefaultCodeSpace(), xref->from), Override::CALL_RETURN); + // If the target location is a imported function, then do the patch. + RzBinReloc *reloc = rz_core_get_reloc_to(core, xref->to); + if (reloc != nullptr && reloc->import != nullptr) + func->getOverride().insertFlowOverride(Address(arch.getDefaultCodeSpace(), xref->from), Override::CALL_RETURN); } - } + }); }