Jill Developer has a new assignment. She needs code that will overwrite data in a sensitive file. Only the local machine administrator has access to the file, but Jill plans to impersonate the admin account to gain access to the file.
Jill first builds a static class to PInvoke LogonUser and start the impersonation. This class (Utility), and it's method (ImpersonateAdministrator) work well. Jill's next step is to write the following code:
Of course, Jill still has some work ahead to verify the path, the data, and the user who is calling this method. At this early point, however, Jill has one worry she wants to put to rest before moving on - is it possible for a malicious caller to take advantage of the impersonation context and do something other than write to a file?
Comments
A good scheduling of thread.Abort() done by a malicious user from another thread (ok it's not that easy to do that) enables the user to avoid the finally statement and keep the impersonnation context in the remaining of the application. Too bad...
I think if "public" cannot be replaced, a specific code attribute should be set up above the method to restrict callers assemblies.
@Laurent: Let's assume anyone can invoke the method, but we can verify the caller and the paramerters. Is there anyway for the impersonation to "escape" outside of the method.
@Philip: That's quite possible given all the naming constructs you can use in a path. Let's assume we can validate that the incoming path points to a local file. Is there any danger in the impersonation?
msdn2.microsoft.com/en-us/library/aa302372.aspx
Putting two level of try / finally should ensure the finally to be executed before any exception to be thrown:
try
{
try
{
using (FileStream fs = File.OpenWrite(path))
{
fs.Write(data, 0, data.Length);
}
}
finally
{
if (impersonationContext != null)
{
impersonationContext.Undo();
}
}
}
catch
{
throw;
}
Eeewww... How tricky :-(
- using this function you could override other sensitive files
blogs.msdn.com/.../542430.aspx
WindowsImpersonationContext impersonationContext = null;
impersonationContext = Utility.ImpersonateAdministrator();
instead of the more "proper"
WindowsImpersonationContext impersonationContext = Utility.ImpersonateAdministrator();
which suggests that there is a way for the impersonation to be in effect with impersonationContext still null. AN exception thrown in ImpersonateAdministrator would do that (but that conflict with the statement that "class (Utility), and it's method (ImpersonateAdministrator) work well."
@John: Correct, except it places a marker in the stack frame where impersonation starts (in 2.0), and in this case the impersonation starts below the current method doing work.
@James: This was just to make the code pretty for the screen. We'll assume the ImpersonateAdminisrtator method is bulletproof and won't let impersonation start if there is an exception.