Potential memory leak due to not implementing IDisposable correctly in LocalGroupTasks

[JohnLaTwC]

I am not an expert on this but from my reading, if you need to free unmanaged resources, you can implement IDisposable. LocalGroupTasks.cs has a helper class that tries to do this, but it looks to have two bugs.

Bug 1: Failure to inherit from IDisposable.

OBJECT_ATTRIBUTES implements Dispose() but does not inherit from IDisposable. Without this, Dispose may never be called.

Note the definition here: (https://www.pinvoke.net/default.aspx/Structures/OBJECT_ATTRIBUTES.html)

-        internal struct OBJECT_ATTRIBUTES
+       internal struct OBJECT_ATTRIBUTES : IDisposable
        {
            public int Length;
            public IntPtr RootDirectory;
            public uint Attributes;
            public IntPtr SecurityDescriptor;
            public IntPtr QualityOfService;
            private IntPtr _objectName;
            public UNICODE_STRING ObjectName;

            public void Dispose()
            {
                if (_objectName == IntPtr.Zero)
                    return;

                Marshal.DestroyStructure(_objectName, typeof(UNICODE_STRING));
                Marshal.FreeHGlobal(_objectName);
                _objectName = IntPtr.Zero;
            }
        }

SharpHound3/SharpHound3/Tasks/LocalGroupTasks.cs

Line 365 in 7615860

internal struct OBJECT_ATTRIBUTES

Bug 2: Need to call Dispose() explicitly

The documentation on implementing Dispose says that you should call it explicitly or wrap the object in a using block--as the GC is not guaranteed to call it. The easiest thing here is to call Dispose in the finally block.

            finally
            {
                //Free memory from handles acquired during the process
                if (serverHandle != IntPtr.Zero)
                    SamCloseHandle(serverHandle);
                if (domainHandle != IntPtr.Zero)
                    SamCloseHandle(domainHandle);
                if (aliasHandle != IntPtr.Zero)
                    SamCloseHandle(aliasHandle);
+                if (objectAttributes != null)
+                    objectAttributes.Dispose();

                if (members != IntPtr.Zero)
                    SamFreeMemory(members);
            }

SharpHound3/SharpHound3/Tasks/LocalGroupTasks.cs

Line 280 in 7615860

SamCloseHandle(aliasHandle);

[rvazarkar]

Will be fixed in vnext:

SpecterOps/SharpHoundCommon@5a05d6d