mach: Some loaded dylibs type aren't handled by parse()

[codido]

load_command::CommandVariant::LoadWeakDylib isn't handled, which may result in a panic when retrieving imports.

[m4b]

Hi @codido! Thanks for the report.

So if the parser ever panics, it’s a serious bug. Can you provide backtrace and perhaps either name of file you tried to parse (eg like a lib in /use/lib or something)

Thanks !

[codido]

Thanks for the prompt response, @m4b!
To reproduce this, you can simply build a file with libSystem set as a weak library:

cat >test.c <<EOF
#include <stdio.h>

int main()
{
        printf("Hello World!\n");
        return 0;
}
EOF

gcc -Wl,-weak_library,/usr/lib/libSystem.B.dylib test.c -o test

Running examples/dyldinfo on the resulting test would result in:

$ RUST_BACKTRACE=1 ./target/debug/examples/dyldinfo -bind /tmp/test
thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 1', src/mach/imports.rs:97:20
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:68
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:57
             at /checkout/src/libstd/panicking.rs:381
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:397
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:577
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:538
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:522
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:498
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:71
   9: core::panicking::panic_bounds_check
             at /checkout/src/libcore/panicking.rs:58
  10: goblin::mach::imports::Import::new
             at src/mach/imports.rs:97
  11: goblin::mach::imports::BindInterpreter::run
             at src/mach/imports.rs:219
  12: goblin::mach::imports::BindInterpreter::imports
             at src/mach/imports.rs:145
  13: goblin::mach::MachO::imports
             at src/mach/mod.rs:131
  14: dyldinfo::main
             at examples/dyldinfo.rs:153
  15: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:101
  16: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:459
             at /checkout/src/libstd/panic.rs:365
             at /checkout/src/libstd/rt.rs:58
  17: main
  18: __libc_start_main
  19: _start

Simply handling LoadWeakDylib similar to other dylib type in parse() seems to resolve this. There might be other missing types, not sure.

Thanks!

[m4b]

@codido Sorry for the delay; yes this was a simple oversight :/

Should be fixed now, but I haven't tested just yet, will probably be able to confirm in a couple hours, or if you see its fixed, feel free to close

[codido]

@m4b Thanks for the fix!
Unable to test it right now, but IIRC that's the fix I used locally.

That being said, shouldn't all *Dylib be handled similarly?

[m4b]

@codido

I think they're all handled now except prebound dylibs, which can be fixed via:

                load_command::CommandVariant::PreboundDylib(command) => {
                    let lib = bytes.pread::<&str>(cmd.offset + command.name as usize)?;
                    libs.push(lib);
                },

Except I don't know if this is correct.

I believe prebound dylibs are added by statically linking the lib, in which case the imports cannot reference this dylib, since they've been statically linked, and hence not needed by the dynamic linker.

I need to do some research, unless you know offhand what the answer is here? I can't even recall if I've seen a mach-o binary with a prebound library...

[willglynn]

I don't know much, but I can say that dyld doesn't handle LC_PREBOUND_DYLIB in any way, and that none of the dyld test cases (which cover the vast majority of Mach-O features) seem to produce binaries containing it.

[m4b]

Ok then I don’t think we need to add it to imports vector; otherwise will likely cause off by one errors for imports in a binary with prebound.

Thanks @willglynn for the info!

[codido]

Looks like prebinding is an obsolete mechanism (from ld(1)):

-prebind    The created output file will be in the prebound format.  This was used in Mac OS X 10.3 and earlier to improve launch performance.

I guess only old binaries will include this command.

By the way, while the patch above will fix valid binaries, goblin would still panic for invalid ones (e.g. with out of bounds ordinals or segment indices), no?

[willglynn]

Ahh, this takes me back. And yes, everything about this is obsoleted by the dyld shared cache, 10.5 (2007) and forward. Best reference is old-and-broken binutils where it actually does the prebinding, and in particular the function which updates the load commands.

If somebody has a backup from 14 years ago (everything was prebound in <= 10.3), it ought to contain executables with this header.

[luser]

If you're really motivated you can download old software update packages from Apple manually. Here's the download for the 10.3.7 Combined Update, for example:
https://support.apple.com/kb/DL543?viewlocale=en_US&locale=en_US

It's sort of a pain to unpack their update files (they're a bunch of nested archives in really obscure formats) but I have scripts that can do it. Most of the useful bits are in here:
https://github.com/luser/breakpad-mac-update-symbols/blob/master/PackageSymbolDumper.py

(we use these scripts to download and unpack macOS software updates to extract symbols for system libraries for Firefox crash reporting.)

[willglynn]

@luser Prebinding is a thing that happens on the end user's machine, much like prelink on Linux. Prebinding doesn't happen until the binaries are extracted onto the user's <= 10.3 machine, after which point Mac installers would do this:

The "optimized" binaries are specific to the user's machine, and I think it's unlikely that we'll find examples in Apple downloads.

[codido]

FWIW, it seems one of the samples in https://objective-see.com/malware.html (OpinionSpy (Premier Opinion)) contains a prebound load command in the ppc's mach-o of JavaApplicationStub.

Just don't run it :)

[m4b]

Ok, I think this was sufficiently resolved? Closing, please re-open if I've misunderstood