Compartilhar via

"Finally" Does Not Mean "Immediately"

  • Artigo

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.

Comments

  • Anonymous
    September 01, 2004
    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).

    And so on ;-)
  • Anonymous
    September 01, 2004
    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.
  • Anonymous
    September 01, 2004
    I think your code would be less gross if you changed:

    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

    To this:

    Dim NewContext As WindowsImpersonationContext
    Try
    NewContext = NewIdentity.Impersonate()
    DoTheAdminThing()
    Catch Ex As Exception When WeHaveCleanedUp(NewContext)
    Throw Ex
    End Try
    NewContext.Undo()

    -----------

    Private Function WeHaveCleanedUp(ByVal myContext As WindowsImpersonationContext) As Boolean
    myContext.Undo()
    Return True
    End Function

    ----------------

    Exception filters are really useful. Too bad C# doesn't have them (yet).
  • Anonymous
    September 01, 2004
    Sorry about the formatting, but you get the idea, I hope.
  • Anonymous
    September 01, 2004
    That's extremely clever!

    That's why I don't like it.

    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.
  • Anonymous
    September 01, 2004
    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...).
  • Anonymous
    September 01, 2004
    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.
  • Anonymous
    September 01, 2004
    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.
  • Anonymous
    September 01, 2004
    I knew someone would spot that. This article is a reworking of part of chapter 5 of my book.

    I got an email the other day from someone who was wondering about the security semantics of exception handlers, so I figured that now would be a good time to get some of this stuff up on the web.
  • Anonymous
    September 01, 2004
    Oh, and thanks for the note Paul, I did not realize that VB had that capability.

    I wonder if there's a way to do that in C#? That would actually potentially fix a problem we've got in the VSTO2 exception handling semantics...
  • Anonymous
    September 01, 2004
    C# should let you do

    throw;

    just the same.
  • Anonymous
    September 01, 2004
    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.
  • Anonymous
    September 01, 2004
    Why were exception filters designed to behave like that? It's clearly counterintuitive.

    Was it just for performance?
  • Anonymous
    September 01, 2004
    The comment has been removed
  • Anonymous
    September 01, 2004
    I learn something new every day in this job.
  • Anonymous
    September 01, 2004
    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.
  • Anonymous
    September 02, 2004
    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.
  • Anonymous
    September 02, 2004
    This seems to be the fault exclusively of the "when" feature.
  • Anonymous
    September 02, 2004
    The comment has been removed
  • Anonymous
    September 02, 2004
    The comment has been removed
  • Anonymous
    September 03, 2004
    I just wrote a four part series about Assert ... looks like the answer to your pop quiz is Assert Myth #4 (http://blogs.msdn.com/shawnfa/archive/2004/08/25/220458.aspx#myth4)
  • Anonymous
    September 03, 2004
    That's not quite right. I'm having trouble explaining why in a way that doesn't spoil the answer for people here. Your Myth #1 is closer to it.
  • Anonymous
    September 05, 2004
    The comment has been removed
  • Anonymous
    September 09, 2004
    The stack is searched for filter blocks which run first, and then unwound, running the finally blocks.

    I'm not sure what led to this design decision in the CLI or in VB.NET. You might want to ask Paul Vick.
  • Anonymous
    September 22, 2004
    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...
  • Anonymous
    August 30, 2005
    The comment has been removed
  • Anonymous
    August 30, 2005
    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
  • Anonymous
    April 27, 2006
    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.