Still A "Multiple Dispose" Issue, Even Though Handled

107 views Asked by At

In some of my projects I've been using a pair of tried-and-true data encryption/decryption methods (the encryption method is pasted below). But I have always been dogged by this nagging CA2202 warning ("Do not dispose objects multiple times"), about the memoryStream object. I believe I handle this in the appropriate manner, but I still get the warning whenever I run an analysis in Visual Studio. It's never thrown exceptions in production code, but I would still like to get rid of the warning once and for all. Is that possible? Or should I just ignore it? Thanks in advance.

public static string Encrypt(string clearText, string passPhrase, string saltValue)
{
    byte[] clearTextBytes = Encoding.UTF8.GetBytes(clearText);
    byte[] saltValueBytes = Encoding.UTF8.GetBytes(saltValue);

    Rfc2898DeriveBytes passPhraseDerviedBytes = new Rfc2898DeriveBytes(passPhrase, saltValueBytes);
    byte[] keyBytes = passPhraseDerviedBytes.GetBytes(32);
    byte[] initVectorBytes = passPhraseDerviedBytes.GetBytes(16);

    RijndaelManaged symmetricKey = new RijndaelManaged() { Mode = CipherMode.CBC };
    ICryptoTransform encryptor = symmetricKey.CreateEncryptor(keyBytes, initVectorBytes);

    byte[] cipherTextBytes = null;
    MemoryStream memoryStream = null;
    try
    {
        memoryStream = new MemoryStream();
        using (CryptoStream cryptoStream = new CryptoStream(memoryStream, encryptor, CryptoStreamMode.Write))
        {
            cryptoStream.Write(clearTextBytes, 0, clearTextBytes.Length);
            cryptoStream.FlushFinalBlock();
            cipherTextBytes = memoryStream.ToArray();
        }
    }
    finally
    {
        if (memoryStream != null)
        {
            memoryStream.Dispose();
        }
    }

    return Convert.ToBase64String(cipherTextBytes);
}
2

There are 2 answers

5
Guy On BEST ANSWER

It's because CryptoStream closes the memoryStream

You are using the constructor

public CryptoStream(Stream stream, ICryptoTransform transform, CryptoStreamMode mode)
    : this(stream, transform, mode, false) {
}

which calls

public CryptoStream(Stream stream, ICryptoTransform transform, CryptoStreamMode mode, bool leaveOpen) {
    _stream = stream;
    _leaveOpen = leaveOpen;
    //...
}

_leaveOpen and _stream are later used in Dispose

protected override void Dispose(bool disposing) {
    try {
        if (!_leaveOpen) {
            _stream.Close();
        }
        //...
    }
}

You can remove the memoryStream.Dispose();, or pass true as parameter to CryptoStream constructor

using (CryptoStream cryptoStream = new CryptoStream(memoryStream, encryptor, CryptoStreamMode.Write, true)) { }

github source code reference

0
Camilo Terevinto On

The problem is that the call to CryptoStream.Dispose can call dispose on the given stream:

protected override void Dispose(bool disposing) 
{
    try 
    {
        if (disposing) 
        {
            if (!_finalBlockTransformed) 
            {
                FlushFinalBlock();
            }
            if (!_leaveOpen) 
            {
                _stream.Close();
            }
        }
    }
    ...
}

If you use the constructor that takes 4 parameters:

CryptoStream(Stream stream, ICryptoTransform transform, CryptoStreamMode mode, bool leaveOpen)

The last argument determines whether the stream is Closed or not. Calling Close() , in turn, by default also calls Dispose:

public virtual void Close()
{
    /* These are correct, but we'd have to fix PipeStream & NetworkStream very carefully.
    Contract.Ensures(CanRead == false);
    Contract.Ensures(CanWrite == false);
    Contract.Ensures(CanSeek == false);
    */

    Dispose(true);
    GC.SuppressFinalize(this);
}

So, it seems that the check cannot correctly determine whether the specific Stream implementation will be Disposed or not, and falls back to think it will be- which is this case.

Note, however, that disposing twice, once or zero times a MemoryStream doesn't matter a lot in most cases.