[Object, MachO] Don't crash on invalid MachO load commands.

[samsonov]

Summary

Currently all load commands are parsed in MachOObjectFile constructor.
If the next load command cannot be parsed, or if command size are too
small, properly report it through the error code and fail to construct
the object, instead of crashing the program.

[filcab]

test/Object/macho-invalid.test

4

Please commit this formatting change separately, then add the case for this patch.

6

Please commit this formatting change separately (for all the previous tests), then do the patch for this case, removing the "not".

[filcab]

(forgot to LGTM)

LGTM, as long as the formatting change gets its own commit.

[• rafael]

llvm-objdump should still exit with a non-zero status, no ?

[samsonov]

test/Object/macho-invalid.test

4

Done in r239068.

[samsonov]

r239080.

include/llvm/Object/Error.h

 
 enum class object_error {
   success = 0,
   arch_not_found,
   invalid_file_type,
   parse_failed,
   unexpected_eof,
   bitcode_section_not_found,
+  macho_small_load_command,
 };
 
 inline std::error_code make_error_code(object_error e) {
   return std::error_code(static_cast<int>(e), object_category());
 }
 
 } // end namespace object.
 
 } // end namespace llvm.
 
 namespace std {
 template <>
 struct is_error_code_enum<llvm::object::object_error> : std::true_type {};
 }
 
 #endif

include/llvm/Object/MachO.h

public:
 
   bool hasPageZeroSegment() const { return HasPageZeroSegment; }
 
   static bool classof(const Binary *v) {
     return v->isMachO();
   }
 
 private:
-  // Walk load commands.
-  LoadCommandInfo getFirstLoadCommandInfo() const;
-  LoadCommandInfo getNextLoadCommandInfo(const LoadCommandInfo &L) const;
-
   MachO::mach_header_64 Header64;
   MachO::mach_header *Header;  // Points inside Header64.
   typedef SmallVector<const char*, 1> SectionList;
   SectionList Sections;
   typedef SmallVector<const char*, 1> LibraryList;
   LibraryList Libraries;
   LoadCommandList LoadCommands;
   typedef SmallVector<StringRef, 1> LibraryShortName;

lib/Object/Error.cpp

std::string _object_error_category::message(int EV) const {
   case object_error::invalid_file_type:
     return "The file was not recognized as a valid object file";
   case object_error::parse_failed:
     return "Invalid data was encountered while parsing the file";
   case object_error::unexpected_eof:
     return "The end of the file was unexpectedly encountered";
   case object_error::bitcode_section_not_found:
     return "Bitcode section not found in object file";
+  case object_error::macho_small_load_command:
+    return "Mach-O load command with size < 8 bytes";
   }
   llvm_unreachable("An enumerator of object_error does not have a message "
                    "defined.");
 }
 
 static ManagedStatic<_object_error_category> error_category;
 
 const std::error_category &object::object_category() {
   return *error_category;
 }

lib/Object/MachOObjectFile.cpp

static void parseHeader(const MachOObjectFile *Obj, T &Header,
                         std::error_code &EC) {
   auto HeaderOrErr = getStructOrErr<T>(Obj, getPtr(Obj, 0));
   if (HeaderOrErr)
     Header = HeaderOrErr.get();
   else
     EC = HeaderOrErr.getError();
 }
 
+static ErrorOr<MachOObjectFile::LoadCommandInfo>
+getLoadCommandInfo(const MachOObjectFile *Obj, const char *Ptr) {
+  auto CmdOrErr = getStructOrErr<MachO::load_command>(Obj, Ptr);
+  if (!CmdOrErr)
+    return CmdOrErr.getError();
+  if (CmdOrErr->cmdsize < 8)
+    return object_error::macho_small_load_command;
+  MachOObjectFile::LoadCommandInfo Load;
+  Load.Ptr = Ptr;
+  Load.C = CmdOrErr.get();
+  return Load;
+}
+
+static ErrorOr<MachOObjectFile::LoadCommandInfo>
+getFirstLoadCommandInfo(const MachOObjectFile *Obj) {
+  unsigned HeaderSize = Obj->is64Bit() ? sizeof(MachO::mach_header_64)
+                                       : sizeof(MachO::mach_header);
+  return getLoadCommandInfo(Obj, getPtr(Obj, HeaderSize));
+}
+
+static ErrorOr<MachOObjectFile::LoadCommandInfo>
+getNextLoadCommandInfo(const MachOObjectFile *Obj,
+                       const MachOObjectFile::LoadCommandInfo &L) {
+  return getLoadCommandInfo(Obj, L.Ptr + L.C.cmdsize);
+}
+
 MachOObjectFile::MachOObjectFile(MemoryBufferRef Object, bool IsLittleEndian,
                                  bool Is64bits, std::error_code &EC)
     : ObjectFile(getMachOType(IsLittleEndian, Is64bits), Object),
       Header(reinterpret_cast<MachO::mach_header*>(&this->Header64)),
       SymtabLoadCmd(nullptr), DysymtabLoadCmd(nullptr),
       DataInCodeLoadCmd(nullptr), LinkOptHintsLoadCmd(nullptr),
       DyldInfoLoadCmd(nullptr), UuidLoadCmd(nullptr),
       HasPageZeroSegment(false) {
   // Parse header.
   if (is64Bit())
     parseHeader(this, Header64, EC);
   else
     parseHeader(this, *Header, EC);
   if (EC)
     return;
 
   uint32_t LoadCommandCount = Header->ncmds;
   if (LoadCommandCount == 0)
     return;
 
   MachO::LoadCommandType SegmentLoadType = is64Bit() ?
     MachO::LC_SEGMENT_64 : MachO::LC_SEGMENT;
 
-  LoadCommandInfo Load = getFirstLoadCommandInfo();
+  auto LoadOrErr = getFirstLoadCommandInfo(this);
+  if (!LoadOrErr) {
+    EC = LoadOrErr.getError();
+    return;
+  }
+  LoadCommandInfo Load = LoadOrErr.get();
   for (unsigned I = 0; I < LoadCommandCount; ++I) {
     LoadCommands.push_back(Load);
     if (Load.C.cmd == MachO::LC_SYMTAB) {
       // Multiple symbol tables
       if (SymtabLoadCmd) {
         EC = object_error::parse_failed;
         return;
       }
if (Load.C.cmd == MachO::LC_SYMTAB) {
         HasPageZeroSegment = true;
     } else if (Load.C.cmd == MachO::LC_LOAD_DYLIB ||
                Load.C.cmd == MachO::LC_LOAD_WEAK_DYLIB ||
                Load.C.cmd == MachO::LC_LAZY_LOAD_DYLIB ||
                Load.C.cmd == MachO::LC_REEXPORT_DYLIB ||
                Load.C.cmd == MachO::LC_LOAD_UPWARD_DYLIB) {
       Libraries.push_back(Load.Ptr);
     }
-    if (I < LoadCommandCount - 1)
-      Load = getNextLoadCommandInfo(Load);
+    if (I < LoadCommandCount - 1) {
+      auto LoadOrErr = getNextLoadCommandInfo(this, Load);
+      if (!LoadOrErr) {
+        EC = LoadOrErr.getError();
+        return;
+      }
+      Load = LoadOrErr.get();
+    }
   }
   assert(LoadCommands.size() == LoadCommandCount);
 }
 
 void MachOObjectFile::moveSymbolNext(DataRefImpl &Symb) const {
   unsigned SymbolTableEntrySize = is64Bit() ?
     sizeof(MachO::nlist_64) :
     sizeof(MachO::nlist);
MachOObjectFile::getAnyRelocationSection(
   if (isRelocationScattered(RE) || getPlainRelocationExternal(RE))
     return *section_end();
   unsigned SecNum = getPlainRelocationSymbolNum(RE) - 1;
   DataRefImpl DRI;
   DRI.d.a = SecNum;
   return SectionRef(DRI, this);
 }
 
-MachOObjectFile::LoadCommandInfo
-MachOObjectFile::getFirstLoadCommandInfo() const {
-  MachOObjectFile::LoadCommandInfo Load;
-
-  unsigned HeaderSize = is64Bit() ? sizeof(MachO::mach_header_64) :
-                                    sizeof(MachO::mach_header);
-  Load.Ptr = getPtr(this, HeaderSize);
-  Load.C = getStruct<MachO::load_command>(this, Load.Ptr);
-  if (Load.C.cmdsize < 8)
-    report_fatal_error("Load command with size < 8 bytes.");
-  return Load;
-}
-
-MachOObjectFile::LoadCommandInfo
-MachOObjectFile::getNextLoadCommandInfo(const LoadCommandInfo &L) const {
-  MachOObjectFile::LoadCommandInfo Next;
-  Next.Ptr = L.Ptr + L.C.cmdsize;
-  Next.C = getStruct<MachO::load_command>(this, Next.Ptr);
-  if (Next.C.cmdsize < 8)
-    report_fatal_error("Load command with size < 8 bytes.");
-  return Next;
-}
-
 MachO::section MachOObjectFile::getSection(DataRefImpl DRI) const {
   assert(DRI.d.a < Sections.size() && "Should have detected this earlier");
   return getStruct<MachO::section>(this, Sections[DRI.d.a]);
 }
 
 MachO::section_64 MachOObjectFile::getSection64(DataRefImpl DRI) const {
   assert(DRI.d.a < Sections.size() && "Should have detected this earlier");
   return getStruct<MachO::section_64>(this, Sections[DRI.d.a]);

test/Object/macho-invalid.test

 // No crash, might not be totally invalid
 RUN: llvm-objdump -private-headers %p/Inputs/macho-invalid-zero-ncmds
 
-RUN: not llvm-objdump -private-headers %p/Inputs/macho64-invalid-incomplete-load-command 2>&1 \
+RUN: llvm-objdump -private-headers %p/Inputs/macho64-invalid-incomplete-load-command 2>&1 \
 RUN:      | FileCheck -check-prefix INCOMPLETE-LOADC %s
+INCOMPLETE-LOADC: Invalid data was encountered while parsing the file.
 
-RUN: not llvm-objdump -private-headers %p/Inputs/macho-invalid-too-small-load-command 2>&1 \
+RUN: llvm-objdump -private-headers %p/Inputs/macho-invalid-too-small-load-command 2>&1 \
 RUN:      | FileCheck -check-prefix SMALL-LOADC-SIZE %s
-RUN: not llvm-objdump -private-headers %p/Inputs/macho64-invalid-too-small-load-command 2>&1 \
+RUN: llvm-objdump -private-headers %p/Inputs/macho64-invalid-too-small-load-command 2>&1 \
 RUN:      | FileCheck -check-prefix SMALL-LOADC-SIZE %s
+SMALL-LOADC-SIZE: Mach-O load command with size < 8 bytes
 
 RUN: not llvm-objdump -private-headers %p/Inputs/macho-invalid-too-small-segment-load-command 2>&1 \
 RUN:      | FileCheck -check-prefix SMALL-SEGLOADC-SIZE %s
 RUN: not llvm-objdump -private-headers %p/Inputs/macho64-invalid-too-small-segment-load-command 2>&1 \
 RUN:      | FileCheck -check-prefix SMALL-SEGLOADC-SIZE %s
+SMALL-SEGLOADC-SIZE: Segment load command size is too small
 
 RUN: not llvm-objdump -private-headers %p/Inputs/macho-invalid-no-size-for-sections 2>&1 \
 RUN:      | FileCheck -check-prefix TOO-MANY-SECTS %s
 RUN: not llvm-objdump -private-headers %p/Inputs/macho64-invalid-no-size-for-sections 2>&1 \
 RUN:      | FileCheck -check-prefix TOO-MANY-SECTS %s
+TOO-MANY-SECTS: Number of sections too large for size of load command
 
 RUN: not llvm-objdump -t %p/Inputs/macho-invalid-bad-symbol-index 2>&1 \
 RUN:      | FileCheck -check-prefix BAD-SYMBOL %s
+BAD-SYMBOL: Requested symbol index is out of range
 
 RUN: not llvm-objdump -t %p/Inputs/macho-invalid-symbol-name-past-eof 2>&1 \
 RUN:      | FileCheck -check-prefix NAME-PAST-EOF %s
+NAME-PAST-EOF: Symbol name entry points before beginning or past end of file
 
 RUN: not llvm-nm %p/Inputs/macho-invalid-section-index-getSectionRawName 2>&1 \
 RUN:      | FileCheck -check-prefix INVALID-SECTION-IDX-SYMBOL-SEC %s
+INVALID-SECTION-IDX-SYMBOL-SEC: getSymbolSection: Invalid section index
 
 RUN: llvm-objdump -private-headers %p/Inputs/macho-invalid-header 2>&1 | FileCheck -check-prefix INVALID-HEADER %s
-
-SMALL-LOADC-SIZE: Load command with size < 8 bytes
-SMALL-SEGLOADC-SIZE: Segment load command size is too small
-INCOMPLETE-LOADC: Malformed MachO file
-TOO-MANY-SECTS: Number of sections too large for size of load command
-BAD-SYMBOL: Requested symbol index is out of range
-NAME-PAST-EOF: Symbol name entry points before beginning or past end of file
-
-INVALID-SECTION-IDX-SYMBOL-SEC: getSymbolSection: Invalid section index
 INVALID-HEADER: Invalid data was encountered while parsing the file.