diff --git a/docs/changelog.txt b/docs/changelog.txt index 9481bbbbd8..e1b0e51eec 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -61,6 +61,7 @@ Template for new versions: ## Fixes ## Misc Improvements +- Core: DFHack now validates vtable pointers in objects read from memory and will throw an exception instead of crashing when an invalid vtable pointer is encountered. This makes it easier to identify which DF data structure contains corrupted data when this manifests in the form of a bad vtable pointer, and shifts blame for such crashes from DFHack to DF. ## Documentation diff --git a/library/DataDefs.cpp b/library/DataDefs.cpp index 54cdfff982..880a8b93a5 100644 --- a/library/DataDefs.cpp +++ b/library/DataDefs.cpp @@ -370,7 +370,7 @@ const virtual_identity *virtual_identity::find(void *vtable) // If using a reader/writer lock, re-grab as write here, and recheck Core &core = Core::getInstance(); - std::string name = core.p->doReadClassName(vtable); + std::string name = core.p->readClassName(vtable); auto name_it = (*name_lookup).find(name); if (name_it != (*name_lookup).end()) { diff --git a/library/Process.cpp b/library/Process.cpp index 3e8ae8de8d..3e3ba6db80 100644 --- a/library/Process.cpp +++ b/library/Process.cpp @@ -236,6 +236,9 @@ Process::~Process() string Process::doReadClassName (void * vptr) { + if (!checkValidAddress(vptr)) + throw std::runtime_error(fmt::format("invalid vtable ptr {}", vptr)); + char* rtti = Process::readPtr(((char*)vptr - sizeof(void*))); #ifndef WIN32 char* typestring = Process::readPtr(rtti + sizeof(void*)); @@ -591,6 +594,20 @@ void Process::getMemRanges(vector& ranges) } #endif +bool Process::checkValidAddress(void* ptr) +{ + uintptr_t addr = reinterpret_cast(ptr); + auto validate = [&] (t_memrange& r) { + uintptr_t lo = reinterpret_cast(r.start); + uintptr_t hi = reinterpret_cast(r.end); + return addr >= lo && addr < hi; + }; + std::vector mr; + getMemRanges(mr); + bool valid = std::any_of(mr.begin(), mr.end(), validate); + return valid; +} + uintptr_t Process::getBase() { #if WIN32 diff --git a/library/include/MemAccess.h b/library/include/MemAccess.h index 19d65468ed..5115348ba2 100644 --- a/library/include/MemAccess.h +++ b/library/include/MemAccess.h @@ -221,7 +221,7 @@ namespace DFHack std::string readClassName(void* vptr) { - std::map::iterator it = classNameCache.find(vptr); + auto it = classNameCache.find(vptr); if (it != classNameCache.end()) return it->second; return classNameCache[vptr] = doReadClassName(vptr); @@ -247,6 +247,9 @@ namespace DFHack /// get virtual memory ranges of the process (what is mapped where) static void getMemRanges(std::vector& ranges); + /// check if an address has a mapping + bool checkValidAddress(void* ptr); + /// get the symbol table extension of this process std::shared_ptr getDescriptor() {