Keith Brown recently pointed out that the issues with first pass exception handling extend well beyond the instance I mention of correctly reverting your impersonation context. Basically, anywhere you rely on a finally block to keep your state consistent in the face of exceptions could have a problem with this same issue.
For many cases, where finally is simply used to call Dispose(), or do some other benign task, this isn’t a huge issue. However, if you’re making assumptions about the state of your object that the finally block is helping to enforce, and the violation of those assumptions can cause your code to behave in a way it wasn’t intended to, then your code may very well be susceptible to the exception filter trick.
Keith proposes also doing a cleanup in a catch block, so that the problem is cut off at the pass. Presumably the code would look something like:
I’m not a huge fan of this approach, for several reasons. This is a topic for another post, but as a side bar — one reason is that in general, I don’t like catching exceptions that I don’t know how to handle. Whenever I write code, I’ll catch only specific exceptions which I know how to recover from. The reasoning behind this pattern is that if I don’t know what happened (and if I wasn’t able to catch the specific exception class, either I didn’t know the exception was possible, or there’s a bug in my code), how could I possibly understand how to recover from it? Taken the extreme, catching the base Exception class means that I’ll catch things like ExecutionEngineException, AccessViolationException, and OutOfMemoryException — which range from completely unrecoverable (in the ExecutionEngineException case), down to very likely unrecoverable.
However, since the purpose of this catch block is not to actually handle the exception, but to clean up the state of my object, we’ll set aside exception handling philosophy. Another issue is that even though we’re using the throw statement to generate a rethrow opcode, which prevents us from destroying the exception’s stack trace, the fact that we caught the exception at all is going to make debugging a bear. For instance, a debugger that is set to break on unhandled exceptions will not break for any unhandled exception thrown within SomeProtectedOperation(), since the exception is “handled” by our cleanup catch. That means that you’ll either have to turn on catching all first chance exceptions (which will lead to the debugger breaking much more often than necessary), or you’ll end up always breaking at the rethrow statement.
This reasoning is the reason I chose to implement the Whidbey impersonation wrapper code in Visual Basic. By using an exception filter to clean up my context, I’ve managed to execute on the first pass of exception handling, yet did not actually cause the exception to be caught.
All that being said, I agree completely with Keith’s conclusion … we do need to think about malicious code being able to catch exceptions on the first pass, before our finally blocks run. This is one area that you should definitely take a look at when doing code reviews, since most of the time we’re not thinking about what would happen if our finally block didn’t run when we write code.
One final note … those that curl up with their copy of the ECMA spec at night (what, you don’t? …), might immediately think of using fault blocks to help with this problem. Fault blocks, which can be created in ILASM, are defined in ECMA partition II section 12.4.2 as:
“A fault handler … shall be executed if an exception occurs, but not on completion of normal control flow.”
At first glance, that seems quite promising, if we were to combine a finally with a fault, then we’d be sure to clean up in both cases. However, in the overview of the exception handling process presented in partition II, section 126.96.36.199 we see that fault handlers, like finally clauses, also run on the second pass of exception handling … which prevents us from being able to use them to solve the first pass problem.