Crashing your Process with Timers
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 = "<USER HERE>";
String pwd = "<PASSWORD HERE>";
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.
Did you file this bug? I believe this is the same: http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=324865.
Seems to be fixed in .NET 4.0 Beta 1.
You can use the “SecurityContext.SuppressFlow” or the “SecurityContext.SuppressFlowWindowsIdentity” method to suppress the flow of the security context or Windows identity across asynchronous threads. I have tested this with SqlConnection and it works. However, this can have some unpredictable consequences depending on if the asynchronous thread actually needs to execute code as the impersonated user.
http://msdn.microsoft.com/en-us/library/system.security.securitycontext_members%28v=VS.90%29.aspx
In my view this problem can be solved by the .NET Framework in two ways:
1. By intentionally suppressing the flow of the security context if running code as the impersonated user is not required (as is in the case with SqlConnection and probably in most of the other cases).
2. By duplicating the token before passing it to the new thread.