[Tools] Manual Validation - Fix Credential Manager native memory handling (#401901)

Author: Madhusudhan-MSFT

Tools/ManualValidation/ManualValidationPipeline.cs

@@ -3979,24 +3979,24 @@ 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;
 			IntPtr credPtr;
 			if (CredRead(target, 1, 0, out credPtr)) { //If found, returns true and adds to credPtr, else false and error. 
-				credMem = Marshal.PtrToStructure<CredentialMem>(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.
-				//Original example doesn't include these:
-				CredRelease(credPtr);
-				CredRelease(credMem.credentialBlob);
-				return cred;
+				try {
+					credMem = Marshal.PtrToStructure<CredentialMem>(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");
 			}
@@ -4014,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)]

Tools/ManualValidation/ManualValidationPipeline.ps1

@@ -7431,21 +7431,25 @@ 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;
 			if (CredRead(target, 1, 0, out credPtr)) { //If found, returns true and adds to credPtr, else false and error. 
-				credMem = Marshal.PtrToStructure<CredentialMem>(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.
-				//Original example doesn't include these:
-				Marshal.FreeHGlobal(credPtr);
-				Marshal.FreeHGlobal(credMem.credentialBlob);
-				return cred;
+				try {
+					credMem = Marshal.PtrToStructure<CredentialMem>(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");
 			}
@@ -7467,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);
 			}
-			//Original example doesn't include this. Was going to use FreeCoTaskMem as recommended, but this gave an error. 
-			Marshal.FreeHGlobal(userCredential.credentialBlob);
 		}
 	}
 }