Page MenuHomePhabricator

Fix Mach-O bind and rebase validation errors in libObject

Authored by mtrent on Mar 19 2019, 9:50 PM.



llvm-objdump (via libObject) validates DYLD_INFO rebase and bind
entries against the basic structure found in the Mach-O file before
evaluating the contents of those entries. Certain malformed Mach-Os can
defeat the validation check and force llvm-objdump (libObject) to crash.

The previous logic verified a rebase or bind started in a valid Mach-O
section, but did not verify that the section wholely contained the
fixup. It also generally allows rebases or binds to start immediately
after a valid section even if that range is not itself part of a valid
section. Finally, bind and rebase opcodes that indicate more than one
fixup (apply N times...) are not completely validated: only the first
and final fixups are checked.

The previous logic also rejected certain binaries as false positives.
Some bind and rebase opcodes can modify the state machine such that the
next bind or rebase will fail. libObject will reject these opcodes as
invalid in order to be helpful and print an error message associated
with the instruction that caused the problem, even though the binary is
not actually illegal until it consumes the invalid state in the state
machine. In other words, libObject may reject a Mach-O binary that
Apple's dynamic linker may consider legal. The original version of
macho-rebase-add-addr-uleb-too-big is an example of such a binary.

I have replaced the existing checkSegAndOffset and checkCountAndSkip
functions with a single function, checkSegAndOffsets, which validates
all of the fixups realized by a DYLD_INFO opcode. checkSegAndOffsets
verifies that a Mach-O section fully contains each fixup. Every fixup
realized by an opcode is validated, and some (but not all!)
inconsistencies in the state machine are allowed until a fixup is
realized. This means that libObject may fail on an opcode that realizes
a fixup, not on the opcode that introduced the arithmetic error.

Existing test cases have been modified to reflect the changes in error
messages returned by libObject. What's more, the test case for
macho-rebase-add-addr-uleb-too-big has been modified so that it actually
triggers the error condition; the new code in libObject considers the
original test binary "legal".


Diff Detail


Event Timeline

mtrent created this revision.Mar 19 2019, 9:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 19 2019, 9:50 PM
pete accepted this revision.Mar 20 2019, 10:48 AM
pete added inline comments.
3993 ↗(On Diff #191430)

Can you add some ()'s? eg:

if ((SI.OffsetInSegment <= Start) && (Start < (SI.OffsetInSegment + SI.Size)))

Otherwise LGTM. And sorry if this makes 80 cols hard to achieve!

This revision is now accepted and ready to land.Mar 20 2019, 10:48 AM
mtrent marked an inline comment as done.Mar 20 2019, 3:53 PM
mtrent added inline comments.
3993 ↗(On Diff #191430)

Will do

This revision was automatically updated to reflect the committed changes.