Andrew Truckle
Andrew Truckle

Reputation: 19147

Object 'iCrypt' can be disposed more than once

I have seen several questions about this issue. For example:

"Object can be disposed of more than once" error

But I can't quite work out what I should be doing with my code. The warning I have from code analysis is:

1>D:\My Programs\2017\PTSTools\PTSTools\PTSTools.cs(81): warning CA2202: Microsoft.Usage : Object 'iCrypt' can be disposed more than once in method 'PTSToolsClass.Crypt(string, string, bool)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 81

The method in question:

private string Crypt(string strData, string strPassword, bool bEncrypt)
{
    byte[] u8Salt = new byte[] { ... };

    PasswordDeriveBytes iPass = new PasswordDeriveBytes(strPassword, u8Salt);

    Rijndael iAlg = Rijndael.Create();
    iAlg.Key = iPass.GetBytes(32);
    iAlg.IV = iPass.GetBytes(16);

    ICryptoTransform iTrans = (bEncrypt) ? iAlg.CreateEncryptor() : iAlg.CreateDecryptor();

    MemoryStream iMem = new MemoryStream();
    CryptoStream iCrypt = new CryptoStream(iMem, iTrans, CryptoStreamMode.Write);

    byte[] u8Data;
    if (bEncrypt)
        u8Data = Encoding.Unicode.GetBytes(strData);
    else
        u8Data = Convert.FromBase64String(strData);

    try
    {
        iCrypt.Write(u8Data, 0, u8Data.Length);
        iCrypt.Close();
        if (bEncrypt)
            return Convert.ToBase64String(iMem.ToArray());
        else
            return Encoding.Unicode.GetString(iMem.ToArray());
    }
    catch
    {
        return null;
    }
    finally
    {
        iCrypt.Close();
    }
}

Can someone please explain why I get this warning raised for the finally line of code?

Thank you.

Update:

Based on comments I changed it to:

private string Crypt(string strData, string strPassword, bool bEncrypt)
{
    byte[] u8Salt = new byte[] { ... };

    PasswordDeriveBytes iPass = new PasswordDeriveBytes(strPassword, u8Salt);

    Rijndael iAlg = Rijndael.Create();
    iAlg.Key = iPass.GetBytes(32);
    iAlg.IV = iPass.GetBytes(16);

    ICryptoTransform iTrans = (bEncrypt) ? iAlg.CreateEncryptor() : iAlg.CreateDecryptor();

    MemoryStream iMem = new MemoryStream();
    using (CryptoStream iCrypt = new CryptoStream(iMem, iTrans, CryptoStreamMode.Write))
    {
         byte[] u8Data;
        if (bEncrypt)
            u8Data = Encoding.Unicode.GetBytes(strData);
        else
            u8Data = Convert.FromBase64String(strData);

        try
        {
            iCrypt.Write(u8Data, 0, u8Data.Length);
            iCrypt.Close();
            if (bEncrypt)
                return Convert.ToBase64String(iMem.ToArray());
            else
                return Encoding.Unicode.GetString(iMem.ToArray());
        }
        catch
        {
            return null;
        }
    }
}

But I now have these warnings:

1>D:\My Programs\2017\PTSTools\PTSTools\PTSTools.cs(77): warning CA2202: Microsoft.Usage : Object 'iCrypt' can be disposed more than once in method 'PTSToolsClass.Crypt(string, string, bool)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 77
1>D:\My Programs\2017\PTSTools\PTSTools\PTSTools.cs(77): warning CA2202: Microsoft.Usage : Object 'iMem' can be disposed more than once in method 'PTSToolsClass.Crypt(string, string, bool)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 77

It now does not like the catch block.

Update:

Understood:

   private string Crypt(string strData, string strPassword, bool bEncrypt)
    {
        byte[] u8Salt = new byte[] { 0x26, 0x19, 0x81, 0x4E, 0xA0, 0x6D, 0x95, 0x34, 0x26, 0x75, 0x64, 0x05, 0xF6 };

        PasswordDeriveBytes iPass = new PasswordDeriveBytes(strPassword, u8Salt);

        Rijndael iAlg = Rijndael.Create();
        iAlg.Key = iPass.GetBytes(32);
        iAlg.IV = iPass.GetBytes(16);

        ICryptoTransform iTrans = (bEncrypt) ? iAlg.CreateEncryptor() : iAlg.CreateDecryptor();

        MemoryStream iMem = new MemoryStream();
        using (CryptoStream iCrypt = new CryptoStream(iMem, iTrans, CryptoStreamMode.Write))
        {
                byte[] u8Data;
            if (bEncrypt)
                u8Data = Encoding.Unicode.GetBytes(strData);
            else
                u8Data = Convert.FromBase64String(strData);

            try
            {
                iCrypt.Write(u8Data, 0, u8Data.Length);
                if (bEncrypt)
                    return Convert.ToBase64String(iMem.ToArray());
                else
                    return Encoding.Unicode.GetString(iMem.ToArray());
            }
            catch
            {
                return null;
            }
        }
    }

Upvotes: 0

Views: 175

Answers (3)

CodeCaster
CodeCaster

Reputation: 151604

Because the finally block of a try-(catch-)finally block is always executed, the analyzer sees this:

try
{
    iCrypt.Close();
}
finally
{
    iCrypt.Close();
}

It then correctly infers that those calls can occur in succession, causing the ObjectDisposedException to be thrown in the second call.

So: you don't need the iCrypt.Close() in the try block, because it'll always be closed in the finally, whether an exception occurs or not.

Upvotes: 2

H.G. Sandhagen
H.G. Sandhagen

Reputation: 822

The message from code analysis is clear. Your code disposes (closes) iCrypt two times. One time inside the try block and one time in the finally block. Remove the line iCrypt.Close(); in the try block (between iCrypt.Write(...) and if(...) and the warning will disappear.

Even better approch is a using block:

using (CryptoStream iCrypt = new CryptoStream(...)) {
   .... // your code without the finally block
}

The using will call iCrypt.Close() regardless whether and exception is thrown inside the block or not.

Upvotes: 1

Andrei Filimon
Andrei Filimon

Reputation: 1188

You are calling iCrypt.Close(); twice, remove the call from inside the try statement and you will be fine.

Upvotes: 1

Related Questions