Validate chained fixup image formats

[aprantl]

Summary

This is part of a series of patches to upstream support for Mach-O chained fixups.

[keith]

llvm/lib/Object/MachOObjectFile.cpp

4826–4829

Is this returning Targets in both cases intentional? Maybe it's a placeholder for more upcoming logic?

[aprantl]

llvm/lib/Object/MachOObjectFile.cpp

4826–4829

It's a placeholder.

llvm/include/llvm/BinaryFormat/MachO.h

struct dyld_chained_fixups_header {
   uint32_t starts_offset;  ///< Offset of dyld_chained_starts_in_image.
   uint32_t imports_offset; ///< Offset of imports table in chain_data.
   uint32_t symbols_offset; ///< Offset of symbol strings in chain_data.
   uint32_t imports_count;  ///< Number of imported symbol names.
   uint32_t imports_format; ///< DYLD_CHAINED_IMPORT*
   uint32_t symbols_format; ///< 0 => uncompressed, 1 => zlib compressed
 };
 
+/// dyld_chained_starts_in_image is embedded in LC_DYLD_CHAINED_FIXUPS payload.
+/// Each each seg_info_offset entry is the offset into this struct for that
+/// segment followed by pool of dyld_chain_starts_in_segment data.
+struct dyld_chained_starts_in_image {
+  uint32_t    seg_count;
+  uint32_t    seg_info_offset[1];
+};
+  
 // Byte order swapping functions for MachO structs
 
 inline void swapStruct(fat_header &mh) {
   sys::swapByteOrder(mh.magic);
   sys::swapByteOrder(mh.nfat_arch);
 }
 
 inline void swapStruct(fat_arch &mh) {

llvm/include/llvm/Object/MachO.h

public:
   MachO::symtab_command getSymtabLoadCommand() const;
   MachO::dysymtab_command getDysymtabLoadCommand() const;
   MachO::linkedit_data_command getDataInCodeLoadCommand() const;
   MachO::linkedit_data_command getLinkOptHintsLoadCommand() const;
   ArrayRef<uint8_t> getDyldInfoRebaseOpcodes() const;
   ArrayRef<uint8_t> getDyldInfoBindOpcodes() const;
   ArrayRef<uint8_t> getDyldInfoWeakBindOpcodes() const;
   ArrayRef<uint8_t> getDyldInfoLazyBindOpcodes() const;
+  /// If the optional is None, no header was found, but the object was well-formed.
+  Expected<Optional<MachO::dyld_chained_fixups_header>>
+  getChainedFixupsHeader() const;
   Expected<std::vector<ChainedFixupTarget>> getDyldChainedFixupTargets() const;
   ArrayRef<uint8_t> getDyldInfoExportsTrie() const;
   SmallVector<uint64_t> getFunctionStarts() const;
   ArrayRef<uint8_t> getUuid() const;
 
   StringRef getStringTableData() const;
   bool is64Bit() const;
   void ReadULEB128s(uint64_t Index, SmallVectorImpl<uint64_t> &Out) const;

llvm/lib/Object/MachOObjectFile.cpp

 
 void MachOAbstractFixupEntry::moveToEnd() { Done = true; }
 
 MachOChainedFixupEntry::MachOChainedFixupEntry(Error *E,
                                                const MachOObjectFile *O,
                                                bool Parse)
     : MachOAbstractFixupEntry(E, O) {
   ErrorAsOutParameter e(E);
-  if (Parse) {
-    if (auto FixupTargetsOrErr = O->getDyldChainedFixupTargets())
-      FixupTargets = *FixupTargetsOrErr;
-    else {
-      *E = FixupTargetsOrErr.takeError();
-      return;
-    }
-  }
-}
+  if (!Parse)
+    return;
 
 void MachOChainedFixupEntry::moveToFirst() {
   MachOAbstractFixupEntry::moveToFirst();
   FixupIndex = 0;
   moveNext();
 }
 
 void MachOChainedFixupEntry::moveToEnd() {
ArrayRef<uint8_t> MachOObjectFile::getDyldInfoLazyBindOpcodes() const {
   if (!DyldInfoOrErr)
     return None;
   MachO::dyld_info_command DyldInfo = DyldInfoOrErr.get();
   const uint8_t *Ptr =
       reinterpret_cast<const uint8_t *>(getPtr(*this, DyldInfo.lazy_bind_off));
   return makeArrayRef(Ptr, DyldInfo.lazy_bind_size);
 }
 
-Expected<std::vector<ChainedFixupTarget>>
-MachOObjectFile::getDyldChainedFixupTargets() const {
+Expected<Optional<MachO::dyld_chained_fixups_header>>
+MachOObjectFile::getChainedFixupsHeader() const {
   // Load the dyld chained fixups load command.
   if (!DyldChainedFixupsLoadCmd)
-    return std::vector<ChainedFixupTarget>();
+    return llvm::None;
   auto DyldChainedFixupsOrErr = getStructOrErr<MachO::linkedit_data_command>(
       *this, DyldChainedFixupsLoadCmd);
   if (!DyldChainedFixupsOrErr)
     return DyldChainedFixupsOrErr.takeError();
   MachO::linkedit_data_command DyldChainedFixups = DyldChainedFixupsOrErr.get();
 
   // If the load command is present but the data offset has been zeroed out,
-  // as is the case for dylib stubs, return an empty list of targets.
+  // as is the case for dylib stubs, return None (no error).
   uint64_t CFHeaderOffset = DyldChainedFixups.dataoff;
-  std::vector<ChainedFixupTarget> Targets;
   if (CFHeaderOffset == 0)
-    return Targets;
+    return DyldChainedFixupsOrErr.takeError();
 
   // Load the dyld chained fixups header.
   const char *CFHeaderPtr = getPtr(*this, CFHeaderOffset);
   auto CFHeaderOrErr =
       getStructOrErr<MachO::dyld_chained_fixups_header>(*this, CFHeaderPtr);
   if (!CFHeaderOrErr)
     return CFHeaderOrErr.takeError();
   MachO::dyld_chained_fixups_header CFHeader = CFHeaderOrErr.get();
 
   // Reject unknown chained fixup formats.
   if (CFHeader.fixups_version != 0)
     return malformedError(Twine("bad chained fixups: unknown version: ") +
                           Twine(CFHeader.fixups_version));
   if (CFHeader.imports_format < 1 || CFHeader.imports_format > 3)
     return malformedError(
         Twine("bad chained fixups: unknown imports format: ") +
         Twine(CFHeader.imports_format));
 
+  // Validate the image format.
+  //
+  // Load the image starts.
+  uint64_t CFImageStartsOffset = (CFHeaderOffset + CFHeader.starts_offset);
+  if (CFHeader.starts_offset < sizeof(MachO::dyld_chained_fixups_header)) {
+    return malformedError(Twine("bad chained fixups: image starts offset ") +
+                          Twine(CFHeader.starts_offset) +
+                          " overlaps with chained fixups header");
+  }
+  uint32_t EndOffset = DyldChainedFixups.dataoff + DyldChainedFixups.datasize;
+  if (CFImageStartsOffset + sizeof(MachO::dyld_chained_starts_in_image) >
+      EndOffset) {
+    return malformedError(Twine("bad chained fixups: image starts end ") +
+                          Twine(CFImageStartsOffset +
+                                sizeof(MachO::dyld_chained_starts_in_image)) +
+                          " extends past end " + Twine(EndOffset));
+  }
+
+  return CFHeader;
+}
+
+Expected<std::vector<ChainedFixupTarget>>
+MachOObjectFile::getDyldChainedFixupTargets() const {
+  auto CFHeaderOrErr = getChainedFixupsHeader();
+  if (!CFHeaderOrErr)
+    return CFHeaderOrErr.takeError();
+  std::vector<ChainedFixupTarget> Targets;
+  if (!(*CFHeaderOrErr))
+    return Targets;
   return Targets;
 }
 
 ArrayRef<uint8_t> MachOObjectFile::getDyldInfoExportsTrie() const {
   if (!DyldInfoLoadCmd)
     return None;
 
   auto DyldInfoOrErr =
     getStructOrErr<MachO::dyld_info_command>(*this, DyldInfoLoadCmd);

llvm/test/Object/AArch64/chained-fixups-header.test

 REQUIRES: host-byteorder-little-endian
 RUN: cat %p/../Inputs/MachO/chained-fixups.yaml \
 RUN:   | sed 's/__LINKEDIT:      00000000/__LINKEDIT:      AB000000/' \
 RUN:   | yaml2obj | not llvm-objdump --macho --dyld_info - 2>&1 \
 RUN:   | FileCheck %s --check-prefix=HEADER1
 HEADER1: truncated or malformed object (bad chained fixups: unknown version: 171)
 
 RUN: cat %p/../Inputs/MachO/chained-fixups.yaml \
 RUN:   | sed 's/1000000010000000/1000000AB0000000/' \
 RUN:   | yaml2obj | not llvm-objdump --macho --dyld_info - 2>&1 \
 RUN:   | FileCheck %s --check-prefix=HEADER2
 HEADER2: truncated or malformed object (bad chained fixups: unknown imports format: 171)
+
+RUN: cat %p/../Inputs/MachO/chained-fixups.yaml \
+RUN:   | sed 's/20000000/01000000/' \
+RUN:   | yaml2obj | not llvm-objdump --macho --dyld_info - 2>&1 \
+RUN:   | FileCheck %s --check-prefix=HEADER3
+HEADER3: truncated or malformed object (bad chained fixups: image starts offset 1 overlaps with chained fixups header)
+
+RUN: cat %p/../Inputs/MachO/chained-fixups.yaml \
+RUN:   | sed 's/20000000/FF000000/' \
+RUN:   | yaml2obj | not llvm-objdump --macho --dyld_info - 2>&1 \
+RUN:   | FileCheck %s --check-prefix=HEADER4
+HEADER4: truncated or malformed object (bad chained fixups: image starts end 33031 extends past end 32856)