Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Update drillresults.py #3

Closed
wants to merge 1 commit into from
Closed

Update drillresults.py #3

wants to merge 1 commit into from

Conversation

redknight99
Copy link

I found a small bug in drillresults.py which will cause it to crash if certain crash reports are read. (See Issue #2 )

I've implemented a small "Try / Except" patch which will allow the script to continue on and which will print out the offending crash report so it can be removed manually.

@ahouseholder
Copy link
Contributor

Thanks for the fix. I'm in the process of getting our more recent code pushed out here (procedural stuff, not technical), should be days not weeks until that happens -- once that's done we can revisit this request.
Just wanted to let you know we're not ignoring you.


try:
crashid['exceptions'][exceptionnum]['pcmodule'] = pc_in_mapped_address(reporttext, instraddr)
except:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't catch all exceptions, it should be more specific.

Also, it's not clear whether the core problem is here or in the pc_in_mapped_address() function.

@ahouseholder
Copy link
Contributor

So I just updated the develop branch this morning -- the code in there is about 2 years newer than the stuff in the master branch. We've made a lot of changes including a significant refactoring of drillresults into an analyzer that runs inline during a campaign.

The line in question has moved to certfuzz.analyzers.drillresults.testcasebundle_base line 159:

        self.details['exceptions'][exceptionnum]['pcmodule'] = self.pc_in_mapped_address(instraddr)

But it's not clear from your description what exception you're catching. Is it something thrown from within pc_in_mapped_address or maybe a keyerror on self.details? (If the latter, which key is missing?) Most of that might not matter given the below:

At the moment, the msec file you put in Issue #2 no longer appears to cause a crash. (The new code fails to recognize the faulting address and returns gracefully.) As a result I'm going to close this one out since the crashing behavior is no longer an issue.

However it's just skipping over that case as not interesting (when in fact it is, since it's an AV on EIP). I'll create a separate issue to track the fact that drillresults' logic is missing the interestingness of this case. That's a distinct issue from the crashing behavior.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants