m4b / goblin

An impish, cross-platform binary parsing crate, written in Rust
MIT License
1.18k stars 158 forks source link

Add `ParseOptions` field to toggle parsing Attribute Certificates for PEs loaded into memory #377

Closed suttonbradley closed 12 months ago

suttonbradley commented 1 year ago

Per this documentation (just above "Section Data"), the attribute certificate table is not loaded into memory along with the rest of the PE. This PR fixes #376 by adding a ParseOptions field that allows a user to skip parsing the attribute certificate table altogether, making parsing of loaded PEs possible.

suttonbradley commented 1 year ago

I'm open to opinions here design-wise -- in order to avoid a major version bump, I feature-gated the new ParseOptions field. Also open to opinions on naming, as I'll prioritize speed here over anything else.

m4b commented 1 year ago

Since there are breaking changes coming up I think the design of this should ignore that as a downside; I don't think a feature flag for this is the best approach, and would prefer it to work programatically instead, if you're still up for fixing it :)

suttonbradley commented 1 year ago

Makes sense, changed it accordingly. Let me know what else I can do here to get it merged!

nightmared commented 12 months ago

Hello, just wanted to drop by and say that this proved useful to me, for parsing a binary loaded in memory inside an UEFI environment.

If I were very fussy, I would say that perhaps you could add a comment to the description of the field parse_attribute_certificates to warn users that as a consequence of disabling that flag, authenticode_excluded_sections will be incomplete.

Except for the 6 commits that could probably be squashed into a single one, this looks good to me.

m4b commented 9 months ago

released in 0.8.0, happy new year!