Dela via

Safely Impersonating Another User

  • Artikel

Yesterday I posted a bit of code that shows how to impersonate another user in managed code.  However, that code had a subtle security hole waiting to bite you if you used it directly.  Both Dean and Eric found the problem.  In fact Eric reminded me of a blog entry he wrote on the same subject last fall.

The basic problem is that unlike most CLR security settings, impersonation is tied to the executing thread and not to the stack frame.  This means that if malicious code sees that your code is going to impersonate a privileged user, and then call into a method that it can cause to throw an exception, that malicious code can catch the exception and be running as the privileged user in its exception handler.

Steve suggested wrapping the entire impersonate / Undo operation in a simple utility object that implements IDisposable.  That way you could use the C# using construct and have the Dispose method undo impersonation if the stack frame was popped of during an exception unwind.  At first brush this seems like a really good idea, however some deeper analysis shows that even this won't stop a determined malicious assembly.

How's that?  CLR exception handling takes place in two passes.  On the first pass, the CLR walks down the call stack checking for a frame willing to handle the exception.  In C# this means that the type specified in your catch block matches the type of the exception.  However, in VB or IL you can write an exception filter, which is a block of arbitrary code that executes to determine if the exception handler should run.

Once the stack frame containing the exception handler is identified, the CLR returns back to the top of the stack to begin its second pass of exception handling.  On this step, finally and fault blocks are executed and then the frame is popped until the handling frame is reached.

So if we put our Undo call in the finally block, it won't get called until the second pass of exception handling.  That means any malicious code that can execute on the first pass, by implementing an exception filter, will still run in the impersonated context, regardless of the best intentions of our finally block.

What's the solution?  If you're coding in C#, the only way to be called on the first pass of exception handling is to actually catch the exception.  That means that there is unfortunately nothing much more elegant than:

// Begin impersonating the user
WindowsImpersonationContext impersonationContext = WindowsIdentity.Impersonate(userHandle.Token);

try
{
    DoSomeWorkWhileImpersonating();
}
catch
{
    // Clean up
    if(userHandle != IntPtr.Zero)
    {
      CloseHandle(userHandle);
      userHandle = IntPtr.Zero;
    }

    if(impersonationContext != null)
    {
      impersonationContext.Undo();
      impersonationContext = null;
    }

    // allow the exception to continue on its way
    throw;
}
finally
{
    // Clean up
    if(userHandle != IntPtr.Zero)
      CloseHandle(userHandle);

    if(impersonationContext != null)
      impersonationContext.Undo();
}

Notice that we use a plain catch, since ECMA allows for objects not derived from Exception to be thrown.  We also use a throw operation to cause the IL rethrow opcode to be emitted, which limits the amount of interfering we did with the exception handling process.

In VB you can be a little more slick and use an exception filter:

' Begin impersonating the user
WindowsImpersonationContext impersonationContext = WindowsIdentity.Impersonate(userHandle.Token)

Try

    DoSomeWorkWhileImpersonating()

Catch When UndoImpersonation(userHandle, impersonationContext) = False

    Debug.Assert(False, "CleanUp returned False")
Finally

    UndoImpersonation(userHandle, impersonationContext)
End Try

' ...

Private Function UndoImpersonation(ByRef userHandle As IntPtr, ByRef impersonationContext As WindowsImpersonationContext) As Boolean
  If Not IntPtr.Zero.Equals(userHandle) Then
    CloseHandle(userHandle)
    userHandle = IntPtr.Zero
  End If
  If impersonationContext <> Nothing Then
    impersonationContext.Undo()
    impersonationContext = Nothing
  End If
  Return True
End Function

Here, we undo the impersonation in the exception filter, but never actually catch the exception, since the filter will never evalutate to True.  It's a nicer approach than the C# method, since by not catching the exception we make debugging a lot easier.

Both approaches are ugly, but in order to be safe you'll need to implement at least one.  The lesson here is that coding when you don't necessarily trust your callers is difficult.  Throw in the fact that exception safe coding is also difficult to do properly, and you've got yourself into a very tricky situation.  Tomorrow, we'll look at using some new Whidbey features to make the whole process a lot nicer and safer.

Updated 3/23: Fixed double-free problem

Comments

  • Anonymous
    March 22, 2005
    I may be missing something here...
    Wont the finally be called even when there was an exception and so the
    impersonationContext.Undo(); will be called twice in case of an exception!


  • Anonymous
    March 22, 2005
    The comment has been removed

  • Anonymous
    March 23, 2005
    The comment has been removed

  • Anonymous
    March 23, 2005
    Good catch Maheshwar -- that could would indeed double undo, and even more importantly double CloseHandle. I've updated it to solve the problem.

    -Shawn

  • Anonymous
    December 06, 2006
    WWWTC #9 ranks 10 out of 10 on the "difficult and subtle" scale. Let's say we write the following...

  • Anonymous
    December 06, 2006
    WWWTC #9 ranks 10 out of 10 on the "difficult and subtle" scale. Let's say we write the following code

  • Anonymous
    February 08, 2007
    Regarding IDisposable wrapping WindowsImpersonationContext -- Dispose method on WindowsImpersonationContext already calls Undo(). // Declaring Type: System.Security.Principal.WindowsImpersonationContext // Assembly: mscorlib, Version=2.0.0.0 [ComVisible(false)] public void Dispose() {  this.Dispose(true); } [ComVisible(false)] protected virtual void Dispose(bool disposing) {  if ((disposing && (this.m_safeTokenHandle != null)) && !this.m_safeTokenHandle.IsClosed)  {    this.Undo();    this.m_safeTokenHandle.Dispose();  } } so couldn't you do try {  // Begin impersonating the user, relying on Dispose to call Undo  using(WindowsImpersonationContext impersonationContext = WindowsIdentity.Impersonate(userHandle.Token))  {    DoSomeWorkWhileImpersonating();  } } catch // end search for qualifying exception handler to prevent exception filter exploit {  throw; }

  • Anonymous
    February 20, 2007
    You're right, it does, but the second call isn't hurting anything either and it makes the intention of the code more clear to the reader. -Shawn

  • Anonymous
    April 20, 2007
    PingBack from http://www.brianlow.com/index.php/2007/04/20/impersonate-user/