“Finally” Does Not Mean “Immediately”

All that talk a while back about error handling in VBScript got me thinking about some error handling security issues that involve VB.NET specifically, though they apply to managed code written in any language.

Consider this scenario: you provide a fully trusted library of useful functions which can be used by partially trusted callers. The partially trusted callers might be hostile— that’s why they’re partially trusted, after all. You have a particular method which needs administration rights. When it’s called, it prompts the user for the administrator password, impersonates the administrator, does work, and then reverts the impersonation.

There’s a potentially huge security hole here. What if DoTheAdminThing throws an exception? There’s no Catch or Finally block around the Undo, so control can return to the partially trusted caller with the impersonation still intact! That seems bad! Let’s stick a Finally on there:

Clearly this an improvement from a code-quality perspective, but does it solve the security problem?

No! A Finally block guarantees that the code will run eventually, but it does not guaranteethatno code will run between the point of the exception and the Finally block! VB .NET provides a syntax specifically for running code before the Finally block: the exception filter. Suppose our hostile code looked like this:

Of course, this doesn’t apply to just thread impersonation.Any time that important global state can be made inconsistent requires very careful exception handling to ensure that no hostile code runs while the state is vulnerable. We can do that by catching the error before the hostile code can:

Does this do the same thing as before — potentially pass control to a partially-trusted exception filter before the assertion is reverted? If so, how do you defend against it? If not, why not, and are there any other potential attacks here?

Tune in next time to find out! I’ll be AFK for the Labour Day weekend, so we’ll have the thrilling conclusion next week. Have a nice weekend, everyone.

I won’t answer the question (it would spoil it for the others 😉 ) but you should really put your Impersonate call *inside* the Try block… theoretically, an exception could occur between the callvirt instruction and the stloc instruction, leaving you impersonating the Administrator.

The exception could either be from the CLR itself (eg, OutOfMemory) or if you were foolish enough to give the partially-trusted code ControlThread access, maybe it tried to Abort() you. (Similar things for ControlAppDomain and unloading you).

For safety’s sake, yes, it’s a good habit to put more stuff inside the try. But can an OOM really happen on a store? And if the thread/ADM goes down then the hostile caller just threw away the impersonated thread. Seems like an unlikely attack.

Clever code is badness. As Brian Kernighan famously pointed out, it takes twice as much cleverness to understand code as to write it; clever code is an impediment to maintenance.

It’s clever because it uses a tool to elegantly achieve a goal, but uses the tool in a manner different from its intended use. The by-design purpose of an exception filter is to MAKE A DECISION as to whether the exception can be handled or not. That it implements it by running the filter code during the search of the catch block chain is an implementation detail of the design that you’re taking advantage of for another purpose.

Similarly, I also object to using exceptions as a control flow mechanism in non-error cases. Exceptions are for exceptional circumstances, not normal control flow. There’s nothing technically stopping you from writing programs that work by doing expected non-local-gotos all over the place, but doing so is using a tool for the opposite of its intended purpose.

Ahh… The joys of two-pass exception handling. One comment, though: in the Catch statement, instead of saying "Throw ex", you can just say "Throw". This does a rethrow of the original exception and doesn’t muck about with the exception origin in the exception. This is nice because it doesn’t make it look like your function threw the exception (unless you want it to look that way…).

I agree with your sentiment about clever code, but I disagree with your assessment of this particular technique. I think it’s easier to see with a slightly more complex example. In real code, the function would make a decision about whether or not to handle the exception. It would also make sure that it protected the internal state of the calling routine in all cases. More importantly, I can’t agree with your assessment of the design of the exception filter. It seems to me that "running the filter code during the search of the catch block" is an inherent part of the design contract, not an implementation detail. I would have expected the design of two-pass exception handling to be very different if your assertion were correct.

I’m positive you’ve written almost this exact article before. I can’t find it online so it may have been in the code security book though.

But to tag onto what Peter was saying, in theory it can get an OOM there although maybe some implementations go beyond the spec to make it safe. If they do an abort/unload on you, they can still save the context by cancelling the abort after it jumps out of your code.

In the IL it’s just a special opcode (0xFE1A) rethrow that you can use in a handler block (but not in a filter block to tie back to the original topic). So I’d expect pretty much every NET language to support it if they have exception handling.

Where MAGIC_NUMBER is clearly the number of milliseconds you need to pause to cause the ThreadAbort at exactly the right time. Or you just keep spinning up new threads and sleeping for increasing numbers of milliseconds until you finally get admin.

WRT to the CAS question, the same problem does not occur. This is because the assertion on the asserting method’s frame is too low on the call stack to have an effect on a demand for the asserted permission that might be initiated from code run separately from the partially trusted caller.

As for other potential attacks, of course there are more. Aren’t there always? <g> Without even digging into anything too obscure, there’s the issue of DoSomething() itself which might, for example, invoke a delegate. In addition, the code as written does not make any verification of the caller’s right to perform the target action. While this might be acceptable in some rare cases, it’s opens the code up to a potential luring attack that could be avoided by a demand x then assert y sequence.

I was mulling over this last night, but it seems like your solution is still vulnerable. The pattern you use goes like:

Try

DoTheAdminThing()

Catch Ex As Exception

…

Throw Ex

Finally

…

End Try

But what if we can trick DoTheAdminThing into throwing something that’s not a [mscorlib]System.Exception? Your catch block won’t be picked to handle it so your finally block won’t run until after the exception filter. Now, VB only lets you throw and run filters on Exception, but I wrote my own in IL that doesn’t have this restriction.

> Why were exception filters designed to behave like that? It’s clearly counterintuitive.

Why is it counterintuitive? Consider the simplest case:

try {

foo()

} catch when blah() {

bar()

} finally {

baz()

}

if foo throws, do you expect the finally to run before the filter or after? Clearly this goes blah, bar, baz. Why should it go in a different order just because the finally is lower on the stack?

I suppose the logic could have been "check for filter handlers in the current scope, if there are some, run them. Then check for finally blocks, run them. If there was no successful catch, throw the exception up to the next frame" Seems like there would be more state to keep around, but I suppose it could have been implemented that way.

I don’t know what pros and cons of each were considered by the CLI designers, and the CLI Annotated Specification, page 156, isn’t much help.

>>> if foo throws, do you expect the finally to run before the filter or after? Clearly this goes blah, bar, baz. Why should it go in a different order just because the finally is lower on the stack? <<<

But but but… that’s not what makes it counterintuitive.

I expect it to unwind up the stack, going through each catch/finally it comes to in turn. In any *specific* try/catch/finally block, sure, the catch can go first as you describe, but I do *not* expect a catch from further away to fire before a much more local finally.

If I understand you correctly, the stack is effectively unwound once for catch blocks then again for finally blocks. Is that right? If so, then that is indeed counter-my-intuition.

I’ve done a fair bit of programming in Delphi, and there the finally is guaranteed to run before you leave its locality. Perhaps I’m blinded by previous experience, but I can’t think why you’d want it to work any other way. Indeed, the potential security issue you’ve highlighted appears to be a great argument against any other approach.

Eric Lippert: "Finally" Does Not Mean "Immediately" I agree w/ Owen that the difference between how filter blocks and finally blocks are processed seems counter intuitive at first glance. It would be good to know whether this is particular…

I’ve been asked to explain this blog entry several times. I wanted to add my comments here (a) so I can stop repeating myself, (b) so people I don’t know will be benefit from any clarification I can add, and (c) because I might be wrong and I’d like someone to point it out if I am.

Q: Why does it do that? Isn’t it wrong to run code out of the try/finally block before the finally is executed?

A: See the CLI Partition I, Section 12.4.2.5. Clearly someone made a conscious decision that is should work this way. I’m not privy to the real though process behind this, but I can take a stab.

I think this is Windows structured exception handling (SEH) showing through. Windows has a nice, robust, fully featured exception handling system. When you cause an exception in Windows (either by calling RaiseException or by causing a hardware fault), the OS walks a chain of exception and termination handlers and evaluates a filter for each exception handler. The filter can say (a) keep looking, I’m not the handler you want, (b) stop looking and use this filter, or (c) stop looking and continue executing where you left off. The third one is useful if you want to fault in memory (or a dll), handle some sort of integer overflow, continue after hitting a breakpoint in a debugger, continue after hitting an *exception* in a debugger, etc.

Because Windows exceptions have this nice feature of being something you can ignore or otherwise recover from, it’s important not to assume you’ll exit the innermost protected block just because you hit an exception. Following this through a little, that means it’s important not to run code in the termination handlers you found along the way unless you find a filter that tells you it’s the codeflow’s really going to exit its protected block. Only after Windows finds an exception filter that will handle the exception is it safe to call the termination handlers for any nested blocks.

Q: But you’re talking about Windows EH; the .Net Framework is platform independent.

A: Yes. You can implement this sort of exception handling on pretty much any platform capable of dealing with C’s setjump/longjump. I just so happens that using Windows EH makes the .Net Framework more efficient, able to more easily supported by debuggers, and more able to interoperate with native code written for Windows. The CLI folks (based my reading of I.12.4.2), did a fairly good job of documenting what behavior to expect.

Q: If I have this code:

IDisposable disposeMe = new SomeDisposableClass();

try

{

/* do something that might throw */

}

catch

{

throw;

}

finally

{

disposeMe.Dispose();

}

Can I be sure any unsafe state created in SomeDisposableClass is disposed of before code exits this block?

A: No. Again looking a I.12.4.2, there are 4 kinds of trys: one with a finally, one with a generic catch, one with a type-filtered catch, and one with a user-filtered catch. The code above really translates to:

IDisposable disposeMe = new SomeDisposableClass();

try

{

try

{

/* do something that might throw */

}

catch

{

throw;

}

}

finally

{

disposeMe.Dispose();

}

Q: How did you find out about all this? When I read I.12.4.2, I don’t see half of what you’re talking about.

A: I.12.4.2 is just a reality check. The only way to be absolutely sure your code is doing what you think it is to step through it in the debugger and run it through interesting cases. Since the language is doing things you might not expect, stepping though the disassembly is probably the way to go (until you again know what to expect from the higher level language).

Where can exception filters be useful? Surely to do anything you can’t do just by catching the exception and doing a "throw;" if you don’t like it they need internal knowledge of the code where the exception is thrown. That just seems wrong – what about encapsulation?

Surely the internal details of a method’s exception state shouldn’t be part of its public interface.