Mach-O x86_64 relocations are dysfunctional

[VisualEhrmanntraut]

Version and Platform (required):

• Binary Ninja Version: 3.6.4751-dev
• OS: macOS
• OS Version: 14.2.1
• CPU Architecture: M1

Bug Description: Opening a mach-o with data sections containing addresses of external symbols are simply null, instead of containing the address of the symbol as imported and shown in the .extern section of the mach-o view.

Steps To Reproduce: Please provide all steps required to reproduce the behavior:

1. Open a mach-o executable, i.e. a macOS kext.
2. Look at a place in the data section that references external symbols, i.e the virtual function table of an IOService derivative.
3. Observe how the referenced symbols are just null bytes, as seen in the raw file itself.

Expected Behavior: The addresses be populated, instead of being null.

Screenshots:

Additional Information: This may be due to lack of handling of the __LINKEDIT segment, but I am uncertain.

[VisualEhrmanntraut]

Some more information: Did not encounter such issue with ARM64e kexts yet, but only x86_64 ones.

[VisualEhrmanntraut]

After further investigation, it appears no relocations actually work for x86_64 mach-o binaries. Example:

Relocation type X86_64_RELOC_BRANCH.

Navigating to 0x7139 brings me to a CALL instruction's value,

The address "1051db4b9" of the relocation as calculated and set in https://github.com/Vector35/view-macho/blob/main/machoview.cpp#L1052 is incorrect.

[plafosse]

This is the code in question that handles that relocation type https://github.com/Vector35/arch-x86/blob/5948b33cf7aa16733997d81ff76cdf6ec5eabdf4/arch_x86.cpp#L4167

[VisualEhrmanntraut]

To be more precise, the problem here is that it calculates that the relocation is at 0x1051db4b9, but in the view it's at 0x7139, so the relocation doesn't actually happen.

[VisualEhrmanntraut]

@plafosse

diff --git a/machoview.cpp b/machoview.cpp
index 7eaacc8..ec5892b 100644
--- a/machoview.cpp
+++ b/machoview.cpp
@@ -270,6 +270,7 @@ MachoView::MachoView(const string& typeName, BinaryView* data, bool parseOnly):
 MachOHeader MachoView::HeaderForAddress(BinaryView* data, uint64_t address, bool isMainHeader, std::string identifierPrefix)
 {
        MachOHeader header;
+       memset(&header, 0, sizeof(MachOHeader));
        header.isMainHeader = isMainHeader;

        header.identifierPrefix = identifierPrefix;

This seems to resolve the problems with the relocations of the instructions, but data references seem to remain broken.

[VisualEhrmanntraut]

[VisualEhrmanntraut]

and the problem with the data sections is here: https://github.com/Vector35/arch-x86/blob/5948b33cf7aa16733997d81ff76cdf6ec5eabdf4/arch_x86.cpp#L4189-L4200 Those places are ones with r_extern: 1 and r_type: 0

[VisualEhrmanntraut]

diff --git a/arch_x86.cpp b/arch_x86.cpp
index e19fbef..191a7f4 100644
--- a/arch_x86.cpp
+++ b/arch_x86.cpp
@@ -4187,15 +4187,15 @@ public:
                        dest32[0] = dest32[0] + target - (uint32_t)pcRelAddr;
                        break;
                case X86_64_RELOC_UNSIGNED:
-                       if (!info.external)
-                       {
+                       // if (!info.external)
+                       // {
                                switch (info.size)
                                {
                                        case 4: *dest32 += target - (uint32_t)pcRelAddr; break;
                                        case 8: *dest64 += info.target - pcRelAddr; break;
                                        default: break;
                                }
-                       }
+                       // }
                        // TODO rebasing
                        break;
                case X86_64_RELOC_SUBTRACTOR:

lol...

[VisualEhrmanntraut]

And actually, this latter issue is seemingly also on ARM64. ARM64_RELOC_UNSIGNED is not handled, so guessing that's probably it.

[VisualEhrmanntraut]

Will be fixed by Vector35/view-macho#7 and Vector35/arch-x86#37

[VisualEhrmanntraut]

(By the way, unrelated but, nice product. Though it has its issues here and there, like this one. Couldn't wait and looked into this myself. I don't actually own Binja, can't afford it right now, sorry. Was actually using Ghidra before, it's gotten so unstable lately. Binary Ninja works very well. I'm also writing a C++ class analysis plug-in, to auto create VTables and such. Might also PR a fix for the arm64 variant of this relocation issue at some point, if you guys don't get to it. :-))

[galenbwill]

Fixed in 4.1.4941-dev