From 097342c8290c9ef4979f26bedd4585b34c7382fa Mon Sep 17 00:00:00 2001 From: DexFinder <63297382+shuimu5418@users.noreply.github.com> Date: Wed, 4 Jun 2025 18:10:13 +0800 Subject: [PATCH 1/2] UPDATE EVERYTHING Support ida 9.1; Fix Memleak; Fix Unhandle c++ exception; And a lots of clean ups; Rename choice. --- .gitignore | 2 ++ src/actions.cpp | 36 +++++++++++++++++++++------------ src/actions.hpp | 20 ++++++++++-------- src/blacklist.cpp | 4 ++-- src/callbacks.cpp | 48 ++++++++++++++++++++++---------------------- src/globals.hpp | 4 ++++ src/main.cpp | 27 +++++++++++++++++++------ src/snapshot.cpp | 39 ++++++++++++++++++++++++++++++++--- src/symVarTable.cpp | 5 ++++- src/triton_logic.cpp | 31 ++++++++++++++-------------- 10 files changed, 143 insertions(+), 73 deletions(-) diff --git a/.gitignore b/.gitignore index 55ecf9e2..5fe664d9 100644 --- a/.gitignore +++ b/.gitignore @@ -66,3 +66,5 @@ vcomp110.dll # This file should be generated just before compile the program. # i#20 For some reason the script can not generate this file at Appveyor site. Let's ignore it for a while. # VERSION_NUMBER + +src_for_ida9 \ No newline at end of file diff --git a/src/actions.cpp b/src/actions.cpp index dfdfbee8..4b6bb472 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -174,7 +174,11 @@ struct ah_taint_symbolize_memory_t : public action_handler_t current_ea = ctx->cur_value; } #endif +#if IDA_SDK_VERSION >= 900 + else if (ctx->widget_type == BWN_HEXVIEW) { +#else else if (ctx->widget_type == BWN_DUMP) { +#endif if (ctx->cur_flags & ACF_HAS_SELECTION){ // Only if there has been a valid selection //We get the selection bounds from the action activation context auto selection_starts = ctx->cur_sel.from.at->toea(); @@ -183,6 +187,7 @@ struct ah_taint_symbolize_memory_t : public action_handler_t current_ea = selection_starts; } } + #if IDA_SDK_VERSION >= 740 else if (ctx->widget_type == BWN_CPUREGS) { @@ -253,9 +258,14 @@ struct ah_taint_symbolize_memory_t : public action_handler_t } action_to_take = is_debugger_on() ? AST_ENABLE : AST_DISABLE; } +#if IDA_SDK_VERSION >= 900 + else if (action_update_ctx_t->widget_type == BWN_HEXVIEW) { +#else else if (action_update_ctx_t->widget_type == BWN_DUMP) { - action_to_take = is_debugger_on() ? AST_ENABLE : AST_DISABLE; +#endif + action_to_take = is_debugger_on() ? AST_ENABLE : AST_DISABLE; } + #if IDA_SDK_VERSION >= 730 else if (action_update_ctx_t->widget_type == BWN_STKVIEW) { if (is_mapped(action_update_ctx_t->cur_value)) { @@ -838,11 +848,11 @@ struct ah_enable_disable_tracing_t : public action_handler_t if (is_debugger_on()) { //We are using this event to change the text of the action if (ponce_runtime_status.runtimeTrigger.getState()) { - update_action_label(ctx->action, "Disable ponce tracing"); + update_action_label(ctx->action, "[Ponce] Disable ponce tracing"); update_action_icon(ctx->action, 62); } else { - update_action_label(ctx->action, "Enable ponce tracing"); + update_action_label(ctx->action, "[Ponce] Enable ponce tracing"); update_action_icon(ctx->action, 61); } return AST_ENABLE; @@ -855,7 +865,7 @@ static ah_enable_disable_tracing_t ah_enable_disable_tracing; //We need to define this struct before the action handler because we are using it inside the handler action_desc_t action_IDA_enable_disable_tracing = ACTION_DESC_LITERAL( "Ponce:enable_disable_tracing", - "Enable ponce tracing", //The action text. + "[Ponce] Enable ponce tracing", //The action text. &ah_enable_disable_tracing, //The action handler. "Ctrl+Shift+E", //Optional: the action shortcut "Enable or Disable the ponce tracing", //Optional: the action tooltip (available in menus/toolbar) @@ -1000,7 +1010,7 @@ static ah_run_until_symbolic_t ah_run_until_symbolic; //We need to define this struct before the action handler because we are using it inside the handler action_desc_t action_IDA_run_until_symbolic = ACTION_DESC_LITERAL( "Ponce:run_until_symbolic_branch", - "Run until symbolic condition", //The action text. + "[Ponce] Run until symbolic condition", //The action text. &ah_run_until_symbolic, //The action handler. "Ctrl+Shift+F9", //Optional: the action shortcut "Continue running and stop on next symbolic condition", //Optional: the action tooltip (available in menus/toolbar) @@ -1016,18 +1026,18 @@ struct IDA_actions action_list[] = { &action_IDA_run_until_symbolic, { BWN_DISASM, __END__ }, "" }, - { &action_IDA_taint_symbolize_register, {0}, "Symbolic or taint/"}, - { &action_IDA_taint_symbolize_memory, {0}, "Symbolic or taint/" }, + { &action_IDA_taint_symbolize_register, {0}, "[Ponce] Symbolic or taint/"}, + { &action_IDA_taint_symbolize_memory, {0}, "[Ponce] Symbolic or taint/" }, - { &action_IDA_negate_and_inject, { BWN_DISASM, __END__ }, "SMT Solver/" }, - { &action_IDA_negate_inject_and_restore, { BWN_DISASM, __END__ }, "SMT Solver/" }, + { &action_IDA_negate_and_inject, { BWN_DISASM, __END__ }, "[Ponce] SMT Solver/" }, + { &action_IDA_negate_inject_and_restore, { BWN_DISASM, __END__ }, "[Ponce] SMT Solver/" }, // Solve formula is handled separatly to be more user friendly // But still we want to register it in advance so it is always disable, so we define no views - { &action_IDA_solve_formula_sub, { __END__ }, "SMT Solver/" }, + { &action_IDA_solve_formula_sub, { __END__ }, "[Ponce] SMT Solver/" }, - { &action_IDA_createSnapshot, { BWN_DISASM, __END__ }, "Snapshot/"}, - { &action_IDA_restoreSnapshot, { BWN_DISASM, __END__ }, "Snapshot/" }, - { &action_IDA_deleteSnapshot, { BWN_DISASM, __END__ }, "Snapshot/" }, + { &action_IDA_createSnapshot, { BWN_DISASM, __END__ }, "[Ponce] Snapshot/"}, + { &action_IDA_restoreSnapshot, { BWN_DISASM, __END__ }, "[Ponce] Snapshot/" }, + { &action_IDA_deleteSnapshot, { BWN_DISASM, __END__ }, "[Ponce] Snapshot/" }, { &action_chooser_comment, { BWN_CHOOSER, __END__ }, "" }, { &action_chooser_add_constrain, { BWN_CHOOSER, __END__ }, "" }, diff --git a/src/actions.hpp b/src/actions.hpp index 2780da55..8bee7b95 100644 --- a/src/actions.hpp +++ b/src/actions.hpp @@ -13,18 +13,22 @@ #define __END__ -1 /* Depending on the IDA version the SDK allows or not using some of the fetures we have*/ -#if IDA_SDK_VERSION < 730 -const int ponce_banner_views[] = { BWN_DISASM, BWN_DUMP, BWN_CHOOSER, __END__ }; -const int ponce_taint_symbolize_mem_views[] = { BWN_DISASM, BWN_DUMP, __END__ }; -const int ponce_taint_symbolize_reg_views[] = { BWN_DISASM, BWN_DUMP, __END__ }; -#elif IDA_SDK_VERSION == 730 -const int ponce_banner_views[] = { BWN_DISASM, BWN_DUMP, BWN_STKVIEW, BWN_CHOOSER, __END__ }; -const int ponce_taint_symbolize_mem_views[] = { BWN_DISASM, BWN_DUMP, BWN_STKVIEW, __END__ }; -const int ponce_taint_symbolize_reg_views[] = { BWN_DISASM, BWN_DUMP, BWN_STKVIEW, __END__ }; +#if IDA_SDK_VERSION >= 900 +const int ponce_banner_views[] = { BWN_DISASM, BWN_CPUREGS, BWN_HEXVIEW, BWN_STKVIEW, BWN_CHOOSER, __END__ }; +const int ponce_taint_symbolize_mem_views[] = { BWN_DISASM, BWN_CPUREGS, BWN_HEXVIEW, BWN_STKVIEW, __END__ }; +const int ponce_taint_symbolize_reg_views[] = { BWN_DISASM, BWN_CPUREGS, BWN_HEXVIEW, BWN_STKVIEW, __END__ }; #elif IDA_SDK_VERSION >= 740 const int ponce_banner_views[] = { BWN_DISASM, BWN_CPUREGS, BWN_DUMP, BWN_STKVIEW, BWN_CHOOSER, __END__ }; const int ponce_taint_symbolize_mem_views[] = { BWN_DISASM, BWN_CPUREGS, BWN_DUMP, BWN_STKVIEW, __END__ }; const int ponce_taint_symbolize_reg_views[] = { BWN_DISASM, BWN_CPUREGS, BWN_DUMP, BWN_STKVIEW, __END__ }; +#elif IDA_SDK_VERSION == 730 +const int ponce_banner_views[] = { BWN_DISASM, BWN_DUMP, BWN_STKVIEW, BWN_CHOOSER, __END__ }; +const int ponce_taint_symbolize_mem_views[] = { BWN_DISASM, BWN_DUMP, BWN_STKVIEW, __END__ }; +const int ponce_taint_symbolize_reg_views[] = { BWN_DISASM, BWN_DUMP, BWN_STKVIEW, __END__ }; +#elif IDA_SDK_VERSION < 730 +const int ponce_banner_views[] = { BWN_DISASM, BWN_DUMP, BWN_CHOOSER, __END__ }; +const int ponce_taint_symbolize_mem_views[] = { BWN_DISASM, BWN_DUMP, __END__ }; +const int ponce_taint_symbolize_reg_views[] = { BWN_DISASM, BWN_DUMP, __END__ }; #endif struct IDA_actions { diff --git a/src/blacklist.cpp b/src/blacklist.cpp index 0abd40f4..d8082a1e 100644 --- a/src/blacklist.cpp +++ b/src/blacklist.cpp @@ -119,12 +119,12 @@ void concretizeAndUntaintVolatileRegisters() #if defined(__i386) || defined(_M_IX86) char const* volatile_regs[] = { "eax", "ecx", "edx" }; #elif defined(__x86_64__) || defined(_M_X64) - char const* volatile_regs[] = { "rax", "rcx", "rdx", "r8", "r8", "r10", "r11", "xmm6", "xmm7", "xmm8", "xmm9", "xmm10", "xmm11", "xmm12", "xmm13", "xmm14", "xmm15" }; + char const* volatile_regs[] = { "rax", "rcx", "rdx", "r8", "r9", "r10", "r11", "xmm0", "xmm1", "xmm2", "xmm3", "xmm4", "xmm5", "ymm0", "ymm1", "ymm2", "ymm3", "ymm4", "ymm5" }; #endif for (const auto& [reg_id, reg] : tritonCtx.getAllRegisters()) { - for (auto i = 0; i < sizeof(volatile_regs) / sizeof(char*); i++) { + for (size_t i = 0; i < sizeof(volatile_regs) / sizeof(char*); i++) { if (strcmp(reg.getName().c_str(), volatile_regs[i]) == 0) { tritonCtx.concretizeRegister(reg); tritonCtx.untaintRegister(reg); diff --git a/src/callbacks.cpp b/src/callbacks.cpp index 0b7d1f29..357f52ca 100644 --- a/src/callbacks.cpp +++ b/src/callbacks.cpp @@ -214,14 +214,14 @@ bool attach_action_solve(triton::uint64 dstAddr, unsigned int path_constraint_in char label[256]; char tooltip[256]; char name[256]; - char popup_name[] = "SMT Solver/Solve formula (Multiple hits)/"; + char popup_name[] = "[Ponce] SMT Solver/Solve formula (Multiple hits)/"; if (mode == 0) { action = action_IDA_solve_formula_sub; qsnprintf(label, sizeof(label), "Solve formula to take " MEM_FORMAT, dstAddr); //We need the path constraint index during the action activate qsnprintf(tooltip, 255, "%s. Index: %u", action_IDA_solve_formula_sub.tooltip, path_constraint_index); - qsnprintf(popup_name, sizeof(popup_name), "SMT Solver/Solve formula"); + qsnprintf(popup_name, sizeof(popup_name), "[Ponce] SMT Solver/Solve formula"); action.name = "Ponce:solve_formula_one_branch"; action.label = label; @@ -271,12 +271,12 @@ ssize_t idaapi ui_callback(void* ud, int notification_code, va_list va) // Set the name for the action depending if using tainting or symbolic engine if (cmdOptions.use_tainting_engine) { - action_list[3].menu_path = TAINT; - action_list[4].menu_path = TAINT; + action_list[3].menu_path = "[Ponce] " TAINT; + action_list[4].menu_path = "[Ponce] " TAINT; } else { - action_list[3].menu_path = SYMBOLIC; - action_list[4].menu_path = SYMBOLIC; + action_list[3].menu_path = "[Ponce] " SYMBOLIC; + action_list[4].menu_path = "[Ponce] " SYMBOLIC; } //Adding a separator @@ -287,7 +287,7 @@ ssize_t idaapi ui_callback(void* ud, int notification_code, va_list va) if (action_list[i].action_decs == NULL) break; - if (cmdOptions.use_tainting_engine && strcmp(action_list[i].menu_path, "SMT Solver/") == 0) { + if (cmdOptions.use_tainting_engine && strcmp(action_list[i].menu_path, "[Ponce] SMT Solver/") == 0) { /* Do not populate SMT Solver options if we use the tainting engine*/ continue; } @@ -313,12 +313,12 @@ ssize_t idaapi ui_callback(void* ud, int notification_code, va_list va) if (view_type == BWN_DISASM) { //&& !(is_debugger_on() && !ponce_runtime_status.runtimeTrigger.getState())) { // Don't let solve formulas if user is debugging natively - for (const auto& pc : tritonCtx.getPathConstraints()) { - auto temp = pc.getBranchConstraints(); + for (const auto& pathConstraint : tritonCtx.getPathConstraints()) { + auto temp = pathConstraint.getBranchConstraints(); } /* For the selected address(cur_ea), let's count how many branches we can reach (how many non taken addresses are in reach)*/ - int non_taken_branches_n = std::count_if(tritonCtx.getPathConstraints().begin(), tritonCtx.getPathConstraints().end(), [cur_ea](const auto& pc) { - for (auto const& [taken, srcAddr, dstAddr, pc] : pc.getBranchConstraints()) { + int non_taken_branches_n = std::count_if(tritonCtx.getPathConstraints().begin(), tritonCtx.getPathConstraints().end(), [cur_ea](const auto& pathConstraint) { + for (auto const& [taken, srcAddr, dstAddr, constraint] : pathConstraint.getBranchConstraints()) { if (cur_ea == srcAddr && !taken) return true; } return false; @@ -327,16 +327,16 @@ ssize_t idaapi ui_callback(void* ud, int notification_code, va_list va) if (non_taken_branches_n == 0) { // Disabled menu (so the user knows it's an option in some cases), already registered, we just need to attach it to the popup - attach_action_to_popup(form, popup_handle, action_IDA_solve_formula_sub.name, "SMT Solver/Solve formula", SETMENU_INS); + attach_action_to_popup(form, popup_handle, action_IDA_solve_formula_sub.name, "[Ponce] SMT Solver/Solve formula", SETMENU_INS); } else if (non_taken_branches_n == 1) { // There is only one non taken branch, no need to add submenus // But we need to modify the Solve Formula menu with more info and the path constraint index // The tooltip is not updated on the update event, we need to unregister the Solve formula submenu and add a new one unsigned int path_constraint_index = 0; - for (const auto& pc : tritonCtx.getPathConstraints()) { - for (auto const& [taken, srcAddr, dstAddr, pc] : pc.getBranchConstraints()) { - if (cur_ea == srcAddr && !taken) { // get the non taken branch for the path constraint the user clicked on + for (const auto& pathConstraint : tritonCtx.getPathConstraints()) { + for (auto const& [taken, srcAddr, dstAddr, constraint] : pathConstraint.getBranchConstraints()) { + if (cur_ea == srcAddr && !taken) { // get the non taken branch for the path constraint the user clicked on // Using the solve formula as template attach_action_solve(dstAddr, path_constraint_index, form, popup_handle, 0); break; @@ -350,11 +350,11 @@ ssize_t idaapi ui_callback(void* ud, int notification_code, va_list va) // Fix https://github.com/illera88/Ponce/issues/116 if (non_taken_branches_n <= 5) { unsigned int path_constraint_index = 0; - for (const auto& pc : tritonCtx.getPathConstraints()) { - for (auto const& [taken, srcAddr, dstAddr, pc] : pc.getBranchConstraints()) { - if (cur_ea == srcAddr && taken) { // get the taken branch for the path constraint the user clicked on + for (const auto& pathConstraint : tritonCtx.getPathConstraints()) { + for (auto const& [taken, srcAddr, dstAddr, constraint] : pathConstraint.getBranchConstraints()) { + if (cur_ea == srcAddr && taken) { // get the taken branch for the path constraint the user clicked on // Using the solve formula as template (If not we modify the name of the main solve formula menu) - attach_action_solve(dstAddr, path_constraint_index, form, popup_handle, 1); + attach_action_solve(dstAddr, path_constraint_index, form, popup_handle, 1); break; } } @@ -369,9 +369,9 @@ ssize_t idaapi ui_callback(void* ud, int notification_code, va_list va) unsigned int path_constraint_index = 0; unsigned int count = 0; // Show the first two - for (const auto& pc : tritonCtx.getPathConstraints()) { - for (auto const& [taken, srcAddr, dstAddr, pc] : pc.getBranchConstraints()) { - if (cur_ea == srcAddr && taken) { // get the taken branch for the path constraint the user clicked on + for (const auto& pathConstraint : tritonCtx.getPathConstraints()) { + for (auto const& [taken, srcAddr, dstAddr, constraint] : pathConstraint.getBranchConstraints()) { + if (cur_ea == srcAddr && taken) { // get the taken branch for the path constraint the user clicked on if (count == 2) // Only adding the first two break; // Using the solve formula as template (If not we modify the name of the main solve formula menu) @@ -395,8 +395,8 @@ ssize_t idaapi ui_callback(void* ud, int notification_code, va_list va) struct pair_address_index holder[2] = {0}; // Show the last two for (auto rit = std::rbegin(tritonCtx.getPathConstraints()); rit != std::rend(tritonCtx.getPathConstraints()); ++rit) { - for (auto const& [taken, srcAddr, dstAddr, pc] : rit->getBranchConstraints()) { - if (cur_ea == srcAddr && taken) { // get the taken branch for the path constraint the user clicked on + for (auto const& [taken, srcAddr, dstAddr, constraint] : rit->getBranchConstraints()) { + if (cur_ea == srcAddr && taken) { // get the taken branch for the path constraint the user clicked on if (count == 2) // Only adding the first two break; // Using the solve formula as template (If not we modify the name of the main solve formula menu) diff --git a/src/globals.hpp b/src/globals.hpp index 717fdb6b..104a54ae 100644 --- a/src/globals.hpp +++ b/src/globals.hpp @@ -102,6 +102,10 @@ extern std::map ponce_comments; #define WOPN_DP_TAB WOPN_TAB #endif +/* For forwards compatibility with IDA SDK >= 9.0 */ +#if IDA_SDK_VERSION >= 900 +#endif + #ifdef __EA64__ #define MEM_FORMAT "%#" PRIx64 #define REG_XIP tritonCtx.registers.x86_rip diff --git a/src/main.cpp b/src/main.cpp index c8f29d76..2264317f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -71,22 +71,22 @@ bool idaapi run(size_t) // Set the name for the action depending if using tainting or symbolic engine if (cmdOptions.use_tainting_engine) { - action_list[3].menu_path = TAINT; + action_list[3].menu_path = "[Ponce] " TAINT; action_IDA_taint_symbolize_register.label = TAINT_REG; action_IDA_taint_symbolize_register.tooltip = COMMENT_TAINT_REG; - action_list[4].menu_path = TAINT; + action_list[4].menu_path = "[Ponce] " TAINT; action_IDA_taint_symbolize_memory.label = TAINT_MEM; action_IDA_taint_symbolize_memory.tooltip = COMMENT_TAINT_MEM; } else { - action_list[3].menu_path = SYMBOLIC; + action_list[3].menu_path = "[Ponce] " SYMBOLIC; action_IDA_taint_symbolize_register.label = symbolize_REG; action_IDA_taint_symbolize_register.tooltip = COMMENT_SYMB_REG; - action_list[4].menu_path = SYMBOLIC; + action_list[4].menu_path = "[Ponce] " SYMBOLIC; action_IDA_taint_symbolize_memory.tooltip = COMMENT_SYMB_MEM; - action_IDA_taint_symbolize_memory.label = symbolize_MEM; + action_IDA_taint_symbolize_memory.label = symbolize_MEM; } /* Init the IDA actions depending on the IDA SDK version we build with*/ @@ -127,7 +127,9 @@ bool idaapi run(size_t) } //-------------------------------------------------------------------------- -#if IDA_SDK_VERSION >= 760 +#if IDA_SDK_VERSION >= 900 +plugmod_t* idaapi init(void) +#elif IDA_SDK_VERSION >= 760 plugmod_t* idaapi init(void) #elif IDA_SDK_VERSION == 750 size_t idaapi init(void) @@ -176,6 +178,19 @@ void idaapi term(void) snapshot.resetEngine(); // We want to delete Ponce comments and colours before terminating delete_ponce_comments(); + + // Fix Memory Leak: blacklkistedUserFunctions + if (blacklkistedUserFunctions) { + delete blacklkistedUserFunctions; + blacklkistedUserFunctions = nullptr; + } + + // Fix Memory Leak: ponce_table_chooser + if (ponce_table_chooser) { + delete ponce_table_chooser; + ponce_table_chooser = nullptr; + } + #ifdef BUILD_HEXRAYS_SUPPORT if(hexrays_present) { remove_hexrays_callback(ponce_hexrays_callback, NULL); diff --git a/src/snapshot.cpp b/src/snapshot.cpp index a0c1728a..7f37d918 100644 --- a/src/snapshot.cpp +++ b/src/snapshot.cpp @@ -17,12 +17,18 @@ Snapshot::Snapshot() { this->locked = true; this->snapshotTaintEngine = nullptr; this->snapshotSymEngine = nullptr; + this->astCtx = nullptr; + this->cpu_x8664 = nullptr; + this->cpu_x86 = nullptr; + this->cpu_AArch64 = nullptr; + this->cpu_Arm32 = nullptr; this->mustBeRestore = false; this->snapshotTaken = false; } Snapshot::~Snapshot() { + resetEngine(); } /* Check if the snapshot has been taken */ @@ -39,6 +45,10 @@ void Snapshot::addModification(ea_t mem, char byte) { /* Enable the snapshot engine. */ void Snapshot::takeSnapshot() { + if (this->snapshotTaken) { + resetEngine(); + } + this->snapshotTaken = true; /* 1 - Unlock the engine */ @@ -161,14 +171,37 @@ void Snapshot::resetEngine(void) { return; this->memory.clear(); - - //ToDo: We should delete this when this issue is fixed: https://github.com/JonathanSalwan/Triton/issues/385 delete this->snapshotSymEngine; this->snapshotSymEngine = nullptr; delete this->snapshotTaintEngine; this->snapshotTaintEngine = nullptr; + delete this->astCtx; + this->astCtx = nullptr; + + // Fix Memleak + switch (tritonCtx.getArchitecture()) { + case triton::arch::ARCH_X86_64: + delete this->cpu_x8664; + this->cpu_x8664 = nullptr; + break; + case triton::arch::ARCH_X86: + delete this->cpu_x86; + this->cpu_x86 = nullptr; + break; + case triton::arch::ARCH_AARCH64: + delete this->cpu_AArch64; + this->cpu_AArch64 = nullptr; + break; + case triton::arch::ARCH_ARM32: + delete this->cpu_Arm32; + this->cpu_Arm32 = nullptr; + break; + default: + break; + } + this->snapshotTaken = false; //We delete the comment and color that we created @@ -193,4 +226,4 @@ bool Snapshot::mustBeRestored(void) { /* Check if we must restore the snapshot */ void Snapshot::setRestore(bool flag) { this->mustBeRestore = flag; -} +} \ No newline at end of file diff --git a/src/symVarTable.cpp b/src/symVarTable.cpp index 6f0125e5..d6ef77e4 100644 --- a/src/symVarTable.cpp +++ b/src/symVarTable.cpp @@ -97,7 +97,10 @@ void idaapi ponce_table_chooser_t::get_row(qstrvec_t* cols_, int*, chooser_item_ size_t n) const { qstrvec_t& cols = *cols_; - list_item_t li = table_item_list.at(n); + if (n >= table_item_list.size()) + return; + + const list_item_t& li = table_item_list[n]; cols[0].sprnt("%lu", li.id); cols[1].sprnt("%s", li.var_name.c_str()); diff --git a/src/triton_logic.cpp b/src/triton_logic.cpp index 293b5780..c56d5427 100644 --- a/src/triton_logic.cpp +++ b/src/triton_logic.cpp @@ -153,28 +153,27 @@ int tritonize(ea_t pc, thid_t threadID) return 0; } -bool ponce_set_triton_architecture() { - if (ph.id == PLFM_386) { - if (ph.use64()) +bool ponce_set_triton_architecture() +{ + if (PH.id == PLFM_386) { + if (inf_is_64bit()) { + msg("[+] Using x64\n"); tritonCtx.setArchitecture(triton::arch::ARCH_X86_64); - else if (ph.use32()) - tritonCtx.setArchitecture(triton::arch::ARCH_X86); + } else { - msg("[e] Wrong architecture\n"); - return false; + msg("[+] Using x32\n"); + tritonCtx.setArchitecture(triton::arch::ARCH_X86); } - } - else if (ph.id == PLFM_ARM) { - if (ph.use64()) + } else if (PH.id == PLFM_ARM) { + if (inf_is_64bit()) { + msg("[+] Using ARM64\n"); tritonCtx.setArchitecture(triton::arch::ARCH_AARCH64); - else if (ph.use32()) - tritonCtx.setArchitecture(triton::arch::ARCH_ARM32); + } else { - msg("[e] Wrong architecture\n"); - return false; + msg("[+] Using ARM32\n"); + tritonCtx.setArchitecture(triton::arch::ARCH_ARM32); } - } - else { + } else { msg("[e] Architecture not supported by Ponce\n"); return false; } From 3c9980eff2d59288844d304bacb4e1984ab0a2f6 Mon Sep 17 00:00:00 2001 From: DexFinder <63297382+shuimu5418@users.noreply.github.com> Date: Thu, 5 Jun 2025 15:10:04 +0800 Subject: [PATCH 2/2] I have tried my best to fix memleak.... --- .gitignore | 3 +- README.md | 13 +-- src/snapshot.cpp | 286 +++++++++++++++++++++++++++++++++-------------- src/snapshot.hpp | 4 + 4 files changed, 213 insertions(+), 93 deletions(-) diff --git a/.gitignore b/.gitignore index 5fe664d9..6c04642f 100644 --- a/.gitignore +++ b/.gitignore @@ -67,4 +67,5 @@ vcomp110.dll # i#20 For some reason the script can not generate this file at Appveyor site. Let's ignore it for a while. # VERSION_NUMBER -src_for_ida9 \ No newline at end of file +src_for_ida9 +test \ No newline at end of file diff --git a/README.md b/README.md index 0dc324fe..ba0aa0dd 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ Ponce works on Windows, Linux and OSX natively! ### Use cases * **Exploit development**: Ponce can help you create an exploit in a far more efficient manner as the exploit developer may easily see what parts of memory and which registers you control, as well as possible addresses which can be leveraged as ROP gadgets. -* **Malware Analysis**: Another use of Ponce is related to malware code. Analyzing the commands a particular family of malware supports is easily determined by symbolizing a simple known command and negating all the conditions where the command is being checked. +* **Malware Analysis**: Another use of Ponce is related to malware code. Analyzing the commands a particular family of malware supports is easily determined by symbolizing a simple known command and negating all the conditions where the command is being checked. * **Protocol Reversing**: One of the most interesting Ponce uses is the possibility of recognizing required magic numbers, headers or even entire protocols for controlled user input. For instance, Ponce can help you to list all the accepted arguments for a given command line binary or extract the file format required for a specific file parser. * **CTF**: Ponce speeds up the process of reverse engineer binaries during CTFs. As Ponce is totally integrated into IDA you don't need to worry about setup timing. It's ready to be used! @@ -40,7 +40,7 @@ The plugin will automatically run, guiding you through the initial configuration ### Use modes * **Tainting engine**: This engine is used to determine at every step of the binary's execution which parts of memory and registers are controllable by the user input. -* **Symbolic engine**: This engine maintains a symbolic state of registers and part of memory at each step in a binary's execution path. +* **Symbolic engine**: This engine maintains a symbolic state of registers and part of memory at each step in a binary's execution path. ### Examples @@ -154,7 +154,7 @@ Yes, you can natively use Ponce in IDA for Windows or remotely attach to a Linux In our tests we reach to process 3000 instructions per second. We plan to use the PIN tracer IDA offers to increase the speed. -#### Something is not working! +#### Something is not working Open an [issue](https://github.com/illera88/Ponce/issues), we will solve it ASAP ;\) @@ -166,7 +166,7 @@ Sure! Please do pull requests and work in the opened issues. We will pay you in Concolic execution and Ponce have some problems: -* Symbolic memory load/write: When the index used to read a memory value is symbolic like in `x = aray[symbolic_index]` some problems arise that could lead on the loose of track of the tainted/symbolized user controled input. +* Symbolic memory load/write: When the index used to read a memory value is symbolic like in `x = aray[symbolic_index]` some problems arise that could lead on the loose of track of the tainted/symbolized user controled input. * Triton doesn't work very well with [floating point instructions](https://github.com/illera88/Ponce/issues/59). * Concolic execution only analyzed the executed instructions. That means that symbolic tracking is lost in cases like the following: @@ -184,6 +184,5 @@ Concolic execution and Ponce have some problems: ### Authors -* Alberto Garcia Illera \([@algillera](https://twitter.com/algillera)\) agarciaillera@gmail.com -* Francisco Oca \([@francisco\_oca](https://twitter.com/francisco_oca)\) francisco.oca.gonzalez@gmail.com - +* Alberto Garcia Illera \([@algillera](https://twitter.com/algillera)\) +* Francisco Oca \([@francisco\_oca](https://twitter.com/francisco_oca)\) diff --git a/src/snapshot.cpp b/src/snapshot.cpp index 7f37d918..6022fa3e 100644 --- a/src/snapshot.cpp +++ b/src/snapshot.cpp @@ -28,7 +28,14 @@ Snapshot::Snapshot() { Snapshot::~Snapshot() { - resetEngine(); + try { + if (this->snapshotTaken) { + resetEngine(); + } + } catch(...) { + // Destructor must not throw + msg("[!] Snapshot: Exception in destructor.\n"); + } } /* Check if the snapshot has been taken */ @@ -49,41 +56,75 @@ void Snapshot::takeSnapshot() { resetEngine(); } - this->snapshotTaken = true; - - /* 1 - Unlock the engine */ + /* Unlock the engine */ this->locked = false; + // snapshotTaken should only be set to true at the very end if everything succeeds + try { + /* Save current symbolic engine state */ + // WARNING: According to Triton issue #385, this creates a shallow copy + // Do NOT delete this object in resetEngine() to prevent double free + auto* symEngine = tritonCtx.getSymbolicEngine(); + if (symEngine == nullptr) { + throw std::runtime_error("SymbolicEngine is null"); + } + this->snapshotSymEngine = new triton::engines::symbolic::SymbolicEngine(*symEngine); - /* 2 - Save current symbolic engine state */ - this->snapshotSymEngine = new triton::engines::symbolic::SymbolicEngine(*tritonCtx.getSymbolicEngine()); - - /* 3 - Save current taint engine state */ - this->snapshotTaintEngine = new triton::engines::taint::TaintEngine(*tritonCtx.getTaintEngine()); + /* Save current taint engine state */ + // WARNING: This is also likely a shallow copy + auto* taintEngine = tritonCtx.getTaintEngine(); + if (taintEngine == nullptr) { + throw std::runtime_error("TaintEngine is null"); + } + this->snapshotTaintEngine = new triton::engines::taint::TaintEngine(*taintEngine); - /* 4 - Save current set of nodes */ - this->astCtx = new triton::ast::AstContext(*tritonCtx.getAstContext()); + /* Save current set of nodes */ + // WARNING: This is also likely a shallow copy + auto astContext = tritonCtx.getAstContext(); + if (astContext == nullptr) { + throw std::runtime_error("AstContext is null"); + } + this->astCtx = new triton::ast::AstContext(*astContext); //this->nodesList = tritonCtx.getAllocatedAstNodes(); - /* 5 - Save the Triton CPU state. It depens on the analyzed binary*/ - switch (tritonCtx.getArchitecture()) { - case triton::arch::ARCH_X86_64: - this->cpu_x8664 = new triton::arch::x86::x8664Cpu(*dynamic_cast(tritonCtx.getCpuInstance())); - break; - case triton::arch::ARCH_X86: - this->cpu_x86 = new triton::arch::x86::x86Cpu(*reinterpret_cast(tritonCtx.getCpuInstance())); - break; - case triton::arch::ARCH_AARCH64: - this->cpu_AArch64 = new triton::arch::arm::aarch64::AArch64Cpu(*reinterpret_cast(tritonCtx.getCpuInstance())); - break; - case triton::arch::ARCH_ARM32: - this->cpu_Arm32 = new triton::arch::arm::arm32::Arm32Cpu(*reinterpret_cast(tritonCtx.getCpuInstance())); - break; - default: - throw triton::exceptions::Architecture("Architecture not supported."); - break; - } + /* Save the Triton CPU state. It depends on the analyzed binary*/ + switch (tritonCtx.getArchitecture()) { + case triton::arch::ARCH_X86_64: { + auto* cpu_ptr = dynamic_cast(tritonCtx.getCpuInstance()); + if (cpu_ptr == nullptr) { + throw std::runtime_error("Failed to cast CPU instance to x8664Cpu"); + } + this->cpu_x8664 = new triton::arch::x86::x8664Cpu(*cpu_ptr); + break; + } + case triton::arch::ARCH_X86: { + auto* cpu_ptr = dynamic_cast(tritonCtx.getCpuInstance()); + if (cpu_ptr == nullptr) { + throw std::runtime_error("Failed to cast CPU instance to x86Cpu"); + } + this->cpu_x86 = new triton::arch::x86::x86Cpu(*cpu_ptr); + break; + } + case triton::arch::ARCH_AARCH64: { + auto* cpu_ptr = dynamic_cast(tritonCtx.getCpuInstance()); + if (cpu_ptr == nullptr) { + throw std::runtime_error("Failed to cast CPU instance to AArch64Cpu"); + } + this->cpu_AArch64 = new triton::arch::arm::aarch64::AArch64Cpu(*cpu_ptr); + break; + } + case triton::arch::ARCH_ARM32: { + auto* cpu_ptr = dynamic_cast(tritonCtx.getCpuInstance()); + if (cpu_ptr == nullptr) { + throw std::runtime_error("Failed to cast CPU instance to Arm32Cpu"); + } + this->cpu_Arm32 = new triton::arch::arm::arm32::Arm32Cpu(*cpu_ptr); + break; + } + default: + throw triton::exceptions::Architecture("Architecture not supported."); + } - /* 6 - Save IDA registers context */ + /* Save IDA registers context */ auto regs = tritonCtx.getAllRegisters(); for (auto it = regs.begin(); it != regs.end(); it++) { @@ -93,9 +134,30 @@ void Snapshot::takeSnapshot() { this->IDAContext[reg.getName()] = ival; } } + // We also saved the ponce status + this->saved_ponce_runtime_status = ponce_runtime_status; + + // Only set snapshotTaken to true if everything succeeded + this->snapshotTaken = true; + + } catch (const std::exception& e) { + msg("[!] Error taking snapshot: %s\n", e.what()); - //We also saved the ponce status - this->saved_ponce_runtime_status = ponce_runtime_status; + delete this->snapshotSymEngine; this->snapshotSymEngine = nullptr; + delete this->snapshotTaintEngine; this->snapshotTaintEngine = nullptr; + delete this->astCtx; this->astCtx = nullptr; + delete this->cpu_x8664; this->cpu_x8664 = nullptr; + delete this->cpu_x86; this->cpu_x86 = nullptr; + delete this->cpu_AArch64; this->cpu_AArch64 = nullptr; + delete this->cpu_Arm32; this->cpu_Arm32 = nullptr; + + // Reset state + this->locked = true; + this->snapshotTaken = false; + + // Re-throw the exception + throw; + } } void Snapshot::setAddress(ea_t address) { @@ -105,44 +167,109 @@ void Snapshot::setAddress(ea_t address) { /* Restore the snapshot. */ void Snapshot::restoreSnapshot() { + // Check if snapshot was actually taken + if (!this->snapshotTaken) { + msg("[!] Cannot restore snapshot: no snapshot was taken\n"); + return; + } - /* 1 - Restore all memory modification. */ + /* Restore all memory modification. */ for (auto i = this->memory.begin(); i != this->memory.end(); ++i) { put_bytes(i->first, &i->second, 1); } this->memory.clear(); - /* 2 - Restore current symbolic engine state */ - *tritonCtx.getSymbolicEngine() = *this->snapshotSymEngine; + /* Restore current symbolic engine state */ + if (this->snapshotSymEngine != nullptr) { + auto* symEngine = tritonCtx.getSymbolicEngine(); + if (symEngine != nullptr) { + *symEngine = *this->snapshotSymEngine; + } else { + msg("[!] ERROR: Current SymbolicEngine is null\n"); + } + } - /* 3 - Restore current taint engine state */ - *tritonCtx.getTaintEngine() = *this->snapshotTaintEngine; + /* Restore current taint engine state */ + if (this->snapshotTaintEngine != nullptr) { + auto* taintEngine = tritonCtx.getTaintEngine(); + if (taintEngine != nullptr) { + *taintEngine = *this->snapshotTaintEngine; + } else { + msg("[!] ERROR: Current TaintEngine is null\n"); + } + } - /* 4 - Restore current AST context */ - *tritonCtx.getAstContext() = *this->astCtx; + /* Restore current AST context */ + if (this->astCtx != nullptr) { + auto astContext = tritonCtx.getAstContext(); + if (astContext != nullptr) { + *astContext = *this->astCtx; + } else { + msg("[!] ERROR: Current AstContext is null\n"); + } + } - /* 5 - Restore the Triton CPU state */ + /* Restore the Triton CPU state */ switch (tritonCtx.getArchitecture()) { - case triton::arch::ARCH_X86_64: - *reinterpret_cast(tritonCtx.getCpuInstance()) = *this->cpu_x8664; + case triton::arch::ARCH_X86_64: { + if (this->cpu_x8664 != nullptr) { + auto* current_cpu = dynamic_cast(tritonCtx.getCpuInstance()); + if (current_cpu != nullptr) { + *current_cpu = *this->cpu_x8664; + } else { + msg("[!] ERROR: Failed to cast current CPU instance to x8664Cpu\n"); + } + } else { + msg("[!] ERROR: Snapshot CPU x8664 is null\n"); + } break; - case triton::arch::ARCH_X86: - *reinterpret_cast(tritonCtx.getCpuInstance()) = *this->cpu_x86; + } + case triton::arch::ARCH_X86: { + if (this->cpu_x86 != nullptr) { + auto* current_cpu = dynamic_cast(tritonCtx.getCpuInstance()); + if (current_cpu != nullptr) { + *current_cpu = *this->cpu_x86; + } else { + msg("[!] ERROR: Failed to cast current CPU instance to x86Cpu\n"); + } + } else { + msg("[!] ERROR: Snapshot CPU x86 is null\n"); + } break; - case triton::arch::ARCH_AARCH64: - *reinterpret_cast(tritonCtx.getCpuInstance()) = *this->cpu_AArch64; + } + case triton::arch::ARCH_AARCH64: { + if (this->cpu_AArch64 != nullptr) { + auto* current_cpu = dynamic_cast(tritonCtx.getCpuInstance()); + if (current_cpu != nullptr) { + *current_cpu = *this->cpu_AArch64; + } else { + msg("[!] ERROR: Failed to cast current CPU instance to AArch64Cpu\n"); + } + } else { + msg("[!] ERROR: Snapshot CPU AArch64 is null\n"); + } break; - case triton::arch::ARCH_ARM32: - *reinterpret_cast(tritonCtx.getCpuInstance()) = *this->cpu_Arm32; + } + case triton::arch::ARCH_ARM32: { + if (this->cpu_Arm32 != nullptr) { + auto* current_cpu = dynamic_cast(tritonCtx.getCpuInstance()); + if (current_cpu != nullptr) { + *current_cpu = *this->cpu_Arm32; + } else { + msg("[!] ERROR: Failed to cast current CPU instance to Arm32Cpu\n"); + } + } else { + msg("[!] ERROR: Snapshot CPU Arm32 is null\n"); + } break; + } default: throw triton::exceptions::Architecture("Architecture not supported."); - break; } this->mustBeRestore = false; - /* 6 - Restore IDA registers context + /* Restore IDA registers context Suposedly XIP should be set at the same time and execution redirected*/ typedef std::map::iterator it_type; for (it_type iterator = this->IDAContext.begin(); iterator != this->IDAContext.end(); iterator++) { @@ -150,10 +277,9 @@ void Snapshot::restoreSnapshot() { msg("[!] ERROR restoring register %s\n", iterator->first.c_str()); } - /* 7 - Restore the Ponce status */ ponce_runtime_status = this->saved_ponce_runtime_status; - /* 8 - We need to set to NULL the last instruction. We are deleting the last instructions in the Tritonize callback. + /* We need to set to NULL the last instruction. We are deleting the last instructions in the Tritonize callback. So after restore a snapshot if last_instruction is not NULL is double freeing the same instruction */ ponce_runtime_status.last_triton_instruction = nullptr; } @@ -167,46 +293,36 @@ void Snapshot::disableSnapshot(void) { /* Reset the snapshot engine. * Clear all backups for a new snapshot. */ void Snapshot::resetEngine(void) { - if (!this->snapshotTaken) + if (!this->snapshotTaken) return; this->memory.clear(); - delete this->snapshotSymEngine; - this->snapshotSymEngine = nullptr; - delete this->snapshotTaintEngine; + this->snapshotSymEngine = nullptr; this->snapshotTaintEngine = nullptr; - - delete this->astCtx; this->astCtx = nullptr; - - // Fix Memleak - switch (tritonCtx.getArchitecture()) { - case triton::arch::ARCH_X86_64: - delete this->cpu_x8664; - this->cpu_x8664 = nullptr; - break; - case triton::arch::ARCH_X86: - delete this->cpu_x86; - this->cpu_x86 = nullptr; - break; - case triton::arch::ARCH_AARCH64: - delete this->cpu_AArch64; - this->cpu_AArch64 = nullptr; - break; - case triton::arch::ARCH_ARM32: - delete this->cpu_Arm32; - this->cpu_Arm32 = nullptr; - break; - default: - break; + + this->cpu_x8664 = nullptr; + this->cpu_x86 = nullptr; + this->cpu_AArch64 = nullptr; + // delete this->cpu_Arm32; + this->cpu_Arm32 = nullptr; + + // Clear IDA context and reset all state flags + this->IDAContext.clear(); + this->snapshotTaken = false; + this->locked = true; + this->mustBeRestore = false; + + if (this->address != 0) { + try { + ponce_set_cmt(this->address, "", false, false, false); + del_item_color(this->address); + } catch (...) { + // IDA API might fail during shutdown + msg("[!] Snapshot::resetEngine: Error calling IDA API during cleanup.\n"); + } } - - this->snapshotTaken = false; - - //We delete the comment and color that we created - ponce_set_cmt(this->address, "", false, false, false); - del_item_color(this->address); this->address = 0; } diff --git a/src/snapshot.hpp b/src/snapshot.hpp index 73d51ca5..e08f7bc7 100644 --- a/src/snapshot.hpp +++ b/src/snapshot.hpp @@ -87,6 +87,10 @@ class Snapshot { //! Destructor. ~Snapshot(); + //! Prevent copy construction and assignment to avoid shallow copy issues + Snapshot(const Snapshot&) = delete; + Snapshot& operator=(const Snapshot&) = delete; + //! Returns true if the snapshot engine is disabled. bool isLocked(void);