-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix fail on malformed certificate table parsing #417
Changes from all commits
2c7f7b5
b46e266
872544b
ac97d4c
aa0de1b
8838feb
1f7cdc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
/// Parsing Options structure for the PE parser | ||
#[non_exhaustive] | ||
#[derive(Debug, Copy, Clone)] | ||
pub struct ParseOptions { | ||
/// Wether the parser should resolve rvas or not. Default: true | ||
|
@@ -8,14 +9,35 @@ pub struct ParseOptions { | |
/// memory](https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#other-contents-of-the-file). | ||
/// For on-disk representations, leave as true. Default: true | ||
pub parse_attribute_certificates: bool, | ||
/// Whether or not to end with an error in case of incorrect data or continue parsing if able. Default: ParseMode::Strict | ||
pub parse_mode: ParseMode, | ||
} | ||
|
||
impl ParseOptions { | ||
#[derive(Debug, Copy, Clone)] | ||
pub enum ParseMode { | ||
/// Always end with error on incorrect data | ||
Strict, | ||
/// Incorrect data will not cause to end with error if possible | ||
Permissive, | ||
} | ||
|
||
impl Default for ParseOptions { | ||
/// Returns a parse options structure with default values | ||
pub fn default() -> Self { | ||
fn default() -> Self { | ||
ParseOptions { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also i don't know who added this but this does not implement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for review, just pushed fixes |
||
resolve_rva: true, | ||
parse_attribute_certificates: true, | ||
parse_mode: ParseMode::Strict, | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you also add a method that returns what the TE parser uses; we can call it |
||
} | ||
} | ||
|
||
impl ParseOptions { | ||
pub(crate) fn te() -> Self { | ||
Self { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. more idiomatic would be something like: Self {
resolve_rva: false,
parse_attribute_certificates: false,
.. Self::default()
} this way if new methods are added don't (maybe) need to update this location in source, but it's fine. I'm also wondering if we should make this non |
||
resolve_rva: false, | ||
parse_attribute_certificates: false, | ||
parse_mode: ParseMode::Strict, | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debating whether we should add
#[non_exhaustive]
to this struct to make it future compatible if we need to add another field like this in the future (and not break people)