search

Adubbz / Ghidra-Switch-Loader

Nintendo Switch loader for Ghidra
ISC License
285 stars 43 forks source link

Remove initElfHeader() override for Ghidra 11 compatability #51

Closed TSRBerry closed 11 months ago

TSRBerry commented 11 months ago

This PR closes #50 by replacing the initElfHeader() override with a ByteArrayProvider containing the ELF magic bytes.

With Ghidra 11 the initElfHeader() method was removed and is now part of the constructor (see: https://github.com/NationalSecurityAgency/ghidra/commit/337b1d09043b88273fcd4cd0baf645a9c393844a#diff-da813dcb74d3fd09c229d988af922102b98e5379d54d9a53cf40d3f1973f8870L106-L135).

The first commit is compatible with Ghidra 10 and 11, but I'm not sure if the same is true for the second one as I only tested 11.

The second commit ensures the internal state matches with the initElfHeader() override we used to have by restoring the old values of every field that gets modified by the new constructor.

After testing the first commit and the second commit separately I couldn't find any difference in the analysis or the import process between them. So I got curious and tested both commits a bit more in depth using ldn from 15.0.0 and 17.0.0. To do this, I imported the binaries and analyzed them. After that I created a HTML export and diffed the resulting files. But for both 15.0.0 and 17.0.0 there was still no difference.

This makes it seem like there is no need for my second commit, so I'd like to get some feedback on that. Do you know if the internal state of ElfHeader is actually important to us during other operations? If it isn't should I just remove my second commit again?

Adubbz commented 11 months ago

I'm unsure how much we actually need to care about its internal state. When implementing this I did the bare minimum necessary to make the loader functional. If everything functions correctly without reflection hackery I'd probably leave it out. I'm not particularly concerned with maintaining compatibility with old Ghidra versions. People can use an old version of the loader if they wish to use an old version of Ghidra.

TSRBerry commented 11 months ago

Alright, thank you! I removed the second commit, so this should be ready for review now!