Ignore nonconforming note section data

konrad-kruczynski / elfsharp

Pure managed C# library for reading ELF, UImage, Mach-O binaries.

https://elfsharp.it

Other

150 stars 56 forks source link

Ignore nonconforming note section data #47

Closed reubeno closed 4 years ago

reubeno commented 5 years ago

I've come across binaries whose note sections contain data that doesn't match the format expected by ElfSharp. This change ignores such data, rather than throwing on parse of the binary.

Any and all feedback very much welcome -- in particular, I wasn't sure if there was a recommended way to non-fatally warn or otherwise log information about the unexpected sizes.

konrad-kruczynski commented 5 years ago

Thanks! Your PR has not been forgotten :) Yet it must wait for the nearest weekend to be processed.

konrad-kruczynski commented 5 years ago

The change is nice and well described. The only thing missing is at least one unit test - please take a look at other tests in the project (use second solution file for that). If for some reason you can't use the binary in which you encountered described problem (e.g. due to copyrights), maybe alter a "normal" binary in hex editor?

As you wrote, it would be nice to have some kind of fault logging mechanism. Another nice feature would be to have a flag set in ELF ctor, telling whether to ignore such errors or fail at them. But this will probably be introduced in the future, so no need to take it into account within this pull request.

reubeno commented 4 years ago

Thanks for the feedback, @konrad-kruczynski . It's taken me a while to get back to this, but I've followed your suggestion and added a test case with a small binary; without the fix in this PR, the test fails -- and with it, it passes. Any and all feedback welcome!

konrad-kruczynski commented 4 years ago

Finally had some time to look at this. Looks good to me! Now I only need some release-related changes:

go to ELFSharp.csproj and bump informational, package and file version to 2.2.0 (or 2.2.0.0 for file), add yourself to the list of authors and describe your change in package release notes;
add yourself to the list of contributors in README.

Once again thanks for the contribution.

reubeno commented 4 years ago

All done! Let me know if you see anything else you'd like me to update.

Will this somehow automatically result in an updated package posted to NuGet, or is that a manual step that you or one of the other project maintainers takes care of?

konrad-kruczynski commented 4 years ago

Great! I'm merging it.