-
Notifications
You must be signed in to change notification settings - Fork 606
Issue #1761: handle new bin_attribute format for module sections #1773
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
Open
Abyss-W4tcher
wants to merge
26
commits into
volatilityfoundation:develop
Choose a base branch
from
Abyss-W4tcher:issue_1761_module_sect_attr_fix
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+902
−134
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
735f593
make module_sect_attr and bin_attribute optional
Abyss-W4tcher ac4326f
determine sect_attrs.attrs subtype dynamically
Abyss-W4tcher 4f296f7
black formatting
Abyss-W4tcher 75e0d04
sections manual enumeration adjustment
Abyss-W4tcher 825cb32
sections manual enumeration adjustment
Abyss-W4tcher 9478d36
get_sections() sanity check
Abyss-W4tcher 70dfa09
add bin_attribute address virtual member
Abyss-W4tcher d8518b3
1774: consolidate module helpers
Abyss-W4tcher c223a12
handle None in _parse_sections caller
Abyss-W4tcher 522c2d4
rollback to already patched version
Abyss-W4tcher 9104882
add ATTRIBUTE_NAME_MAX_SIZE constant
Abyss-W4tcher a5d1641
use ATTRIBUTE_NAME_MAX_SIZE constant
Abyss-W4tcher 2cda8e3
make binary attributes iteration NULL terminated
Abyss-W4tcher 1c11791
add dynamically_sized_array_of_pointers() helper
Abyss-W4tcher 15a8af5
use dynamically_sized_array_of_pointers in _get_sect_count
Abyss-W4tcher b20da9c
correct type hinting
Abyss-W4tcher 4496909
lru_cache get_modules_memory_boundaries()
Abyss-W4tcher 6e67674
add section address sanity check
Abyss-W4tcher a5cc616
leverage the existing Array facility
Abyss-W4tcher c8d90cf
adjust to the new NULL-terminated processing
Abyss-W4tcher 5d50ad2
slight readability adjustment
Abyss-W4tcher c89c7c3
rollback to 635237b to prevent circular import
Abyss-W4tcher b8e12be
move ModuleExtract class in modules.py
Abyss-W4tcher e9ce095
black formatting
Abyss-W4tcher 32f37ee
remove hasattr check on bin_attribute
Abyss-W4tcher 8b132ea
extend _fix_sym_table's docstring
Abyss-W4tcher File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I don't know whether this is handled in
array_of_pointers, but my worry is reading off the edge of mapped memory, because we're reading all values up toiterator_guard_value. My concern is that somehow that trips an exception which we don't seem to catch in here?I guess the iteration should stop when one of the return values isn't readable, but what about applying this to 100 bytes of mapped memory and asking for 100 4-byte pointers? How would this handle simply not being able to read enough memory? Presumably it'd throw an exception, and that may be what we want, because it should only throw an exception when one of the values it expected to be able to read, could be read, but I just wanted to check...
Uh oh!
There was an error while loading. Please reload this page.
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.
I think that is
array_of_pointers's responsibility, and it seems to raise an exception if it goes OOB:We could add a try/except block right here:
volatility3/volatility3/framework/objects/__init__.py
Lines 811 to 818 in a17281a
+ a
error_on_oob-ish parameter to the array object builder (default to True?):object("array", offset=(2**64)-1), error_on_oob=Falsewhich would return all the entries constructed up to the breaking point, but without raising an exception. What do you think?
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.
I think making a "give me your best attempt" flag can be dangerous because people will use it all the time without considering the consequences, so I'd rather not provide it as a matter of course, but have each caller handle and explain why they can make use of partial results. Does that seem reasonable or am I just making everyone write checking code that could be done once because I don't trust people to code sensibly? What do you think would be best? 5:S