Here’s a bug I had to diagnose this weekend that had me scratching my head for a bit before getting it. Consider this sample piece of code [1]:
using System;
using System.Collections.Generic;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Security;
using System.Security.Principal;
using System.Text;
using System.Threading;
namespace Winterdom.Samples {
class Program {
private const int LOGON32_PROVIDER_DEFAULT = 0;
private const int LOGON32_LOGON_NETWORK = 3;
[DllImport("advapi32.dll", SetLastError = true)]
public static extern bool LogonUser(
string lpszUsername, string lpszDomain, string lpszPassword,
int dwLogonType, int dwLogonProvider, out IntPtr phToken
);
static void Main(string[] args) {
AppDomain domain = AppDomain.CurrentDomain;
domain.UnhandledException += (sender, e) =>
Console.WriteLine(e.ExceptionObject.ToString());
WindowsIdentity wi = new WindowsIdentity(GetLogon());
WindowsImpersonationContext wic = wi.Impersonate();
Timer timer = new Timer(
(state) => {}, // do nothing on the callback!
null, 3000, 3000
);
wic.Undo();
// If we remove this line, no crash happens!
wi.Dispose();
Console.WriteLine("Wait for the crash!");
Console.ReadLine();
}
static IntPtr GetLogon() {
String user = "" ;
String pwd = "" ;
IntPtr token = IntPtr.Zero;
bool success = LogonUser(
user, ".", pwd, LOGON32_LOGON_NETWORK,
LOGON32_PROVIDER_DEFAULT, out token
);
if ( !success )
throw new System.ComponentModel.Win32Exception();
return token;
}
}
} Now put a valid username/password in GetLogon() and run it. Then watch the application crash with an unhandled exception in a secondary thread. You should’ve spotted an exception like this one:
System.ObjectDisposedException: Safe handle has been closed
at System.Security.Principal.Win32.ImpersonateLoggedOnUser(SafeTokenHandle hToken)
at System.Security.Principal.WindowsIdentity.SafeImpersonate(SafeTokenHandle userToken, WindowsIdentity wi, StackCrawlMark& stackMark)
at System.Security.Principal.WindowsIdentity.Impersonate(StackCrawlMark& stackMark)
at System.Security.SecurityContext.SetSecurityContext(SecurityContext sc, SecurityContext prevSecurityContext, StackCrawlMark& stackMark)
at System.Threading.ExecutionContext.SetExecutionContext(ExecutionContext executionContext)
at System.Threading.ExecutionContext.runTryCode(Object userData)
at System.Runtime.CompilerServices.RuntimeHelpers.ExecuteCodeWithGuaranteedCleanup(TryCode code, CleanupCode backoutCode, Object userData)
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading._TimerCallback.PerformTimerCallback(Object state) Why does it happen?
The problem appears because we impersonate an identity and soon after destroys the WindowsIdentity instance used to create impersonation token.
Between those two moments, we start a timer. When that happens, the runtime will capture the state of the calling thread, including its SecurityContext, and save it so that it can restore it on the background thread used to invoke the timer callback.
Since we destroy the WindowsIdentity instance soon after starting the timer, by the time the callback tries to fire, the authentication token has been closed already. The problem is that the framework makes no attempt to verify that the security context it saved is still valid before trying to impersonate the identity on the callback thread as well.
This leads to an unhandled exception which, on .NET 2.0, will cause your process to crash inevitably. It’s fair to say, though, that even if the framework actively checked for this condition and found that the authentication token was no longer valid, there would not be much else it could do, since it might be a bit dangerous to simply ignore it and invoke the thread on the original process security context (or at least it would lead to some unexpected behavior).
The bad part
The bad part about this issue is that you really don’t know who might be starting a new timer (or an
y other framework facility that saves the SecurityContext for later use) in between your Impersonate() and Undo() calls.
For example, the original code I was tracking this problem in was a lot more complex, involving a custom Windows PowerShell host and the invoke-sqlcmd cmdlet that comes with SQL Server 2008. In that scenario, the problem wasn’t invoke-sqlcmd per se.
It just turned out that the ADO.NET Connection Pooling facilities will create background timers to handle cleanup of the connection pools the first time you get a new connection from the pool.
The really nasty part, however, was that the timer is created with a random interval, which meant you could never be sure when the process would crash: Sometimes it would happen just a few seconds after disposing the WindowsIdentity instance, sometimes several minutes later. This made figuring out the problem a lot harder.
[1] Yes, I’m aware the sample is leaking a handle, but that’s not really the problem.
