"Finally" Does Not Mean "Immediately"

"Finally" Does Not Mean "Immediately"

Rate This
  • Comments 34

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.

Public Sub DoTheThing()
  ' Omitted -- prompt user to obtain password
  NewIdentity = GetWindowsIdentity(AdminName, Domain, Password)
  NewContext = NewIdentity.Impersonate()
  DoTheAdminThing()
  NewContext.Undo()
End Sub

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:

  NewIdentity = GetWindowsIdentity(UserName, Domain, Password)
  NewContext = NewIdentity.Impersonate()
  Try
    DoTheAdminThing() 
  Finally
    NewContext.Undo()
  End Try

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 guarantee that no 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:

Public Sub IAmSoEvil()
  Try
    NiceObject.DoTheThing()
  Catch Ex As System.Exception When ExploitFlaw()
    DoSomethingElse()
  End Try
End Sub
Public Function ExploitFlaw() As Boolean
  DoSomethingEvil() 
  ExploitFlaw = True
End Function

Let's trace through what happens.

  • First, the hostile partially trusted IAmSoEvil calls the fully trusted DoTheThing method.
  • DoTheThing impersonates the administrator and calls DoTheAdminThing.
  • DoTheAdminThing throws an exception (possibly because the partially trusted code has previously eaten up lots of memory or otherwise caused error conditions to become more likely.)
  • The .NET Runtime finds the Catch block in IAmSoEvil and runs ExploitFlaw to see if this block can handle the exception.
  • ExploitFlaw calls DoSomethingEvil, which attempts to take advantage of the fact that it is now impersonating an administrator.
  • DoSomethingEvil finishes its attempt at an exploit and returns.
  • ExploitFlaw returns True. The .NET Runtime stops its search for exception handlers.
  • The .NET Runtime runs the code in the Finally block of DoTheThing. The admin impersonation is reverted.
  • The .NET Runtime runs DoSomethingElse in the Catch block of IAmSoEvil.

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:

Dim Reverted As Boolean
Reverted = False
NewContext = NewIdentity.Impersonate()
Try 
  DoTheAdminThing()
Catch Ex As Exception
  NewContext.Undo()
  Reverted = True
  Throw Ex
Finally 
  If Not Reverted Then 
    NewContext.Undo()
  End If
End Try

Gross, but there's not much else you can do about it.  Fortunately, these kinds of situations are pretty rare.

Pop quiz: Is this another example of the flaw pattern?

FileIOPermission.Assert()
Try 
  DoSomething()
Finally 
  CodeAccessPermission.RevertAssert()
End Try

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’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).


  • Thanks Sean -- spec diving is always interesting.

    For the expert take on SEH and CLR exception handling, see CBrumme's massive blog entry on the subject:

    http://blogs.msdn.com/cbrumme/archive/2003/10/01/51524.aspx
  • 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.
Page 3 of 3 (34 items) 123