From 59195b4cb017337a161e5d7ec02d33d0deb14677 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Mon, 13 Jul 2026 17:05:19 -0700 Subject: [PATCH 1/2] [ManualValidation, CredManager] Free credential handles with their matching allocators ## Problem - The CredRead buffer was released with `Marshal.FreeHGlobal` instead of the `CredFree` API it requires, and `credentialBlob` -- an interior pointer into that same buffer -- was then freed a second time. - The write path freed a `StringToCoTaskMemUni` allocation with `Marshal.FreeHGlobal` instead of `Marshal.FreeCoTaskMem`. ## Solution - Add a `CredFree` P/Invoke (`CredRelease`) and release the CredRead buffer once with it in `GetUserCredential`; drop the duplicate `credentialBlob` free. - Free the write blob with `Marshal.FreeCoTaskMem` to match its allocator. - Apply the same fix to both the PowerShell embedded C# and the .cs mirror, and declare `CredRelease` as `void` to match the native signature. ## Impact - Credential read/write no longer performs mismatched or duplicate frees; observable behavior is otherwise unchanged. ## How Validated - Embedded C# compiles and loads on Windows PowerShell 5.1 and pwsh 7; standalone PoC round-trips a credential across repeated read/write with no crash. --- Tools/ManualValidation/ManualValidationPipeline.cs | 7 +++---- Tools/ManualValidation/ManualValidationPipeline.ps1 | 13 ++++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Tools/ManualValidation/ManualValidationPipeline.cs b/Tools/ManualValidation/ManualValidationPipeline.cs index de8b8b6d66a86..1b817e061c349 100644 --- a/Tools/ManualValidation/ManualValidationPipeline.cs +++ b/Tools/ManualValidation/ManualValidationPipeline.cs @@ -3979,8 +3979,8 @@ public void AddValidationData(string PackageIdentifier,string gitHubUserName = " //Extend/reference CredWriteW as CredWrite. [DllImport("Advapi32.dll", SetLastError = true, EntryPoint = "CredFree", CharSet = CharSet.Unicode)] - private static extern bool CredRelease([In] IntPtr credentialPtr); - //Extend/reference CredFree as CredWrite. + private static extern void CredRelease([In] IntPtr credentialPtr); + //Extend/reference CredFree as CredRelease. public static Credential GetUserCredential(string target) { CredentialMem credMem; @@ -3993,9 +3993,8 @@ public static Credential GetUserCredential(string target) { //Copies data from an unmanaged memory pointer to a managed 8-bit unsigned integer array. // Credential cred = new Credential(credMem.targetName, credMem.userName, Encoding.Unicode.GetString(passwordBytes)); //Make a new Credential object cred. - //Original example doesn't include these: + //credentialBlob is an interior pointer into credPtr; the release above already frees it. CredRelease(credPtr); - CredRelease(credMem.credentialBlob); return cred; } else { throw new Exception("Failed to retrieve credentials"); diff --git a/Tools/ManualValidation/ManualValidationPipeline.ps1 b/Tools/ManualValidation/ManualValidationPipeline.ps1 index b131b59b583f1..02614ceff1ace 100644 --- a/Tools/ManualValidation/ManualValidationPipeline.ps1 +++ b/Tools/ManualValidation/ManualValidationPipeline.ps1 @@ -7431,6 +7431,10 @@ namespace CredManager { private static extern bool CredRead(string target, int type, int reservedFlag, out IntPtr credentialPtr); //Extend/reference CredReadW as CredRead. + [DllImport("Advapi32.dll", SetLastError = true, EntryPoint = "CredFree", CharSet = CharSet.Unicode)] + private static extern void CredRelease([In] IntPtr credentialPtr); + //Extend/reference CredFree as CredRelease. + public static Credential GetUserCredential(string target) { CredentialMem credMem; IntPtr credPtr; @@ -7442,9 +7446,8 @@ namespace CredManager { //Copies data from an unmanaged memory pointer to a managed 8-bit unsigned integer array. // Credential cred = new Credential(credMem.targetName, credMem.userName, Encoding.Unicode.GetString(passwordBytes)); //Make a new Credential object cred. - //Original example doesn't include these: - Marshal.FreeHGlobal(credPtr); - Marshal.FreeHGlobal(credMem.credentialBlob); + //credentialBlob is an interior pointer into credPtr; free the buffer once via CredFree. + CredRelease(credPtr); return cred; } else { throw new Exception("Failed to retrieve credentials"); @@ -7470,8 +7473,8 @@ namespace CredManager { if (!CredWrite(ref userCredential, 0)) { throw new System.ComponentModel.Win32Exception(Marshal.GetLastWin32Error()); } - //Original example doesn't include this. Was going to use FreeCoTaskMem as recommended, but this gave an error. - Marshal.FreeHGlobal(userCredential.credentialBlob); + //Match StringToCoTaskMemUni allocation with FreeCoTaskMem. + Marshal.FreeCoTaskMem(userCredential.credentialBlob); } } } From ffe4aa88b516b250ed8ecde11583a7d11143d56f Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Mon, 13 Jul 2026 17:16:03 -0700 Subject: [PATCH 2/2] [ManualValidation, CredManager] Free credential buffers on failure paths ## Problem - `GetUserCredential` freed the CredRead buffer only after the marshaling steps succeeded, so an exception during `PtrToStructure`/`Copy`/construction would leak the native buffer. - `SetUserCredential` freed the write blob only after a successful `CredWrite`; a write failure threw before the free, leaking the allocation on every failed write. ## Solution - Wrap the credential body in `try/finally` in both methods so the buffer is always released once it has been allocated, on both success and exception paths. - Guard the raw `CredFree` P/Invoke with a non-null check (`credPtr != IntPtr.Zero`) as defense-in-depth. - Apply identically to the PowerShell embedded C# and the .cs mirror. ## How Validated - Embedded C# compiles and loads on Windows PowerShell 5.1 and pwsh 7. --- .../ManualValidationPipeline.cs | 32 +++++++++++-------- .../ManualValidationPipeline.ps1 | 32 +++++++++++-------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/Tools/ManualValidation/ManualValidationPipeline.cs b/Tools/ManualValidation/ManualValidationPipeline.cs index 1b817e061c349..b4bd5234607eb 100644 --- a/Tools/ManualValidation/ManualValidationPipeline.cs +++ b/Tools/ManualValidation/ManualValidationPipeline.cs @@ -3986,16 +3986,17 @@ public static Credential GetUserCredential(string target) { CredentialMem credMem; IntPtr credPtr; if (CredRead(target, 1, 0, out credPtr)) { //If found, returns true and adds to credPtr, else false and error. - credMem = Marshal.PtrToStructure(credPtr); - //"Marshals data from an unmanaged block of memory to a newly allocated managed object of the type specified by a generic type parameter." - byte[] passwordBytes = new byte[credMem.credentialBlobSize]; //Make a new byte array passwordBytes of size credentialBlobSize. - Marshal.Copy(credMem.credentialBlob, passwordBytes, 0, credMem.credentialBlobSize); - //Copies data from an unmanaged memory pointer to a managed 8-bit unsigned integer array. - // - Credential cred = new Credential(credMem.targetName, credMem.userName, Encoding.Unicode.GetString(passwordBytes)); //Make a new Credential object cred. - //credentialBlob is an interior pointer into credPtr; the release above already frees it. - CredRelease(credPtr); - return cred; + try { + credMem = Marshal.PtrToStructure(credPtr); + //"Marshals data from an unmanaged block of memory to a newly allocated managed object of the type specified by a generic type parameter." + byte[] passwordBytes = new byte[credMem.credentialBlobSize]; //Make a new byte array passwordBytes of size credentialBlobSize. + Marshal.Copy(credMem.credentialBlob, passwordBytes, 0, credMem.credentialBlobSize); + //Copies data from an unmanaged memory pointer to a managed 8-bit unsigned integer array. + return new Credential(credMem.targetName, credMem.userName, Encoding.Unicode.GetString(passwordBytes)); //Make a new Credential object cred. + } finally { + //credentialBlob is an interior pointer into credPtr; free the buffer once via CredFree. + if (credPtr != IntPtr.Zero) { CredRelease(credPtr); } + } } else { throw new Exception("Failed to retrieve credentials"); } @@ -4013,11 +4014,14 @@ public static void SetUserCredential(string target, string userName, string pass userCredential.credentialBlobSize = (int)bpassword.Length; userCredential.credentialBlob = Marshal.StringToCoTaskMemUni(password); //If write fails, emit last error. - if (!CredWrite(ref userCredential, 0)) { - throw new System.ComponentModel.Win32Exception(Marshal.GetLastWin32Error()); + try { + if (!CredWrite(ref userCredential, 0)) { + throw new System.ComponentModel.Win32Exception(Marshal.GetLastWin32Error()); + } + } finally { + //Match StringToCoTaskMemUni allocation with FreeCoTaskMem. + Marshal.FreeCoTaskMem(userCredential.credentialBlob); } - //Original example doesn't include this. Was going to use FreeCoTaskMem as recommended, but this gave an error. - Marshal.FreeCoTaskMem(userCredential.credentialBlob); } [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] diff --git a/Tools/ManualValidation/ManualValidationPipeline.ps1 b/Tools/ManualValidation/ManualValidationPipeline.ps1 index 02614ceff1ace..b619787377c92 100644 --- a/Tools/ManualValidation/ManualValidationPipeline.ps1 +++ b/Tools/ManualValidation/ManualValidationPipeline.ps1 @@ -7439,16 +7439,17 @@ namespace CredManager { CredentialMem credMem; IntPtr credPtr; if (CredRead(target, 1, 0, out credPtr)) { //If found, returns true and adds to credPtr, else false and error. - credMem = Marshal.PtrToStructure(credPtr); - //"Marshals data from an unmanaged block of memory to a newly allocated managed object of the type specified by a generic type parameter." - byte[] passwordBytes = new byte[credMem.credentialBlobSize]; //Make a new byte array passwordBytes of size credentialBlobSize. - Marshal.Copy(credMem.credentialBlob, passwordBytes, 0, credMem.credentialBlobSize); - //Copies data from an unmanaged memory pointer to a managed 8-bit unsigned integer array. - // - Credential cred = new Credential(credMem.targetName, credMem.userName, Encoding.Unicode.GetString(passwordBytes)); //Make a new Credential object cred. - //credentialBlob is an interior pointer into credPtr; free the buffer once via CredFree. - CredRelease(credPtr); - return cred; + try { + credMem = Marshal.PtrToStructure(credPtr); + //"Marshals data from an unmanaged block of memory to a newly allocated managed object of the type specified by a generic type parameter." + byte[] passwordBytes = new byte[credMem.credentialBlobSize]; //Make a new byte array passwordBytes of size credentialBlobSize. + Marshal.Copy(credMem.credentialBlob, passwordBytes, 0, credMem.credentialBlobSize); + //Copies data from an unmanaged memory pointer to a managed 8-bit unsigned integer array. + return new Credential(credMem.targetName, credMem.userName, Encoding.Unicode.GetString(passwordBytes)); //Make a new Credential object cred. + } finally { + //credentialBlob is an interior pointer into credPtr; free the buffer once via CredFree. + if (credPtr != IntPtr.Zero) { CredRelease(credPtr); } + } } else { throw new Exception("Failed to retrieve credentials"); } @@ -7470,11 +7471,14 @@ namespace CredManager { userCredential.credentialBlobSize = (int)bpassword.Length; userCredential.credentialBlob = Marshal.StringToCoTaskMemUni(password); //If write fails, emit last error. - if (!CredWrite(ref userCredential, 0)) { - throw new System.ComponentModel.Win32Exception(Marshal.GetLastWin32Error()); + try { + if (!CredWrite(ref userCredential, 0)) { + throw new System.ComponentModel.Win32Exception(Marshal.GetLastWin32Error()); + } + } finally { + //Match StringToCoTaskMemUni allocation with FreeCoTaskMem. + Marshal.FreeCoTaskMem(userCredential.credentialBlob); } - //Match StringToCoTaskMemUni allocation with FreeCoTaskMem. - Marshal.FreeCoTaskMem(userCredential.credentialBlob); } } }