.NET memory leak when using native crypto resources

Question

I have an application that needs to encrypt and decrypt strings. Below is my decryption method:

public static string Decrypt(string cipherText) { try { //Decrypt: byte[] keyArray; byte[] toDecryptArray = Convert.FromBase64String(cipherText); keyArray = UTF8Encoding.UTF8.GetBytes(key); AesCryptoServiceProvider Aes = new AesCryptoServiceProvider(); Aes.Key = keyArray; Aes.Mode = CipherMode.CBC; Aes.Padding = PaddingMode.PKCS7; Aes.IV = IV; ICryptoTransform cTransform = Aes.CreateDecryptor(); byte[] resultArray = cTransform.TransformFinalBlock(toDecryptArray, 0, toDecryptArray.Length); Aes.Clear(); return UTF8Encoding.UTF8.GetString(resultArray, 0, resultArray.Length); } catch (Exception ex) { return "FAILED:*" + cipherText + "*" + ex.Message; } } 

However, this seems to be flowing. As you can see, all variables are locally localized, so they should be freed when this block is executed. As a background, I sometimes call this method almost non-stop.

To determine that I am a leak, I run my server with a lot of requests. By carefully monitoring each request for each code path that is required, I decided that this piece of code is the culprit. When commenting in my memory, the usage rises sharply and linearly, when commented out (and the cipherText value is simply returned) the memory usage remains even.

Answer 1

AesCryptoServiceProvider implements IDisposable. Try using it in a using block. Something like below:

 using(AesCryptoServiceProvider Aes = new AesCryptoServiceProvider()){ Aes.Key = keyArray; Aes.Mode = CipherMode.CBC; Aes.Padding = PaddingMode.PKCS7; Aes.IV = IV; using (ICryptoTransform cTransform = Aes.CreateDecryptor()){ byte[] resultArray = cTransform.TransformFinalBlock(toDecryptArray, 0, toDecryptArray.Length); Aes.Clear(); return UTF8Encoding.UTF8.GetString(resultArray, 0, resultArray.Length); } } 

Answer 2

Generally, you should always have objects that implement IDisposable, such as AesCryptoServiceProvider. Usually they use unmanaged resources that are not cleared by Garbagecollector. Therefore, instead of

 AesCryptoServiceProvider Aes = new AesCryptoServiceProvider(); 

records

 using (AesCryptoServiceProvider Aes = new AesCryptoServiceProvider()) { } 

Answer 3

• Drop Aes into the finally {} block, so it will be deleted even if an exception occurs.

• Remove ICryptoTransform . Since ICryptoTransform is IDisposable , it also transfers it to the using statement

   public static string Decrypt(string cipherText) { AesCryptoServiceProvider Aes; try { //Decrypt: byte[] keyArray; byte[] toDecryptArray = Convert.FromBase64String(cipherText); keyArray = UTF8Encoding.UTF8.GetBytes(key); Aes = new AesCryptoServiceProvider(); Aes.Key = keyArray; Aes.Mode = CipherMode.CBC; Aes.Padding = PaddingMode.PKCS7; Aes.IV = IV; using (ICryptoTransform cTransform = Aes.CreateDecryptor()) { byte[] resultArray = cTransform.TransformFinalBlock(toDecryptArray, 0, toDecryptArray.Length); Aes.Clear(); return UTF8Encoding.UTF8.GetString(resultArray, 0, resultArray.Length); } } catch (Exception ex) { return "FAILED:*" + cipherText + "*" + ex.Message; } finally { Aes.Dispose(); } } 

Answer 4

As stated elsewhere, objects that implement IDisposable do not immediately collect garbage. If you do not call Dispose on a disposable object, either explicitly or wrapping it in use, this object will take longer to collect garbage.

If you use a try, you should consider declaring such variables outside of try and implement deletion in the finally block. In particular, with AesCryptoServiceProvider, where you want to make sure that the Clear () method is launched even if an error occurs, since using it will not do this for you.

 public static string Decrypt(string cipherText) { string decryptedMessage = null; AesCryptoServiceProvider Aes = null; ICryptoTransform cTransform = null; try { //Decrypt: byte[] keyArray = UTF8Encoding.UTF8.GetBytes(key); byte[] toDecryptArray = Convert.FromBase64String(cipherText); AesCryptoServiceProvider Aes = new AesCryptoServiceProvider(); Aes.Key = keyArray; Aes.Mode = CipherMode.CBC; Aes.Padding = PaddingMode.PKCS7; Aes.IV = IV; ICryptoTransform cTransform = Aes.CreateDecryptor(); byte[] resultArray = cTransform.TransformFinalBlock(toDecryptArray, 0, toDecryptArray.Length); decryptedMessage = UTF8Encoding.UTF8.GetString(resultArray, 0, resultArray.Length); } catch (Exception ex) { decryptedMessage = "FAILED:*" + cipherText + "*" + ex.Message; } finally { if (cTransform != null) { cTransform.Dispose(); } if (Aes != null) { Aes.Clear(); Aes.Dispose(); } } return decryptedMessage; } 

You should also consider allowing an exception by excluding the catch block and saving it, and handling it outside of this method.

You can also return bool for success / failure and pass your decrypted string using out. Thus, you will not confuse your mistakes with the contents of your message:

 public bool string Decrypt(string cipherText, out string decryptedMessage) { bool succeeded = false; decryptedMessage = null; AesCryptoServiceProvider Aes = null; ICryptoTransform cTransform = null; try { //Decrypt: byte[] keyArray = UTF8Encoding.UTF8.GetBytes(key); byte[] toDecryptArray = Convert.FromBase64String(cipherText); AesCryptoServiceProvider Aes = new AesCryptoServiceProvider(); Aes.Key = keyArray; Aes.Mode = CipherMode.CBC; Aes.Padding = PaddingMode.PKCS7; Aes.IV = IV; ICryptoTransform cTransform = Aes.CreateDecryptor(); byte[] resultArray = cTransform.TransformFinalBlock(toDecryptArray, 0, toDecryptArray.Length); decryptedMessage = UTF8Encoding.UTF8.GetString(resultArray, 0, resultArray.Length); succeeded = true; } catch (Exception ex) { decryptedMessage = "FAILED:*" + cipherText + "*" + ex.Message; } finally { if (cTransform != null) { cTransForm.Dispose(); } if (Aes != null) { Aes.Clear(); Aes.Dispose(); } } return succeeded; }