• remove comments from func header
• comments should be global items
• functions should have a size
• update fill_function in all plugins for the new format

In both IDA and angr-management

This will be non-trivial to do:

• Ghidra supports Jython, which is only Python 2.
• Ghidra is mostly Java

The infrastructure should be something like this:

• Use the ghidra-bridge for Python3 support to send commands that will be evaluated in Python2
  • This will require us to write a Python client program to connect to the Ghidra server.
• Write the GUI for Ghidra in Python (I am open to ideas for a good Java one)
• Try to use the change hooking/callback approach for pushing from Ghidra

Background

There is currently a bug where if someone pulls from another user it will pull the changes, write those changes to their local state, and commit it back to the repo on their state. This so far is correct behavior. The bug is that when the commit back to the remote happens it marks it in the pulling user's metadata that they did a new push (because they just edited their own state). This causes the table that shows the latest pushes to now show the user that did the latest pull as the latest pusher (because they are the person to most recently push).

This problem is a deep issue in changes caused by pulling from binsync in ida causes the ida API to trigger a write (as it should), but this is shown as the user making a new write when indeed it was actually the API pulling someone else's.

Proposed Fix

First, take a look at the comment in the IDA Plugin's controller (in sync_all) function. The idea is that since we always know how many "push" operations happen after a "pull", we could create a semaphore that increments as many times as we are about to push. This way, anytime the semaphore is greater than 0, we pass a special parameter out of the IDA hook that causes it to not update the time change.

Support return types of functions, as well as their argument names and types.

See line 9: https://github.com/angr/binsync/blob/master/plugins/binja_binsync/plugin.json

Open phonebook in IDA, go to choose_relationship @ 0x1a8b, from there click on business_call to highlight it (do not enter the function), hit Y to change the type. This does not show up in binsync for me.

To fix: find a way to tell what is auto-generated and what is not, then catch that in the hooking of comment making.

Open phonebook in binja, sync person_t from the globals tab

  File "/home/honululu/lukas/tools/binsync/binsync/common/ui/tables/ctx_table.py", line 110, in <lambda>
    menu.addAction("Sync", lambda: self.controller.fill_function(func_addr, user=username))
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 26, in initcheck
    return f(self, *args, **kwargs)
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 93, in state_check
    return f(self, *args, **kwargs)
  File "/home/honululu/.binaryninja/plugins/binja_binsync/controller.py", line 159, in fill_function
    type_, _ = bn_func.view.parse_type_string(stack_var.type)
  File "/home/honululu/lukas/tools/binja/binaryninja/plugins/../python/binaryninja/binaryview.py", line 6173, in parse_type_string
    raise SyntaxError(error_str)
SyntaxError: input:1: error: type 'person_t' is not defined
Traceback (most recent call last):
  File "/home/honululu/lukas/tools/binsync/binsync/common/ui/tables/globals_table.py", line 110, in <lambda>
    filler_func = lambda: self.controller.fill_struct(global_name, user=user_name)
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 26, in initcheck
    return f(self, *args, **kwargs)
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 93, in state_check
    return f(self, *args, **kwargs)
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 315, in fill_struct
    raise NotImplementedError
NotImplementedError

Open phonebook in binja, go to the runner function, hit sync on honululu's version

Traceback (most recent call last):
  File "/home/honululu/lukas/tools/binsync/binsync/common/ui/tables/ctx_table.py", line 110, in <lambda>
    menu.addAction("Sync", lambda: self.controller.fill_function(func_addr, user=username))
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 26, in initcheck
    return f(self, *args, **kwargs)
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 93, in state_check
    return f(self, *args, **kwargs)
  File "/home/honululu/.binaryninja/plugins/binja_binsync/controller.py", line 158, in fill_function
    type_, _ = bn_func.view.parse_type_string(stack_var.type)
  File "/home/honululu/lukas/tools/binja/binaryninja/plugins/../python/binaryninja/binaryview.py", line 6173, in parse_type_string
    raise SyntaxError(error_str)
SyntaxError: input:1: error: syntax error

Version: 2.1.0

Traceback (most recent call last):
  File "/Applications/IDA Pro 7.7/ida64.app/Contents/MacOS/plugins/ida_binsync/hooks.py", line 62, in initcheck
    return f(self, *args, **kwargs)
  File "/Applications/IDA Pro 7.7/ida64.app/Contents/MacOS/plugins/ida_binsync/hooks.py", line 203, in struc_member_renamed
    stack_var_info = compat.get_func_stack_var_info(func_addr)[mptr.soff]
KeyError: 24

BinSync Repo: https://github.com/mahaloz/dreamland
Platform: IDA Pro 7.7
Challenge: dreamland (the patched version)
Function: 0x1349
Action: Sync

We need an options panel in the control panel that allows one to access some runtime options like:

• Syncing Level
• Debug Level
• Shutting down BinSync

Here is a mockup of the design:

This update can come after #67 if we are strapped for time.

Take a look at the api_lock usage in the controller code of IDA to get an idea of how this is fixed with other reversing structures.

Reproduction Info

BinSync Repo: https://github.com/mahaloz/dreamland
Platform: IDA Pro 7.7
Challenge: dreamland (the patched version)
Function: 0x1c27
Action: Sync
Binary: #107 same binary

Crash

Traceback (most recent call last):
  File "/Users/mahaloz/github/binsync/binsync/common/ui/tables/functions_table.py", line 101, in <lambda>
    menu.addAction("Sync", lambda: self.controller.fill_function(func_addr, user=self.item(selected_row, 2).text()))
  File "/Users/mahaloz/github/binsync/binsync/common/controller.py", line 26, in initcheck
    return f(self, *args, **kwargs)
  File "/Users/mahaloz/github/binsync/binsync/common/controller.py", line 93, in state_check
    return f(self, *args, **kwargs)
  File "/Applications/IDA Pro 7.7/ida64.app/Contents/MacOS/plugins/ida_binsync/controller.py", line 285, in fill_function
    data_changed |= self.fill_structs(user=user, state=state)
  File "/Users/mahaloz/github/binsync/binsync/common/controller.py", line 26, in initcheck
    return f(self, *args, **kwargs)
  File "/Users/mahaloz/github/binsync/binsync/common/controller.py", line 93, in state_check
    return f(self, *args, **kwargs)
  File "/Applications/IDA Pro 7.7/ida64.app/Contents/MacOS/plugins/ida_binsync/controller.py", line 219, in fill_structs
    data_changed |= compat.set_ida_struct(struct, self)
  File "/Applications/IDA Pro 7.7/ida64.app/Contents/MacOS/plugins/ida_binsync/compat.py", line 101, in wrapper
    thunk()
  File "/Applications/IDA Pro 7.7/ida64.app/Contents/MacOS/plugins/ida_binsync/compat.py", line 97, in thunk
    output[0] = func(*args, **kwargs)
  File "/Applications/IDA Pro 7.7/ida64.app/Contents/MacOS/plugins/ida_binsync/compat.py", line 462, in set_ida_struct
    data_changed |= ida_struct.add_struc_member(
  File "/Applications/IDA Pro 7.7/ida64.app/Contents/MacOS/python/3/ida_struct.py", line 922, in add_struc_member
    return _ida_struct.add_struc_member(*args)
TypeError: in method 'add_struc_member', argument 4 of type 'flags_t'

Speculation

I think this happens because the flags passed is not actually a flags_t and we need real support for converting nested types into a real flags_t in ida plugin

... instead of a floating dialog.

Background

During the run-up to DEF CON quals (May 27), lots of rapid changes are happing in the Core that is only used fully in the IDA plugin. This issue is a list of items we need to port to the other decompilers after quals.

Items

• goto_address for all plugins (#127)
• enum support (#104)
• type conversion across decompiler (#125)

We currently support Enums in the core, but the plugins still need to use the interface.

Triggered when hitting Sync in the Globals tab

INFO | 2022-04-17 19:52:18,489 | ida_binsync.controller | New data synced for 'redgate' on function 0x1842.
Traceback (most recent call last):
  File "/home/honululu/lukas/tools/binsync/binsync/common/ui/tables/globals_table.py", line 110, in <lambda>
    filler_func = lambda: self.controller.fill_struct(global_name, user=user_name)
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 26, in initcheck
    return f(self, *args, **kwargs)
  File "/home/honululu/lukas/tools/binsync/binsync/common/controller.py", line 93, in state_check
    return f(self, *args, **kwargs)
  File "/home/honululu/lukas/tools/ida/idapro-7.6.2/plugins/ida_binsync/controller.py", line 212, in fill_struct
    compat.set_ida_struct(struct)
  File "/home/honululu/lukas/tools/ida/idapro-7.6.2/plugins/ida_binsync/compat.py", line 101, in wrapper
    thunk()
  File "/home/honululu/lukas/tools/ida/idapro-7.6.2/plugins/ida_binsync/compat.py", line 97, in thunk
    output[0] = func(*args, **kwargs)
TypeError: set_ida_struct() missing 1 required positional argument: 'controller'

See: https://github.com/mahaloz/sync_test/blob/binsync/user0_ida/functions.toml

somehow the function last_push > -1, yes the metadata last_push == -1. Should be impossible.

UI

For merging, we should display a single function in the declaration style of C. We can show which variable has been changed by highlighting them and making them clickable, allowing users to select which version of the change they would like.

We should have a large test case that does like 20 users in the core and uses structs with possible
dependencies on other structs.

When an undo occurs, support changing the BinSync state back to the state it was pre-last-operation. This may be harder since we will need to keep a history of changes either in memory or through the commit history.

• Make it so that you can change user and repo without restarting the decompiler

This is what the UI looks like in angr-management on smaller screens. Please fix @frqmod. Related to closed PR #77

Use some of the binaries from DEF CON Finals 2021, many of them caused stack variable errors which stopped them from syncing.

• remove fill_structs with a single fill_struct, for the needed type

When a user first connects to a project, we should automatically (or with an ask), try to pull all the most relevant data from other users that have done work on functions this user has not.

This is a need as well for reducing some of the GUI needed to do things.
If you are right clicking while inside a function, it should allow you to easily pull from a user.

Binja is done with PyQT so it should be medium difficulty

... so that users do not have to input the repo path all the time.

For IDA to really be useful, we need to be able to pull over types of variables, not just their names.

The most common example of this is casting a variable to a struct pointer of a custom struct that you have defined.

Open phonebook in binja, go to get_relationship @ 0x1a8b, hit sync from redgate.

Binaryninja will hang for a few seconds before finally syncing

True should mean there were absolutely no errors while pulling. False should mean something failed.

Currently we only have fauxware, but we really ought to try some harder more messed up binaries, like the ones from DEF CON Finals 2021 as mentioned in #52.

Since we are using types between IDA and Binary Ninja more its time we start using a real TypeEngine as proposed in one of the internal BinSync meetings.

Proposal

In the core of BinSync, we need a to_crepr and from_crepr for every plugin that is supported in BinSync. In the BinSync database we will store everything as a crepr, then when a plugin pulls, we will detect which plugin it is and change the type as we return it to them. This will be actually not to hard to do. After this is done, it must be integrated into the diffing system.

Deadline for this will be DEF CON Quals.

Closed by: #116

With the following features:

• Select all/none.
• Filtering displayed functions based on function names.
• For all selected functions, setting which user to sync from.
• For each function, display which user it is syncing from (or if it is syncing from another user at all).