Allow target application to handle continuable exception

[BiTOk]

Most exceptions can be handled by the target application. WinAFL should not terminate the application with every exception, only after EXCEPTION_NONCONTINUABLE one. Fixes #209.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[BiTOk]

@googlebot I signed it!

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[ifratric]

Interesting, do you know in which cases exactly EXCEPTION_NONCONTINUABLE will be set and (perhaps more importantly) when it will not be set? My concern here is missing some exceptions that are actual security problems.

[BiTOk]

I don't know a close list of cases where EXCEPTION_NONCONTINUABLE is uses. Maybe my condition is too strict and may miss some valuable cases, but current condition is too weak and some exceptions cause process termination improperly.
Could you clarify why you registered the onexception func as an exception handler in dr_client_main? Maybe better to pass at least EXCEPTION_ACCESS_VIOLATION and EXCEPTION_INT_DIVIDE_BY_ZERO to application handlers and catch it in UnhandledExceptionFilter in case if it isn't handled by the application?
When should we consider an exception as a security problem in your opinion?

[ifratric]

IIRC, when a target is running under DynamoRIO, some exceptions will normally be generated and handled by DynamoRIO. drmgr_register_exception_event is a mechanism for the client to get only those exceptions that are not a part of DynamoRIO's workflow and are caused by the application code itself.

In the past, I've seen cases where exploitable exceptions happened that were behind exception handlers, this is the reason why I'm perhaps too fast in declaring something a bug. But I realize that might not work best for some targets that use exceptions internally.

Perhaps a compromise would be to have this feature behind a flag?

Also, have you tested that, with this change, valid security problems are still caught?

[expend20]

Hey guys, very interesting topic. Let me ask you some question. If the fuzzed program had heap OOB write and that write is handled by try/except block correctly without crashing, would it still be a vulnerability? Imho, yes, although I didn't see any particular write ones. But I've seen a lot of OOB read which was covered by handler and imho it is worthwhile to know about them.

[ifratric]

"If the fuzzed program had heap OOB write and that write is handled by try/except block correctly without crashing, would it still be a vulnerability?"

I'm not even sure how you would recover from an OOB write unless there is a guard page immediately after the affected buffer. So, in a general case, yes, I would consider it a vulnerability.

[expend20]

Another raw thought: there is no tools to catch stack OOB read afaik. So, developing one would lead to some good catches, probably :)

[BiTOk]

Also, have you tested that, with this change, valid security problems are still caught?

Not enough yet, for starters I wanted to discuss this with you. I'll do it soon and add the flag.

[BiTOk]

Tests showed that my current solution is too narrow. What do you think of such an exception handling scheme: by default, we catch exceptions only in the UnhandledExceptionFilter, i.e. only when the application cannot handle it by itself, there is a flag of “tough exception handling” that handles exceptions as it is happening now.
In this case, without a flag, there will be no subscription to exceptions in dr_client_main and I'll fix UnhandledExceptionFilter hook for Windows older than 8.

[ifratric]

Going through UnhandledExceptionFilterWorth sounds like it would be worth a shot (again, DR messes with exceptions internally so hopefully the relevant stuff goes through), but I'd leave the current behavior as the default.