Use proper BN_DECLARE_CORE_ABI_VERSION macro for Binary Ninja plugin

[jonpalmisc]

Binary Ninja has a very fast development cycle and sometimes has changes to the core ABI. We have designed a system to prevent loading plugins that use a different core ABI version than the product itself to avoid crashes and other issues.

BinExport tries (and succeeds) to circumvent this safety check by falsely reporting the core ABI version it was linked against:

binexport/binaryninja/main_plugin.cc

Lines 490 to 499 in 43b8d68

	extern "C" BINARYNINJAPLUGIN uint32_t CorePluginABIVersion() {
	// BinExport should work on both channels of the 2.x series as it only uses
	// API functions that have remained relatively stable.
	// Note: This works around Binary Ninja's ABI version handling and BinExport
	// will fail to load with an error message if the "dev" channel diverges
	// too far from stable. However, users on "dev" should expect some
	// breakage and failing to load the plugin still leaves Binary Ninja
	// functional.
	return BNGetMinimumCoreABIVersion();
	}

As a result, BinExport is a regular source of crashes for a lot of users. Migrating to using the official BN_DECLARE_CORE_ABI_VERSION macro would resolve this issue, and is recommended. All that is needed is to replace the entire CorePluginABIVersion function (shown above) with the BN_DECLARE_CORE_ABI_VERSION macro.

Additionally, it seems that the BinExport plugin re-populates itself after some time if removed. This is very frustrating, especially if BinExport is causing crashes on startup, which it recently has for some users.

[cblichmann]

If I remember correctly, I had a discussion with @psifertex about this at some point.
The initial rationale for BinExport circumventing the check was to provide more forward compatibility. I only have a limited amount of time to spent on BinExport/BinDiff and I wanted people to be able to use it without having to provide newer binaries that chase after Binary Ninja. Note that this was around the 1.x <-> 2.x time frame so likely things have settled down a bit.

In an ideal world, Binary Ninja would load plugin binaries and check that all functions are actually exported (i.e. depend on the OS loader, like IDA :)). That way, BinExport can continue to work, even if Binary Ninja introduces additional APIs, and it can be skipped if Binary Ninja removes APIs. This of course only works if incompatibly changed APIs come with a rename.

Additionally, it seems that the BinExport plugin re-populates itself after some time if removed. This is very frustrating, especially if BinExport is causing crashes on startup, which it recently has for some users.
Yes, this is part of BinDiff's bindiff_config_setup that re-creates symlinks in the user's $HOME if missing (as there's no way to figure out Binary Ninja's per-machine install directory).
Fortunately, fixing this issue (and re-releasing BinDiff) fixes that as well.

All that is to say that I will look into this a bit more, and I'm not opposed to changing to BN_DECLARE_CORE_ABI_VERSION.

[melomac]

In the meantime, the /Library/LaunchAgents/com.google.security.zynamics.bindiff.plist agent brings back ~/Library/Application Support/Binary Ninja/plugins/binexport12_binaryninja.dylib on every restart and users have to remove the plugin to prevent Binary Ninja from crashing.

A work-around seems to create an immutable empty file at that location so the agent isn't able to re-install the crashing plugin

[cblichmann]

Please don't. People will forget about the chflags and be very confused as to why newer versions of BinDiff will no longer properly install their plugins.
Instead, I suggest to just disable the launch agent:

/bin/launchctl unload \
    -F /Library/LaunchAgents/com.google.security.zynamics.bindiff.plist

[cblichmann]

Just re-checked: The current stable channel of the Binary Ninja API defines

#define BN_CURRENT_CORE_ABI_VERSION 21
#define BN_MINIMUM_CORE_ABI_VERSION 20

and the current dev version (from 22c0d1f defines

#define BN_CURRENT_CORE_ABI_VERSION 24
#define BN_MINIMUM_CORE_ABI_VERSION 24

So the BinExport binary will not be able to support both, even though Binary Ninja is at 3.1 for both. This is unfortunate, as this means that people will have to build their own BinExport plugins if they want to use BinDiff 8 and above with the dev channel.

[melomac]

Many thanks for following up with BN support and for maintaining BinDiff of course. BinDiff is powerful and has been a strong and useful software in my reverse engineering toolbox.

Would you please be so kind to consider adding BUILD instructions? May be a Wiki page for start, with BN's and progressively adding other supported tools?

FWIW, the launchctl unload -F command didn't persist across reboot. May be, assuming the Installer package overwrites the launchd.plist file on update, running this command additionally would have help:

sudo defaults write /Library/LaunchAgents/com.google.security.zynamics.bindiff.plist Disabled -bool YES

[cblichmann]

Would you please be so kind to consider adding BUILD instructions? May be a Wiki page for start, with BN's and progressively adding other supported tools?

You mean like these instructions: https://github.com/google/binexport#how-to-build?

If you don't have IDA Pro, use -DBINEXPORT_ENABLE_IDAPRO=OFF on the command line.

FWIW, the launchctl unload -F command didn't persist across reboot. May be, assuming the Installer package overwrites the launchd.plist file on update, running this command additionally would have help:

sudo defaults write /Library/LaunchAgents/com.google.security.zynamics.bindiff.plist Disabled -bool YES

Ah yes that might work better :)

[melomac]

I see you got us all covered from the beginning! Confirming it is trivial to build BinExport plugins on macOS following the README instructions 👏

BinExport 12 (@9a69edc, Aug 29 2022), (c)2004-2011 zynamics GmbH, (c)2011-2022 Google LLC.

You may want to remove the extra trailing backslash here though:

cmake .. \
    -G Ninja \
    -DCMAKE_BUILD_TYPE=Release \
    "-DCMAKE_INSTALL_PREFIX=${PWD}" \
    -DBINEXPORT_ENABLE_IDAPRO=ON \
    "-DIdaSdk_ROOT_DIR=${PWD}/../third_party/idasdk" \
    -DBINEXPORT_ENABLE_BINARYNINJA=ON \

Thank you (again) 🙏

[psifertex]

FYI -- related is that we're using Vector35/binaryninja-api#3399 to track the ability to have github actions that could potentially product releases that track both dev and stable automatically.

[psifertex]

This issue has been resolved for a while I believe?

[cblichmann]

Yes, I don't think we currently face this issue. Closing.

[psifertex]

So, I think this is my fault since I suggested this be closed already but as of right now the stable installer for bindiff still has the unfortunate behavior of putting back the "old" version of the library even when one already exists. Should I open a new issue to track disabling that as that is the cause of the vast majority of the complaints we receive from users about MacOS BinaryNinja crashing. In particular, it's bitten me several times in the last few days of testing before even I remembered!

[cblichmann]

Fixed in BinExport. Still present in BinDiff 7 stable.