Extract crash line if symbols present #44
Conversation
There was a problem hiding this comment.
Thanks for your contribution!
Reporting the crash location in terms of location in a particular source file might indeed be useful. I also like the proposed introduction of a Crash (or maybe better Sample) object for increased readability.
However please consider the following changes in order to keep the code as high quality as possible:
- Please rebase your changes onto branch
experimental!masteris used for stable code only, so PRs should be requested againstexperimental. - Please run the test suite
python setup.py testand provide tests for newly added code! Inspecting the results on coveralls.io greatly helps finding code paths uncovered by the test suite. - Please see the inline comments I made to the proposed changes!
- It may be useful to also add the newly introduced line info to the crashes database
afl-collectcan generate. However this change would be a bit more involved since a db schema update is needed to incorporate the new field.
| self.exploitability="" | ||
| self.description="" | ||
| self.hash="" | ||
| self.line="" |
There was a problem hiding this comment.
You probably wanted to initialize member variables from arguments (even it's not used anywhere in the code atm)?
self.sample=sample
...| @@ -80,16 +80,22 @@ def run(self): | |||
| self.in_queue_lock.release() | |||
| self.exit = True | |||
|
|
|||
| class Crash: | |||
There was a problem hiding this comment.
Well, maybe it's more suitable to call this class Sample since not necessarily all of the testing samples result in crashes?
| @@ -234,13 +234,6 @@ def execute_gdb_script(out_dir, script_filename, num_samples, num_threads): | |||
|
|
|||
| out_dir = os.path.expanduser(out_dir) + "/" | |||
|
|
|||
| grep_for = [ | |||
There was a problem hiding this comment.
I'd prefer to keep the list of exploitable output parsing keywords. This way they're all listed in one place throughout the code. So whenever the output of exploitable changes, we'll only have to update this list.
| crash_obj.description=line.split("Short description: ")[1] | ||
| elif "Hash: " in line: | ||
| crash_obj.hash=line.split("Hash: ")[1] | ||
| elif " at " in line: |
There was a problem hiding this comment.
" at " does not match any output line of my personal gdb+exploitable setup (rendering the entire pr uneffective). Could this be improved to be more generic?
This is supposed to extract source file and line (f.e. main.c:123) from the crashing location, right?
|
Ping @illera88, do you plan to implement (some of) the requested changes? Let me know if you want me to help. |
|
Unfortunately I have no time to do it. :/ |
Very useful when analyzing multiple crashes.
Cleaned up some code. making it more legible.
Cheers