removed from plugin manager
Opened this issue · 7 comments
Apologies for the late notice, we removed this from the plugin manager some time last year due to:
https://github.com/fluxchief/binaryninja_avr/blob/master/__init__.py#L416-L418
I'd love to re-add it back once that is resolved. Very sorry for not notifying you directly earlier (don't remember if we talked via another mechanism or not)
You actually mentioned this in issue #14, so this was no surprise.
As explained in the bug, fixing this block is actually a bit challenging there are no magic bytes which can be used to differentiate between a valid AVR binary or just a random blob.
It might be possible to work around this with some heuristics, however that's where the second issue comes into play: different AVR chips have different amounts of ISRs and rom sizes. A heuristic that checks that ISRs actually point into the image needs to know how many ISRs exists for the specific chip.
A better solution would be to have to explicitly open the bin with the architecture plugin and having to select the corresponding chip.
Realistically I still don't see myself fixing this anytime soon, haven't been doing lots of RE work lately (and don't even have a up2date BN)
Closing this as dupe of #14
On a second thought let's keep it open so that it's clear to others why this is not part of the plugin repo
With the "linear sweep" pass added to BN, creating the ISR routines is not as important anymore - that code is currently disabled.
The "apply BV during open-with-options" would work, but ideally we'd figure out why it is locking up to begin with.
So I think the link I had above is incorrect due to changes in the file. The problem isn't adding the ISR functions but the is_valid always returning true which means this view gets called every time and not only slows down analysis for all other files but means any bug in the lifting or analysis can cause problems even with files that aren't AVR. That looks to still be true and would have to change before it can be re-added.
You can also register as an ELF handler even so that elf based on AVR will still automatically load like:
https://github.com/Vector35/arch-mips/blob/master/arch_mips.cpp#L2136-L2139
Basically, just call register_arch with the appropriate ident.
That said, yeah, I don't think we have a good way to let the user override which BinaryViews are loaded for a given file, but it sounds like something that would be a good idea and easy enough to do. Lemme talk to the team and see if there's any concerns.
I believe this to be unblocked now with the resolution of #3721.
Happy to submit a PR, but the TL;DR is implement is_force_loadable and have it return true in your BinaryView. This will prevent it from applying to all files but let the user specify they'd like to load it using "open with options".
Awesome, thanks for letting me know. I'll have a look but probably won't be able to do fix this in the next week. Happy to accept your PR if you feel like sending one before that.
Totally fine, no rush on that.