This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by JDevlieghere on Sep 4 2017, 8:23 AM.

Details

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

Diff Detail

Repository
rL LLVM

Event Timeline

JDevlieghere created this revision.Sep 4 2017, 8:23 AM
kcc added a comment.Sep 4 2017, 2:17 PM

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)

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.

aprantl added inline comments.Sep 12 2017, 8:46 AM
lib/Object/MachOObjectFile.cpp
186 ↗(On Diff #113756)

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

aprantl accepted this revision.Sep 12 2017, 2:24 PM

Okay, thanks for the explanation, Kevin!

This revision is now accepted and ready to land.Sep 12 2017, 2:24 PM
This revision was automatically updated to reflect the committed changes.
kcc added a comment.Sep 14 2017, 4:39 PM

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