vector35 / workflow_objc Goto Github PK

Data variable prefixes (e.g. cl_, sr_, etc.) currently have a few problems:

• They are hardcoded
• They are only two characters and some natural collisions arise
• They are not all very descriptive

The short prefixes were originally chosen to prevent bloating the name of the variable and contributing excessive width to lines in which they are used one or more times.

A couple things should be done:

• Pick better prefixes (maybe copy IDA or do something in between)
• Make prefixes constants in the code to prevent accidental collisions or de-syncs

When trying to load this file DADocSetAccess.zip I get a structure created for class_DSADocSet_FastTokenCache that has members which are named type references whose names are empty, ie:

struct class_DSADocSet_FastTokenCache
{
    tokenCache;
    tokenNames1;
    tokenNames2;
    scopes;
    tokenUSRIndex;
};

... with no names on any of the members. This causes weird problems in all sorts of places considering they are typedef ; type and not even a void pointer or something reasonable.

It would be nice to figure out why these are happening and stop them from happening, either by fixing the bug or patching around the bugged behavior making its way out of the plugin.

Is it enough to install the plugin via the instructions, or do I also need to uncheck this?

This is potentially a duplicate of #8 only in the sense that #8 is so vague it could basically be "Issue: product has bugs"

In typestrings, Pointers to types, e.g. ^@ == "id *", ^^^^c == "char ****" are incorrectly parsed.

It would be nice if the signatures of the Objective-C messages would match more closely their form in practical use/documentation, etc. This is how they look right now:

Improvements would be:

• No _ prefix in the signature
• Automatic rename of arg1 to self and arg2 to sel in the disassembly
• Removal of self and sel in the signature (these are implicit)
  • Or only sel for class selectors
• Selector arguments in-line in the selector, e.g. -[NSView addSubview:(NSView*)arg1], instead of suffixed with ()

Something changed recently in the type parser, which has caused a massive amount of error spew from the clang type parser. The errors are not fatal and do not affect analysis in any way.

Presumably, this will be fixed with #19 , however it should probably be looked into in the interim.

In the readme it may be helpful to point contributors to https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGObjCMac.cpp and other CGObjC* files to help them find the relevant code to implement features.

The workflow for rewriting objc_msgSend calls fails to rewrite tail calls, as shown here:

In instances where an objc_msgSend call has been rewritten to a direct method call, displaying the selector parameter is both not useful (since it is likely tagged, etc.) and redundant. Perhaps it should be hidden in these cases?

Calls to Objective-C methods defined outside the binary—e.g. NSString methods, etc.—currently can't be rewritten because there is no function inside the binary to replace the call destination with. Creating stub functions in a fake section for imported methods might allow for clearer decompilation.

There are numerous functions that produce a lot of noise in Objective-C code, examples include:

• _objc_retainAutoreleasedReturnValue
• _objc_retainAutorelease

These are not often the interesting bits of the function, and it might be helpful if these could be optionally hidden to produce more readable decompilation.

Class references in the objc_classrefs section should be annotated/named. Right now, they aren't.

See title; details to be added later.

Lets say there are two class. Employee class and Student class. They are not related at all but they both have function called print.
current _objc_msgSend resolver doesn't account for that and sets everyone function to just one function.

You can see below its calling a function from Student when Employee is not even related to it

I think it should only set the address when you know for sure which object it is. maybe do sink -> init source analysis to know which class it is. and only change the call when it is likely correct.

I ran a binary through Binary Ninja and it didn't name anything that was a class method. All the instance methods came through but the class methods didn't seem to get names.

This project uses different, undocumented formatting conventions than the wider binaryninja project, but working on it under the larger project inherits its clang_format, so in any IDEs that automatically enforce ClangFormat rules, working on the project becomes a headache and requires a constant regex-find-replace window open.

See title; details to be added later.

We can scan and automatically annotate objc functions to provide better disassembly

Currently working on a large "binja safe" objc.h file

This decompilation is incorrect:

The correct output can be seen here:

The first argument of objc_msgSendSuper2 is an objc_super struct containing a pointer to the self Class object and a pointer to the superclass.

The full type definitions can be seen starting here: https://github.com/apple-oss-distributions/objc4/blob/main/runtime/message.h#L34

Today parameters for objc_msgSend don't work well because its a varadiditc function. By using the number of ":" in the immediate selector as a guide we can override the type at the callsite with the correct prototype

The .gitmodules file is empty, so git submodule update --init --recursive doesn't do anything.

And building results in:

❯ cmake -S . -B build -GNinja
-- The C compiler identification is AppleClang 14.0.0.14000029
-- The CXX compiler identification is AppleClang 14.0.0.14000029
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at CMakeLists.txt:9 (add_subdirectory):
  add_subdirectory given source "Vendor/BinaryNinjaAPI" which is not an
  existing directory.


CMake Error at CMakeLists.txt:86 (bn_install_plugin):
  Unknown CMake command "bn_install_plugin".


-- Configuring incomplete, errors occurred!
See also "/Users/torarne/dev/binaryninja/workflow_objc/build/CMakeFiles/CMakeOutput.log".
See also "/Users/torarne/dev/binaryninja/workflow_objc/build/CMakeFiles/CMakeError.log".

I'm guessing there should be something in Vendor?

Objective-C method type strings can contain named types using the quote syntax, as well as structure types using curly brace syntax; see attached image. These are currently skipped over entirely in the type parser, but shouldn't be.

Inline CFSTR rendering occasionally breaks on Enterprise whenever architecture plugins load before workflow_objc

This will allow dynamically defined types, which may be required for implementing objc support for other architectures. It is also faster and more reliable.

As BinaryNinja now processes chained fixups, it's likely that the pointer detagging routine and data renderer are no longer needed.

Hi,

I have found that these https://github.com/Vector35/workflow_objc/blob/master/Core/AnalysisProvider.cpp#L22-L23 two analyzers raise out-of-bound exceptions.

Used LaunchServices binary extracted using https://github.com/keith/dyld-shared-cache-extractor from arm64 cache.

Will dig into it tomorrow and update the ticket.

A bunch of information detailing the layout of local types is contained inside of Objective-C binaries and is not currently parsed. Parsing it would be a big win.

Before doing this:

after:

Not sure if this is a workflows problem or an issue we can mitigate somehow...

This allows proper inlining of strings and other types passed to these methods (e.g. objc_msgSend)

https://github.com/opensource-apple/objc4/blob/master/runtime/objc-runtime-old.h
https://github.com/opensource-apple/objc4/blob/master/runtime/objc-runtime-new.h

If a class has a @property <whatever> lock; that contains a method named lock, the function call rewriter will erronously assume calls to [self.lock lock] (thread that locks the mutex) are calls to [self lock]` (getter function for the mutex)

workflow_objc was formerly a standalone project with slightly different goals than our current plugin.

Currently the architecture is so:

workflow_objc
|- Internal virtual API
|
|- BinaryNinja API Interaction
|  |- Internal API Implementation wrapping BinaryNinja API
|  |- Workflow implementation, etc
|  
|- Core
   |- ObjC Processing using Internal API

Usage of the Internal virtual API should be gradually phased out, and the BinaryNinja API instead should be passed through to the "Core" processing where it makes sense in new code.

ObjC selectively detects [[SomeItem alloc] init] and compiles it down to a single runtime call objc_alloc_init(). We can reverse this.

These are used by Apple in the iOS 16 aligned releases, and can be enabled by hand using -objc_stubs_small when passed to the linker. This video has the details but the TL;DR is that a direct call is made to a uniqued stub that loads the SEL prior to jumping to objc_msgSend.

This may be a dyld-cache extractor issue, but I'm seeing this:

Hopper has the same issue. Is there any way to follow the rabbit hole for these references to end up with the implementation?

This seems bad and since we don't have any better information, we shouldn't nuke what's there. This seems like it might be the product of a separate bug (see the wacky symbol names), but I'm reporting this before I forget. Binary attached.

FTServices.zip

https://github.com/Vector35/workflow_objc/blob/master/Core/AbstractFile.h#L58

Could also potentially rename the datavariables.

Manually defining these results in the analysis improvement:

to (ignore anything other than the pointer in this screenshot I didn't have my PR fork loaded for the first two but the analysis is the same)

These strings are good candidates for a data renderer.

Should there also be an opaque typedef for strings of this type?

Rendering CFString instances in linear view as Objective-C style strings, e.g. @"Hello, world!", would be far more useful than seeing the structure itself.

The code for this is already done, just needs to be cleaned up, checked-in, and merged.