[MachO] Prevent heap overflow when load command extends past EOF

[JDevlieghere]

Summary

This patch fixes a heap-buffer-overflow when a malformed Mach-O has a
load command who's size extends past the end of the binary.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3225

[kcc]

Looks good (but I don't know this code).
Thanks for adding the test input -- once the shallow bugs are cleaned up I'll use test/Object/Inputs/ as the seed corpus.

I've run the fuzzer manually and the most frequent failure looks like this:

LLVM ERROR: Invalid data was encountered while parsing the file
 #7 0x560c25 in llvm::object::RelocVisitor::getELFAddend(llvm::object::RelocationRef) Object/RelocVisitor.h:120:7

(and a few similar ones)

Are these something you could fix as well?
W/o fixing these, fuzzing won't go deep (as it crashes almost instantly)

[JDevlieghere]

In D37439#860575, @kcc wrote:

Looks good (but I don't know this code).
Thanks for adding the test input -- once the shallow bugs are cleaned up I'll use test/Object/Inputs/ as the seed corpus.

Thanks Kostya!

I've run the fuzzer manually and the most frequent failure looks like this:

LLVM ERROR: Invalid data was encountered while parsing the file
 #7 0x560c25 in llvm::object::RelocVisitor::getELFAddend(llvm::object::RelocationRef) Object/RelocVisitor.h:120:7

(and a few similar ones)

Are these something you could fix as well?
W/o fixing these, fuzzing won't go deep (as it crashes almost instantly)

I'll have a look if I can find some spare time, but I'll be focussing on the Mach-O stuff mostly.

[JDevlieghere]

ping

[aprantl]

lib/Object/MachOObjectFile.cpp

186

What happens on a 32-bit platform when cmdsize is so large that the addition wraps around? Or is cmdsize < 32bit ?

[aprantl]

Okay, thanks for the explanation, Kevin!

[kcc]

Thank you very much for addressing this!
Oss-fuzz has just confirmed the bug is fixed.

lib/Object/MachOObjectFile.cpp

static uint32_t getSectionFlags(const MachOObjectFile &O,
   MachO::section Sect = O.getSection(Sec);
   return Sect.flags;
 }
 
 static Expected<MachOObjectFile::LoadCommandInfo>
 getLoadCommandInfo(const MachOObjectFile &Obj, const char *Ptr,
                    uint32_t LoadCommandIndex) {
   if (auto CmdOrErr = getStructOrErr<MachO::load_command>(Obj, Ptr)) {
+    if (CmdOrErr->cmdsize + Ptr > Obj.getData().end())
+      return malformedError("load command " + Twine(LoadCommandIndex) +
+                            " extends past end of file");
     if (CmdOrErr->cmdsize < 8)
       return malformedError("load command " + Twine(LoadCommandIndex) +
                             " with size less than 8 bytes");
     return MachOObjectFile::LoadCommandInfo({Ptr, *CmdOrErr});
   } else
     return CmdOrErr.takeError();
 }
 
static Error checkVersCommand(const MachOObjectFile &Obj,
   return Error::success();
 }
 
 static Error checkNoteCommand(const MachOObjectFile &Obj,
                               const MachOObjectFile::LoadCommandInfo &Load,
                               uint32_t LoadCommandIndex,
                               std::list<MachOElement> &Elements) {
   if (Load.C.cmdsize != sizeof(MachO::note_command))
-    return malformedError("load command " + Twine(LoadCommandIndex) + 
+    return malformedError("load command " + Twine(LoadCommandIndex) +
                           " LC_NOTE has incorrect cmdsize");
   MachO::note_command Nt = getStruct<MachO::note_command>(Obj, Load.Ptr);
   uint64_t FileSize = Obj.getData().size();
   if (Nt.offset > FileSize)
     return malformedError("offset field of LC_NOTE command " +
                           Twine(LoadCommandIndex) + " extends "
                           "past the end of the file");
   uint64_t BigSize = Nt.offset;

test/Object/macho-invalid.test

 INVALID-DYLIB-ID-MORE-THAN-ONE: macho-invalid-dylib-id-more-than-one': truncated or malformed object (more than one LC_ID_DYLIB command)
 
 RUN: not llvm-objdump -macho -private-headers %p/Inputs/macho-invalid-dylib-wrong-filetype  2>&1 | FileCheck -check-prefix INVALID-DYLIB-WRONG-FILETYPE %s
 INVALID-DYLIB-WRONG-FILETYPE: macho-invalid-dylib-wrong-filetype': truncated or malformed object (LC_ID_DYLIB load command in non-dynamic library file type)
 
 RUN: not llvm-objdump -macho -private-headers %p/Inputs/macho-invalid-dylib-no-id  2>&1 | FileCheck -check-prefix INVALID-DYLIB-NO-ID %s
 INVALID-DYLIB-NO-ID: macho-invalid-dylib-no-id': truncated or malformed object (no LC_ID_DYLIB load command in dynamic library filetype)
 
+RUN: not llvm-objdump -macho -private-headers %p/Inputs/macho-invalid-dylib-cmdsize-past-eof 2>&1 | FileCheck -check-prefix INVALID-DYLIB-CMDSIZE %s
+INVALID-DYLIB-CMDSIZE: macho-invalid-dylib-cmdsize-past-eof': truncated or malformed object (load command 0 extends past end of file)
+
 RUN: not llvm-objdump -macho -private-headers %p/Inputs/macho-invalid-uuid-more-than-one  2>&1 | FileCheck -check-prefix INVALID-UUID-MORE-THAN-ONE %s
 INVALID-UUID-MORE-THAN-ONE: macho-invalid-uuid-more-than-one': truncated or malformed object (more than one LC_UUID command)
 
 RUN: not llvm-objdump -macho -private-headers %p/Inputs/macho-invalid-uuid-bad-size  2>&1 | FileCheck -check-prefix INVALID-UUID-BAD-SIZE %s
 INVALID-UUID-BAD-SIZE: macho-invalid-uuid-bad-size': truncated or malformed object (LC_UUID command 0 has incorrect cmdsize)
 
 RUN: not llvm-objdump -macho -private-headers %p/Inputs/macho-invalid-function_starts-dataoff  2>&1 | FileCheck -check-prefix INVALID-FUNCTION_STARTS-DATAOFF %s
 INVALID-FUNCTION_STARTS-DATAOFF: macho-invalid-function_starts-dataoff': truncated or malformed object (dataoff field of LC_FUNCTION_STARTS command 0 extends past the end of the file)