From 9b8bcc70ec79e874a0dc7ee881c43fe873d59720 Mon Sep 17 00:00:00 2001 From: Herzeh Date: Thu, 10 Dec 2015 02:12:01 +0100 Subject: [PATCH] CPUDisassembly : Removed some code that was repeated from DisassembleAt function Added a warning when user tries to assemble an instruction thats points to Non-executable memory and DEP is enabled --- src/bridge/bridgemain.cpp | 5 ++ src/bridge/bridgemain.h | 2 + src/dbg/_global.h | 2 +- src/dbg/assemble.cpp | 83 ++++++++++++++++++++++++++++++ src/dbg/assemble.h | 1 + src/gui/Src/Bridge/Bridge.cpp | 8 +++ src/gui/Src/Bridge/Bridge.h | 1 + src/gui/Src/Gui/CPUDisassembly.cpp | 25 +++------ src/gui/Src/Gui/CPUDisassembly.h | 1 + 9 files changed, 109 insertions(+), 19 deletions(-) diff --git a/src/bridge/bridgemain.cpp b/src/bridge/bridgemain.cpp index 93696359..0b6c81a8 100644 --- a/src/bridge/bridgemain.cpp +++ b/src/bridge/bridgemain.cpp @@ -1250,6 +1250,11 @@ BRIDGE_IMPEXP void GuiDumpAtN(duint va, int index) _gui_sendmessage(GUI_DUMP_AT_N, (void*)va, (void*)index); } +BRIDGE_IMPEXP void GuiDisplayWarning(const char *title, const char *text) +{ + _gui_sendmessage(GUI_DISPLAY_WARNING, (void*)title, (void*)text); +} + BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) { hInst = hinstDLL; diff --git a/src/bridge/bridgemain.h b/src/bridge/bridgemain.h index c97345ce..37e7ecaf 100644 --- a/src/bridge/bridgemain.h +++ b/src/bridge/bridgemain.h @@ -787,6 +787,7 @@ typedef enum GUI_SET_DEBUGGEE_NOTES, // param1=const char* text, param2=unused GUI_GET_DEBUGGEE_NOTES, // param1=char** text, param2=unused GUI_DUMP_AT_N, // param1=int index, param2=duint va + GUI_DISPLAY_WARNING // param1=const char *text, param2=unused } GUIMSG; //GUI Typedefs @@ -886,6 +887,7 @@ BRIDGE_IMPEXP void GuiGetGlobalNotes(char** text); BRIDGE_IMPEXP void GuiSetDebuggeeNotes(const char* text); BRIDGE_IMPEXP void GuiGetDebuggeeNotes(char** text); BRIDGE_IMPEXP void GuiDumpAtN(duint va, int index); +BRIDGE_IMPEXP void GuiDisplayWarning(const char *title, const char *text); #ifdef __cplusplus } diff --git a/src/dbg/_global.h b/src/dbg/_global.h index 37416281..c73d7f9c 100644 --- a/src/dbg/_global.h +++ b/src/dbg/_global.h @@ -1,7 +1,7 @@ #ifndef _GLOBAL_H #define _GLOBAL_H -#define _WIN32_WINNT 0x0501 +#define _WIN32_WINNT 0x0601 #define WINVER 0x0501 #define _WIN32_IE 0x0500 diff --git a/src/dbg/assemble.cpp b/src/dbg/assemble.cpp index 66a4300e..a61645bf 100644 --- a/src/dbg/assemble.cpp +++ b/src/dbg/assemble.cpp @@ -9,6 +9,7 @@ #include "XEDParse\XEDParse.h" #include "value.h" #include "disasm_helper.h" +#include "debugger.h" static bool cbUnknown(const char* text, ULONGLONG* value) { @@ -67,6 +68,10 @@ bool assembleat(duint addr, const char* instruction, int* size, char* error, boo if(size) *size = destSize; + // Check if the instruction doesn't set IP to non-executable memory + if (!isInstructionPointingToExMemory(addr, dest)) + GuiDisplayWarning("Non-executable memory region", "Assembled instruction points to non-executable memory region !"); + bool ret = MemPatch(addr, dest, destSize); if (ret) @@ -91,3 +96,81 @@ bool assembleat(duint addr, const char* instruction, int* size, char* error, boo return ret; } + +bool isInstructionPointingToExMemory(duint addr, const unsigned char* dest) +{ + MEMMAP wMemMapStruct; + DISASM_ARG arg; + DISASM_INSTR wDisasInstrStruct; + MEMORY_BASIC_INFORMATION *wMbiStruct = NULL; + duint instrArgCount; + dsint instrMemValues[3] = {0}; + duint instrMemValuesIndex = 0; + dsint mbiBaseAddr, mbiEndAddr; + + disasmget((unsigned char*)dest, 0, &wDisasInstrStruct); + + instrArgCount = wDisasInstrStruct.argcount; + if (instrArgCount) + { + // Loop through all arguements + for (int i = 0; i < wDisasInstrStruct.argcount; i++) + { + arg = wDisasInstrStruct.arg[i]; + + // Check if any of the arguments is a memory + if (arg.type == arg_memory) + { + instrMemValues[instrMemValuesIndex] = addr + arg.memvalue; // add current instruction VA for rip-relative addressing + instrMemValuesIndex++; + } + else if (wDisasInstrStruct.type == instr_branch) + { + instrMemValues[instrMemValuesIndex] = addr + arg.value; + instrMemValuesIndex++; + } + } + } + + // No memory pointer in the instruction, no need to go further + if (!instrMemValuesIndex) + return false; + + // Get memory map to locate the sections to which the instr memory address belongs to + DbgMemMap(&wMemMapStruct); + + // For each memPointerValue + for (auto & memValue : instrMemValues) + { + // Loop through the memMaps + for (int i = 0; i < wMemMapStruct.count; i++) + { + wMbiStruct = &(wMemMapStruct.page)[i].mbi; + mbiBaseAddr = (dsint)wMbiStruct->BaseAddress; + mbiEndAddr = (dsint)wMbiStruct->BaseAddress + (dsint)wMbiStruct->RegionSize; + if (memValue >= mbiBaseAddr && memValue < mbiEndAddr) + { + if (wMbiStruct->Protect == PAGE_EXECUTE || + wMbiStruct->Protect == PAGE_EXECUTE_READ || + wMbiStruct->Protect == PAGE_EXECUTE_READWRITE || + wMbiStruct->Protect == PAGE_EXECUTE_WRITECOPY) + { + // Memory region is marked as executable +#ifndef _WIN64 + DWORD lpFlagsDep; + BOOL bPermanentDep; + + // DEP is disabled if lpFlagsDep == 0 + if (GetProcessDEPPolicy(fdProcessInfo->hProcess, &lpFlagsDep, &bPermanentDep) && lpFlagsDep != 0) + return true; +#else + // DEP enabled on x64 + return true; +#endif + } + } + } + } + + return false; +} diff --git a/src/dbg/assemble.h b/src/dbg/assemble.h index 89865a55..6b4bd572 100644 --- a/src/dbg/assemble.h +++ b/src/dbg/assemble.h @@ -5,5 +5,6 @@ bool assemble(duint addr, unsigned char* dest, int* size, const char* instruction, char* error); bool assembleat(duint addr, const char* instruction, int* size, char* error, bool fillnop); +bool isInstructionPointingToExMemory(duint addr, const unsigned char* dest); #endif // _ASSEMBLE_H diff --git a/src/gui/Src/Bridge/Bridge.cpp b/src/gui/Src/Bridge/Bridge.cpp index c44417bb..d2f00d81 100644 --- a/src/gui/Src/Bridge/Bridge.cpp +++ b/src/gui/Src/Bridge/Bridge.cpp @@ -528,6 +528,14 @@ void* Bridge::processMessage(GUIMSG type, void* param1, void* param2) emit dumpAtN((duint)param1, (int)param2); } break; + + case GUI_DISPLAY_WARNING: + { + QString title = QString((const char*)param1); + QString text = QString((const char*)param2); + emit displayWarning(title, text); + } + break; } return nullptr; } diff --git a/src/gui/Src/Bridge/Bridge.h b/src/gui/Src/Bridge/Bridge.h index 8d8dc371..c7bc4ebe 100644 --- a/src/gui/Src/Bridge/Bridge.h +++ b/src/gui/Src/Bridge/Bridge.h @@ -115,6 +115,7 @@ signals: void setDebuggeeNotes(const QString text); void getDebuggeeNotes(void* text); void dumpAtN(duint va, int index); + void displayWarning(QString title, QString text); private: QMutex* mBridgeMutex; diff --git a/src/gui/Src/Gui/CPUDisassembly.cpp b/src/gui/Src/Gui/CPUDisassembly.cpp index 19b0cee1..e5d0d1c7 100644 --- a/src/gui/Src/Gui/CPUDisassembly.cpp +++ b/src/gui/Src/Gui/CPUDisassembly.cpp @@ -24,6 +24,7 @@ CPUDisassembly::CPUDisassembly(CPUWidget* parent) : Disassembly(parent) connect(Bridge::getBridge(), SIGNAL(dbgStateChanged(DBGSTATE)), this, SLOT(debugStateChangedSlot(DBGSTATE))); connect(Bridge::getBridge(), SIGNAL(selectionDisasmGet(SELECTIONDATA*)), this, SLOT(selectionGetSlot(SELECTIONDATA*))); connect(Bridge::getBridge(), SIGNAL(selectionDisasmSet(const SELECTIONDATA*)), this, SLOT(selectionSetSlot(const SELECTIONDATA*))); + connect(Bridge::getBridge(), SIGNAL(displayWarning(QString, QString)), this, SLOT(displayWarningSlot(QString, QString))); Initialize(); } @@ -704,24 +705,7 @@ void CPUDisassembly::assembleSlot() duint wVA = rvaToVa(wRVA); QString addr_text = QString("%1").arg(wVA, sizeof(dsint) * 2, 16, QChar('0')).toUpper(); - QByteArray wBuffer; - - dsint wMaxByteCountToRead = 16 * 2; - - //TODO: fix size problems - dsint size = getSize(); - if(!size) - size = wRVA; - - // Bounding - wMaxByteCountToRead = wMaxByteCountToRead > (size - wRVA) ? (size - wRVA) : wMaxByteCountToRead; - - wBuffer.resize(wMaxByteCountToRead); - - mMemPage->read(reinterpret_cast(wBuffer.data()), wRVA, wMaxByteCountToRead); - - QBeaEngine disasm(-1); - Instruction_t instr = disasm.DisassembleAt(reinterpret_cast(wBuffer.data()), wMaxByteCountToRead, 0, 0, wVA); + Instruction_t instr = this->DisassembleAt(wRVA); QString actual_inst = instr.instStr; @@ -1250,6 +1234,11 @@ void CPUDisassembly::decompileFunctionSlot() } } +void CPUDisassembly::displayWarningSlot(QString title, QString text) +{ + QMessageBox::QMessageBox(QMessageBox::Information, title, text, QMessageBox::Ok).exec(); +} + void CPUDisassembly::paintEvent(QPaintEvent* event) { // Hook/hack to update the sidebar at the same time as this widget. diff --git a/src/gui/Src/Gui/CPUDisassembly.h b/src/gui/Src/Gui/CPUDisassembly.h index 02da9752..fa51d5c8 100644 --- a/src/gui/Src/Gui/CPUDisassembly.h +++ b/src/gui/Src/Gui/CPUDisassembly.h @@ -83,6 +83,7 @@ public slots: void openSourceSlot(); void decompileSelectionSlot(); void decompileFunctionSlot(); + void displayWarningSlot(QString title, QString text); protected: void paintEvent(QPaintEvent* event);