Harden the view layer against malformed input (found via real arm64 binaries)

src/libmoex/node/MachSection.cpp

@@ -31,8 +31,23 @@ char *MachSection::GetOffset(){
     return offset;
 }
 uint32_t MachSection::GetSize() {
-    uint32_t size = (uint32_t) sect().size_both();
-    return size;
+    uint64_t size = sect().size_both();
+    // Clamp the section data size to the mapped file so the data readers
+    // (string/literal/pointer parsing) cannot run past a truncated section.
+    auto c = ctx();
+    if(c && c->file_start != nullptr){
+        const char *off = GetOffset();
+        const char *fstart = static_cast<const char*>(c->file_start);
+        if(off < fstart)
+            return 0;
+        const uint64_t off_in_file = static_cast<uint64_t>(off - fstart);
+        if(off_in_file >= c->file_size)
+            return 0;
+        const uint64_t avail = c->file_size - off_in_file;
+        if(size > avail)
+            size = avail;
+    }
+    return (uint32_t)size;
 }
 
 void MachSection::ForEachAs_S_CSTRING_LITERALS(std::function<void(char* str)> callback){

src/libmoex/node/Util.cpp

@@ -473,10 +473,11 @@ namespace util {
             bit += 7;
         } while (byte & 0x80);
 
-        // sign extend negative numbers
+        // sign extend negative numbers (shift unsigned to avoid UB on a
+        // negative left shift, then reinterpret).
         if ( (byte & 0x40) != 0 && bit < 64 )
         {
-            result |= (-1LL) << bit;
+            result |= static_cast<int64_t>(~0ULL << bit);
         }
 
         data = result;
@@ -499,10 +500,18 @@ namespace util {
                 ++cur;
                 continue;
             }
-            results.push_back(cur);
+            char *str_start = cur;
             while (cur < end && *cur != 0) {
                 ++cur;
             }
+            // Only return strings that are NUL-terminated within the region, so
+            // callers can safely treat the pointer as a C string.
+            if(cur < end){
+                results.push_back(str_start);
+                ++cur; // skip terminator
+            }else{
+                break; // unterminated trailing bytes (truncated/crafted input)
+            }
         }
 
         return results;

src/libmoex/node/loadcmd/LoadCommand_DYLD_INFO.cpp

@@ -100,6 +100,17 @@ void LoadCommand_DYLD_INFO::ForEachRebaseOpcode(std::function<void(const RebaseO
     char * begin = header()->header_start() + cmd()->rebase_off;
     uint32_t size = cmd()->rebase_size;
     char * end = begin + size;
+    // Clamp to the mapped file: a truncated/crafted rebase_off/size can run the
+    // opcode walk past the end of the file.
+    {
+        auto fc = header()->ctx();
+        if(fc && fc->file_start != nullptr){
+            char *fstart = (char*)fc->file_start;
+            char *fend = fstart + fc->file_size;
+            if(begin < fstart || begin > fend) end = begin;
+            else if(end > fend) end = fend;
+        }
+    }
     char * cur = begin;
     while(cur < end && !done){
         // read and move next
@@ -256,6 +267,17 @@ void LoadCommand_DYLD_INFO::ForEachBindingOpcode(BindNodeType node_type,uint32_t
     char * begin = header()->header_start() + bind_off;
     uint32_t size = bind_size;
     char * end = begin + size;
+    // Clamp to the mapped file: a truncated/crafted bind_off/size can run the
+    // opcode walk past the end of the file.
+    {
+        auto fc = header()->ctx();
+        if(fc && fc->file_start != nullptr){
+            char *fstart = (char*)fc->file_start;
+            char *fend = fstart + fc->file_size;
+            if(begin < fstart || begin > fend) end = begin;
+            else if(end > fend) end = fend;
+        }
+    }
     char * cur = begin;
     while(cur < end && !done) {
         // read and move next

src/libmoex/node/loadcmd/LoadCommand_DYSYMTAB.cpp

@@ -7,8 +7,22 @@
 
 MOEX_NAMESPACE_BEGIN
 
-
-
+namespace {
+// Clamp [offset, offset+size) to the mapped file so a truncated or crafted
+// table cannot make the iterators below dereference past the end.
+uint64_t ClampSizeToFile(const NodeContextPtr &ctx, const char *offset, uint64_t size){
+    if(!ctx || ctx->file_start == nullptr)
+        return size;
+    const char *fstart = static_cast<const char*>(ctx->file_start);
+    if(offset < fstart)
+        return 0;
+    const uint64_t off = static_cast<uint64_t>(offset - fstart);
+    if(off >= ctx->file_size)
+        return 0;
+    const uint64_t avail = ctx->file_size - off;
+    return size < avail ? size : avail;
+}
+} // namespace
 
 std::tuple<bool, uint32_t, uint32_t> LoadCommand_LC_DYSYMTAB::GetDataRange()
 {
@@ -33,8 +47,9 @@ std::tuple<bool, uint32_t, uint32_t> LoadCommand_LC_DYSYMTAB::GetDataRange()
 
 void LoadCommand_LC_DYSYMTAB::ForEachIndirectSymbols(std::function<void(uint32_t* indirect_index)> callback){
     char * offset = (char*)header_->header_start() + cmd_->indirectsymoff;
-    uint32_t size = cmd_->nindirectsyms * sizeof(uint32_t);
-    auto array = util::ParseDataAsSize(offset,size,sizeof(uint32_t));
+    uint64_t size = static_cast<uint64_t>(cmd_->nindirectsyms) * sizeof(uint32_t);
+    size = ClampSizeToFile(ctx(), offset, size);
+    auto array = util::ParseDataAsSize(offset,(uint32_t)size,sizeof(uint32_t));
     for(char *cur : array){
         callback((uint32_t*)(void*)cur);
     }
@@ -45,8 +60,10 @@ void LoadCommand_LC_DYSYMTAB::ForEachExternalRelocations(std::function<void(char
 
     char *offset = (char*)header_->header_start() + cmd_->extreloff;
     const uint32_t entry_size = sizeof(struct qv_relocation_info);
-    for(uint32_t i = 0; i < cmd_->nextrel; ++i){
-        callback(offset + i * entry_size, i);
+    const uint64_t avail = ClampSizeToFile(ctx(), offset, static_cast<uint64_t>(cmd_->nextrel) * entry_size);
+    const uint64_t count = avail / entry_size;
+    for(uint64_t i = 0; i < count; ++i){
+        callback(offset + i * entry_size, (uint32_t)i);
     }
 }
 
@@ -55,8 +72,10 @@ void LoadCommand_LC_DYSYMTAB::ForEachLocalRelocations(std::function<void(char *e
 
     char *offset = (char*)header_->header_start() + cmd_->locreloff;
     const uint32_t entry_size = sizeof(struct qv_relocation_info);
-    for(uint32_t i = 0; i < cmd_->nlocrel; ++i){
-        callback(offset + i * entry_size, i);
+    const uint64_t avail = ClampSizeToFile(ctx(), offset, static_cast<uint64_t>(cmd_->nlocrel) * entry_size);
+    const uint64_t count = avail / entry_size;
+    for(uint64_t i = 0; i < count; ++i){
+        callback(offset + i * entry_size, (uint32_t)i);
     }
 }
 

src/libmoex/node/loadcmd/LoadCommand_LINKEDIT_DATA.cpp

@@ -25,14 +25,30 @@ std::vector<DataInCodeEntryPtr> &LoadCommand_LC_DATA_IN_CODE::GetDices(){
     }
 
     char* cur_offset = (char*)this->header_->header_start() + cmd_->dataoff;
-    uint32_t cur_size = 0;
-    while(cur_size < cmd_->datasize){
+    uint64_t datasize = cmd_->datasize;
+    // Clamp the table to the mapped file so a truncated/crafted datasize cannot
+    // create entries that point past the end of the file.
+    auto fc = this->ctx();
+    if(fc && fc->file_start != nullptr){
+        const char* fstart = static_cast<const char*>(fc->file_start);
+        const char* fend = fstart + fc->file_size;
+        if(cur_offset < fstart || cur_offset > fend){
+            return dices_;
+        }
+        const uint64_t avail = static_cast<uint64_t>(fend - cur_offset);
+        if(datasize > avail) datasize = avail;
+    }
+
+    uint64_t cur_size = 0;
+    while(true){
         DataInCodeEntryPtr entry = std::make_shared<DataInCodeEntry>();
+        const uint64_t esz = entry->DATA_SIZE();
+        if(esz == 0 || cur_size + esz > datasize) break;
         entry->Init(cur_offset,ctx_);
         dices_.push_back(entry);
 
-        cur_offset += entry->DATA_SIZE();
-        cur_size += entry->DATA_SIZE();
+        cur_offset += esz;
+        cur_size += esz;
     }
 
     return dices_;
@@ -45,6 +61,17 @@ std::vector<Uleb128Data> &LoadCommand_LC_FUNCTION_STARTS::GetFunctions(){
 
     const char* start = (char*)this->header_->header_start() + cmd_->dataoff;
     const char* end = start + cmd_->datasize;
+    // Clamp the data range to the mapped file: a truncated/crafted command can
+    // claim a datasize that runs past the end of the file.
+    auto fctx = this->ctx();
+    if(fctx && fctx->file_start != nullptr){
+        const char* fstart = static_cast<const char*>(fctx->file_start);
+        const char* fend = fstart + fctx->file_size;
+        if(start < fstart || start > fend){
+            return functions_;
+        }
+        if(end > fend) end = fend;
+    }
     const char* cur_offset = start;
     while(cur_offset < end){
         Uleb128Data data;

src/libmoex/node/loadcmd/LoadCommand_SEGMENT.cpp

@@ -9,7 +9,13 @@ MOEX_NAMESPACE_BEGIN
 void LoadCommand_LC_SEGMENT::Init(void * offset,NodeContextPtr & ctx){
     LoadCommandImpl::Init(offset,ctx);
 
-    for(uint32_t idx = 0; idx < cmd_->nsects; ++idx){
+    // Section structs live inside this load command; bound nsects to what
+    // cmdsize (already validated against the file) can actually hold so a
+    // crafted nsects cannot create sections that point past the command/file.
+    uint64_t avail = cmd_->cmdsize > data_size_cmd ? (cmd_->cmdsize - data_size_cmd) : 0;
+    uint64_t count = avail / sizeof(qv_section);
+    if(count > cmd_->nsects) count = cmd_->nsects;
+    for(uint64_t idx = 0; idx < count; ++idx){
         qv_section * cur = reinterpret_cast<qv_section*>((char*)offset_ + data_size_cmd + idx * sizeof(qv_section));
 
         MachSectionPtr section = std::make_shared<MachSection>();
@@ -34,7 +40,11 @@ std::vector<std::tuple<qv_vm_prot_t,std::string>> LoadCommand_LC_SEGMENT::GetIni
 void LoadCommand_LC_SEGMENT_64::Init(void * offset,NodeContextPtr & ctx){
     LoadCommandImpl::Init(offset,ctx);
 
-    for(uint32_t idx = 0; idx < cmd_->nsects; ++idx){
+    // See LoadCommand_LC_SEGMENT::Init: bound nsects to what cmdsize can hold.
+    uint64_t avail = cmd_->cmdsize > data_size_cmd ? (cmd_->cmdsize - data_size_cmd) : 0;
+    uint64_t count = avail / sizeof(qv_section_64);
+    if(count > cmd_->nsects) count = cmd_->nsects;
+    for(uint64_t idx = 0; idx < count; ++idx){
         qv_section_64 * cur = reinterpret_cast<qv_section_64*>((char*)offset_ + data_size_cmd + idx * sizeof(qv_section_64));
         MachSectionPtr section = std::make_shared<MachSection>();
         section->set_header(header_);

src/libmoex/node/loadcmd/LoadCommand_SYMTAB.cpp

@@ -10,15 +10,35 @@ void LoadCommand_LC_SYMTAB::LazyInit(){
 
     // symbol list
     char * symbol_offset = (char*)GetSymbolTableOffsetAddress();
-    if(header_->Is64()){
-        for(uint32_t idx = 0; idx < GetSymbolTableSize(); ++idx){
+    const bool is64 = header_->Is64();
+    const uint64_t entry_size = is64 ? sizeof(struct qv_nlist_64) : sizeof(struct qv_nlist);
+
+    // Bound the entry count to what actually fits in the mapped file: a
+    // truncated or crafted symtab can claim more nlist entries than exist, and
+    // NList holds raw pointers into the file, so reading past the end crashes.
+    uint64_t count = GetSymbolTableSize();
+    if(ctx_ && ctx_->file_start != nullptr){
+        const char *fstart = static_cast<const char*>(ctx_->file_start);
+        if(symbol_offset < fstart){
+            count = 0;
+        }else{
+            const uint64_t off_in_file = static_cast<uint64_t>(symbol_offset - fstart);
+            const uint64_t avail = off_in_file <= ctx_->file_size
+                                       ? (ctx_->file_size - off_in_file) / entry_size
+                                       : 0;
+            if(count > avail) count = avail;
+        }
+    }
+
+    if(is64){
+        for(uint64_t idx = 0; idx < count; ++idx){
             struct qv_nlist_64 * cur = reinterpret_cast<struct qv_nlist_64*>(symbol_offset + idx * sizeof(struct qv_nlist_64));
             NListPtr symbol = std::make_shared<NList>();
             symbol->Init(cur,ctx_,true);
             nlists_.push_back(symbol);
         }
     }else{
-        for(uint32_t idx = 0; idx < GetSymbolTableSize(); ++idx) {
+        for(uint64_t idx = 0; idx < count; ++idx) {
             struct qv_nlist *cur = reinterpret_cast<struct qv_nlist *>(symbol_offset + idx * sizeof(struct qv_nlist));
             NListPtr symbol = std::make_shared<NList>();
             symbol->Init(cur, ctx_,false);

src/libmoex/viewnode/ViewNodeDumper.cpp

@@ -11,6 +11,11 @@ namespace {
 
 using Json = nlohmann::json;
 
+// Hard cap on view-tree recursion depth. A real Mach-O view tree is only a
+// handful of levels deep; this guards against stack overflow if malformed input
+// ever yields a pathologically deep or cyclic node tree.
+static const size_t kMaxDumpDepth = 256;
+
 static std::vector<ViewNode *> CollectChildren(ViewNode *node) {
     std::vector<ViewNode *> children;
     node->ForEachChild([&](ViewNode *child) {
@@ -278,6 +283,9 @@ static void DumpNodeText(ViewNode *node,
     if (options.max_depth > 0 && depth >= options.max_depth) {
         return;
     }
+    if (depth >= kMaxDumpDepth) {
+        return;
+    }
 
     auto children = CollectChildren(node);
     for (auto *child : children) {
@@ -312,7 +320,7 @@ static Json NodeToJson(ViewNode *node,
     }
 
     j["children"] = Json::array();
-    if (options.max_depth == 0 || depth < options.max_depth) {
+    if ((options.max_depth == 0 || depth < options.max_depth) && depth < kMaxDumpDepth) {
         auto children = CollectChildren(node);
         for (size_t i = 0; i < children.size(); ++i) {
             auto *child = children[i];
@@ -360,6 +368,9 @@ static void AccumulateSummary(ViewNode *node,
     if (options.max_depth > 0 && depth >= options.max_depth) {
         return;
     }
+    if (depth >= kMaxDumpDepth) {
+        return;
+    }
 
     auto children = CollectChildren(node);
     for (auto *child : children) {

src/libmoex/viewnode/views/DynamicLoaderInfoViewNode.cpp

@@ -490,7 +490,7 @@ void RebaseInfoViewNode::InitViewDatas()
                     break;
                 }
                 case REBASE_OPCODE_DO_REBASE_ADD_ADDR_ULEB:{
-                    auto code = static_cast<moex::Wrap_REBASE_OPCODE_ADD_ADDR_ULEB*>(codebase);
+                    auto code = static_cast<moex::Wrap_REBASE_OPCODE_DO_REBASE_ADD_ADDR_ULEB*>(codebase);
 
                     print_->AddRow({ToHexString(info->header()->GetRAW(ctx->pbyte)),ToHexString((int)ctx->byte),
                                     "REBASE_OPCODE_DO_REBASE_ADD_ADDR_ULEB",

src/libmoex/viewnode/views/SectionViewNode.h

@@ -26,8 +26,24 @@ class SectionViewChildNode : public ViewNode{
         return offset;
     }
     uint32_t GetSize(){
-        uint32_t size = (uint32_t)d_->sect().size_both();
-        return size;
+        uint64_t size = d_->sect().size_both();
+        // Clamp the section data size to what is actually present in the mapped
+        // file: a truncated or crafted section can claim more bytes than exist,
+        // and the data readers below would otherwise run off the end.
+        auto ctx = d_->ctx();
+        if(ctx && ctx->file_start != nullptr){
+            const char *off = GetOffset();
+            const char *fstart = static_cast<const char*>(ctx->file_start);
+            if(off < fstart)
+                return 0;
+            const uint64_t off_in_file = static_cast<uint64_t>(off - fstart);
+            if(off_in_file >= ctx->file_size)
+                return 0;
+            const uint64_t avail = ctx->file_size - off_in_file;
+            if(size > avail)
+                size = avail;
+        }
+        return (uint32_t)size;
     }
 
     void InitViewDatas()override {
@@ -56,8 +72,24 @@ class SectionViewNode : public ViewNode{
         return offset;
     }
     uint32_t GetSize(){
-        uint32_t size = (uint32_t)d_->sect().size_both();
-        return size;
+        uint64_t size = d_->sect().size_both();
+        // Clamp the section data size to what is actually present in the mapped
+        // file: a truncated or crafted section can claim more bytes than exist,
+        // and the data readers below would otherwise run off the end.
+        auto ctx = d_->ctx();
+        if(ctx && ctx->file_start != nullptr){
+            const char *off = GetOffset();
+            const char *fstart = static_cast<const char*>(ctx->file_start);
+            if(off < fstart)
+                return 0;
+            const uint64_t off_in_file = static_cast<uint64_t>(off - fstart);
+            if(off_in_file >= ctx->file_size)
+                return 0;
+            const uint64_t avail = ctx->file_size - off_in_file;
+            if(size > avail)
+                size = avail;
+        }
+        return (uint32_t)size;
     }
 
     void InitViewDatas()override;