Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong symbol resolved from LC_DYLD_INFO_ONLY on iOS MachO binary #6994

Open
TheDauntless opened this issue Oct 2, 2024 · 7 comments
Open
Assignees
Labels

Comments

@TheDauntless
Copy link

TheDauntless commented Oct 2, 2024

Describe the bug

I have a (non-public) MachO iOS file for which Ghidra identifies the wrong symbol/linked library for certain imports. Hopper and LIEF do identify the correct symbol and I've confirmed this via dynamic analysis. I can't share the binary, but I'm trying to debug Ghidra to figure out the error, so some hints as to where to continue my examination could help me in fixing the bug.

The dynamic linking info is available in LC_DYLD_INFO_ONLY and it does get processed, but not applied for some reason (see further down)

There is also a MachoProcessBindScript.java script, which seems related, but this does not fix the problem and it appears that the bind processing is already done by one of the general analyzers? So not sure if this script actually still serves a purpose?

Specifics
Ghidra is showing uname as the import at 0x1c85288 while it should be Swift.String.hasSuffix:

image

In Hopper, it's String.hasSuffix, which is correct:

image

And in LIEF it's correct too:

address=0x01c85288, addend=0x0
  symbol=_$sSS9hasSuffixySbSSF
  segment=__DATA
  library=/usr/lib/swift/libswiftCore.dylib

Additional Binary info
There are two __LINKEDIT sections:

image

(Actually not entirely sure where the second one comes from)

And inside of it there are the following commands:
image

> otool -l binary
...
Load command 2
      cmd LC_SEGMENT_64
  cmdsize 1672
  segname __DATA
   vmaddr 0x0000000001c84000
   vmsize 0x00000000005d0000
  fileoff 29900800
 filesize 655360
  maxprot 0x00000003
 initprot 0x00000003
   nsects 20
    flags 0x0
...

The second section starts at 1c84000 so add 1288h becomes 1c85288 and so hasSuffix should be set where _uname currently is set.

I've debugged the analyzer a bit and the __LINKEDIT commands are read by dyld/BindingTable.java:

image

I wasn't able to figure out yet where the binding table is further used in the flow.

Environment:

  • OS: macOS / Linux / Windows
  • Java Version: openjdk 23 2024-09-17
  • Ghidra Version: 11.2 (and master)
  • Ghidra Origin: Github release & locally built
@ryanmkurtz ryanmkurtz self-assigned this Oct 3, 2024
@ryanmkurtz ryanmkurtz added Feature: Loader/Mach-O Status: Triage Information is being gathered labels Oct 3, 2024
@ryanmkurtz
Copy link
Collaborator

Sorry for the delay in getting back to you.

There is also a MachoProcessBindScript.java script, which seems related, but this does not fix the problem and it appears that the bind processing is already done by one of the general analyzers? So not sure if this script actually still serves a purpose?

This is ancient...I'll remove it.

I wasn't able to figure out yet where the binding table is further used in the flow.

protected void processDyldInfo(boolean doClassic, List<String> libraryPaths) throws Exception {
List<DyldInfoCommand> commands = machoHeader.getLoadCommands(DyldInfoCommand.class);
for (DyldInfoCommand command : commands) {
processRebases(command.getRebaseTable());
processBindings(command.getBindingTable(), libraryPaths);
processBindings(command.getLazyBindingTable(), libraryPaths);
processBindings(command.getWeakBindingTable(), libraryPaths);
}
if (commands.size() == 0 && doClassic) {
ClassicBindProcessor classicBindProcess =
new ClassicBindProcessor(machoHeader, program);
try {
classicBindProcess.process(monitor);
}
catch (Exception e) {
log.appendException(e);
}
ClassicLazyBindProcessor classicLazyBindProcess =
new ClassicLazyBindProcessor(machoHeader, program);
try {
classicLazyBindProcess.process(monitor);
}
catch (Exception e) {
log.appendException(e);
}
}
}
private void processRebases(RebaseTable rebaseTable) throws Exception {
// If we ever support rebasing a Mach-O at load time, this should get implemented
}
private void processBindings(BindingTable bindingTable, List<String> libraryPaths)
throws Exception {
DataConverter converter = DataConverter.getInstance(program.getLanguage().isBigEndian());
SymbolTable symbolTable = program.getSymbolTable();
Address imagebase = getMachoBaseAddress();
List<Binding> bindings = bindingTable.getBindings();
List<Binding> threadedBindings = bindingTable.getThreadedBindings();
List<SegmentCommand> segments = machoHeader.getAllSegments();
if (threadedBindings != null) {
DyldChainedImports chainedImports = new DyldChainedImports(bindings);
for (Binding threadedBinding : threadedBindings) {
List<DyldFixup> fixups = DyldChainedFixups.getChainedFixups(reader,
chainedImports, DyldChainType.DYLD_CHAINED_PTR_ARM64E,
segments.get(threadedBinding.getSegmentIndex()).getFileOffset(),
threadedBinding.getSegmentOffset(), 0, imagebase.getOffset(),
symbolTable, log, monitor);
DyldChainedFixups.fixupChainedPointers(fixups, program, imagebase, libraryPaths,
log, monitor);
}
}
else {
for (Binding binding : bindings) {
if (binding.getUnknownOpcode() != null) {
log.appendMsg(
"Unknown bind opcode: 0x%x".formatted(binding.getUnknownOpcode()));
continue;
}
List<Symbol> symbols = symbolTable.getGlobalSymbols(binding.getSymbolName());
if (symbols.isEmpty()) {
continue;
}
Symbol symbol = symbols.get(0);
long offset = symbol.getAddress().getOffset();
byte[] bytes = (program.getDefaultPointerSize() == 8) ? converter.getBytes(offset)
: converter.getBytes((int) offset);
Address addr =
space.getAddress(segments.get(binding.getSegmentIndex()).getVMaddress() +
binding.getSegmentOffset());
fixupExternalLibrary(binding.getLibraryOrdinal(), symbol, libraryPaths);
boolean success = false;
try {
program.getMemory().setBytes(addr, bytes);
success = true;
}
catch (MemoryAccessException e) {
handleRelocationError(addr, String.format(
"Relocation failure at address %s: error accessing memory.", addr));
}
finally {
program.getRelocationTable()
.add(addr, success ? Status.APPLIED_OTHER : Status.FAILURE,
binding.getType(), null, bytes.length, binding.getSymbolName());
}
}
}
}

@ryanmkurtz
Copy link
Collaborator

I will try to find the same flaw in my set of samples...hopefully there is something consistently wrong.

@ryanmkurtz
Copy link
Collaborator

There are two __LINKEDIT sections:
(Actually not entirely sure where the second one comes from)

This is typically due to an older memory block convention we use that creates a 2nd block for the padded out part of the memory block, which we make as uninitialized. The PE loader has dropped this convention, but the Mach-O loader hasn't yet.

@ryanmkurtz
Copy link
Collaborator

I think i have reproduced the issue in one of my binaries. I will begin looking into a fix.

@ryanmkurtz
Copy link
Collaborator

I take it back, i am not reproducing it. Looking at your initial screen shot, it looks like uname is simply referencing 1c85288. In Ghidra, what is at address 15a1a80?

@ryanmkurtz ryanmkurtz added Status: Waiting on customer Waiting for customer feedback and removed Status: Triage Information is being gathered labels Oct 7, 2024
@TheDauntless
Copy link
Author

@ryanmkurtz
image
and
image

I put a breakpoint all over the range you indicated in MachoProgramBuilder but none of them actually get triggered. This is by the way the stack trace of that previous debugger screenshot; maybe I'm in a different code path?
image

@ryanmkurtz
Copy link
Collaborator

Any way you can share the binary with me privately? If so, making a private GitHub repo and inviting just me to it has worked in the past. i understand if you can't do that tho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants