fluxchief
Infinite loop when loading non-AVR file
When I try to open a small non-AVR file on my computer, the whole UI freezes. If I disable this plugin it works again. I'm trying to debug this to find the root cause but I'm posting this now as a preliminary finding.
Specifically, changing __init__.py:392 from
jmp_target = f.lifted_il[0].operands[0].operands[0]
to
jmp_target = None
avoids the infinite loop and the UI freeze. Obviously this also breaks functionality but at least it's a more specific root cause.
This bug is especially exacerbated by the fact that this plugin registers a binaryview that claims to be valid for all files: https://github.com/fluxchief/binaryninja_avr/blob/master/init.py#L416-L419
This is enough of a problem that I'm going to temporarily remove the plugin from the plugin manager until it's resolved.
I've looked into this a couple of months ago but unfortunately could not fix the bug properly. The issue is related to defining functions at addresses that cannot be be decoded and then trying to lift the instructions, specifically https://github.com/fluxchief/binaryninja_avr/blob/master/init.py#L392 which seemes to be a bug in BN and not the extension (unless that's nested calls in the extension code but I have no way to debug this) - it should be able to handle that gracefully (IIRC accessing f.lifted_il[0] is enough to cause it to hang).
Commenting this line out makes it work properly but prevents renaming of the ISR functions. Probably better than causing BN to hang but not a proper fix IMO. That's why I was hoping that it would fix itself as BN gets more mature.
claims to be valid for all files
Which is unfortunately not really something I can change. There is no way to identify a blob of data as valid firmware or not. There are some checks in the init, but they can always be somewhat bypassed to trigger the bug. Maybe some of the checks that are currently in init could be moved here but I have no clue at which step this function is called and whether it takes analysis options into account /shrug.
This is enough of a problem that I'm going to temporarily remove the plugin from the plugin manager until it's resolved.
Sounds good. I suspect unless there is some magic that can be done to work around this and someone knowledgeable will take care of this this won't happen anytime soon. There are a few things that could be better with the extension (mainly better integration with BNs features / analysis options / calling convention etc) but I'm not that familiar with the API and I'm currently not using this enough to warrant spending a weekend trying to figure out how things work.
If anyone wants to check this out and needs a repo file, usually running dd if=/dev/urandom of=file.bin bs=1k count=1 works, I've attached a file that triggers this just in case.
