Why is SafeHandle.DangerousGetHandle() "Dangerous"?
Asked Answered
R

2

7

This is the first time ever that I'll be using SafeHandle.

I need to call this P/Invoke method that needs an UIntPtr.

    [DllImport("advapi32.dll", CharSet = CharSet.Auto)]
    public static extern int RegOpenKeyEx(
      UIntPtr hKey,
      string subKey,
      int ulOptions,
      int samDesired,
      out UIntPtr hkResult);

This UIntPtr will be derived from .NET's RegistryKey class. I will be using the method above to convert the RegistryKey class to an IntPtr so I can use the above P/Invoke:

        private static IntPtr GetRegistryKeyHandle(RegistryKey rKey)
        {
            //Get the type of the RegistryKey
            Type registryKeyType = typeof(RegistryKey);

            //Get the FieldInfo of the 'hkey' member of RegistryKey
            System.Reflection.FieldInfo fieldInfo =
                registryKeyType.GetField("hkey", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);

            //Get the handle held by hkey
            if (fieldInfo != null)
            {
                SafeHandle handle = (SafeHandle)fieldInfo.GetValue(rKey);

                //Get the unsafe handle
                IntPtr dangerousHandle = handle.DangerousGetHandle();                
                return dangerousHandle;
            }
}

Questions:

  1. Is there a better way to write this without using "unsafe" handles?
  2. Why are unsafe handles dangerous?
Recapitulate answered 6/12, 2011 at 8:12 Comment(0)
G
4

What you are doing is in fact dangerous. The RegistryKey object you use can get garbage collected and finalized while you are using the IntPtr. Which makes the handle value invalid which makes your code randomly fail. Well, okay, random failure is not exactly dangerous but it does open the door to a handle recycle attack if you in fact keep a hold of the handle for an extended period of time. The random failure mode ought to be enough to inspire you to do something about it.

Make your pinvoke declaration look like this:

[DllImport("advapi32.dll", CharSet=CharSet.Auto)]
internal static extern int RegOpenKeyEx(SafeRegistryHandle key, string subkey, 
    int options, int sam, out SafeRegistryHandle result);

So you can consistently use the safe handle wrapper class. Adjust the reflection code accordingly.

Gynous answered 6/12, 2011 at 12:14 Comment(2)
I forgot to mention that the purpose of my code is to mimic NETFX4's 64-bit registry support. We are only using NETFX 3.5 so there is no SafeRegistryHandle available.Recapitulate
Just make it SafeHandleZeroOrMinusOneIsInvalid, the base class of SafeRegistryHandle. Or just plain SafeHandle if you hate typing the name (who doesn't).Gynous
R
5

The RegistryKey has a handle property. So you can use

private static IntPtr GetRegistryKeyHandle(RegistryKey rKey)
{
    return rKey.Handle.DangerousGetHandle();
}

This is potentially dangerous, because the pointer you are getting may not be valid anymore when you are using it. Quote from MSDN

Using the DangerousGetHandle method can pose security risks because, if the handle has been marked as invalid with SetHandleAsInvalid, DangerousGetHandle still returns the original, potentially stale handle value. The returned handle can also be recycled at any point. At best, this means the handle might suddenly stop working. At worst, if the handle or the resource that the handle represents is exposed to untrusted code, this can lead to a recycling security attack on the reused or returned handle. For example, an untrusted caller can query data on the handle just returned and receive information for an entirely unrelated resource. See the DangerousAddRef and the DangerousRelease methods for more information about using the DangerousGetHandle methodsafely.

Retrad answered 6/12, 2011 at 8:20 Comment(1)
I forgot to mention that the purpose of my code is to mimic NETFX4's 64-bit registry support. We are only using NETFX 3.5 so there is no Handle member in the RegistryKey class.Recapitulate
G
4

What you are doing is in fact dangerous. The RegistryKey object you use can get garbage collected and finalized while you are using the IntPtr. Which makes the handle value invalid which makes your code randomly fail. Well, okay, random failure is not exactly dangerous but it does open the door to a handle recycle attack if you in fact keep a hold of the handle for an extended period of time. The random failure mode ought to be enough to inspire you to do something about it.

Make your pinvoke declaration look like this:

[DllImport("advapi32.dll", CharSet=CharSet.Auto)]
internal static extern int RegOpenKeyEx(SafeRegistryHandle key, string subkey, 
    int options, int sam, out SafeRegistryHandle result);

So you can consistently use the safe handle wrapper class. Adjust the reflection code accordingly.

Gynous answered 6/12, 2011 at 12:14 Comment(2)
I forgot to mention that the purpose of my code is to mimic NETFX4's 64-bit registry support. We are only using NETFX 3.5 so there is no SafeRegistryHandle available.Recapitulate
Just make it SafeHandleZeroOrMinusOneIsInvalid, the base class of SafeRegistryHandle. Or just plain SafeHandle if you hate typing the name (who doesn't).Gynous

© 2022 - 2024 — McMap. All rights reserved.