Checkmarx C# - Improper_Resource_Shutdown_or_Release

Question

I have a code block that looks like this:

        StringBuilder csvData = new StringBuilder();
        StreamWriter fileWriter = new StreamWriter(filepath.ToString());
        try
        {
            csvData.Append(Constants.CSVQuote);
            csvData.Append(Constants.Code);
            csvData.Append(Constants.CSVDelimiter);
            csvData.Append(Constants.Description);
            csvData.Append(Constants.CSVDelimiter);
            csvData.Append(Constants.Comments);
            csvData.Append(Constants.CSVQuote);

            fileWriter.WriteLine(csvData);
            csvData.Clear();
            for( int i=0; i<StatusEntities.Count(); i++)
            {
                csvData.Append(Constants.CSVQuote);
                csvData.Append(StatusEntities[i].Code != null ? Common.ToCSVString(StatusEntities[i].Code) : string.Empty);
                csvData.Append(Constants.CSVDelimiter);
                csvData.Append(StatusEntities[i].Description != null ? Common.ToCSVString(StatusEntities[i].Description) : string.Empty);
                csvData.Append(Constants.CSVDelimiter);
                csvData.Append(StatusEntities[i].Comments != null ? Common.ToCSVString(StatusEntities[i].Comments) : string.Empty);
                csvData.Append(Constants.CSVQuote);
                fileWriter.WriteLine(csvData);
                csvData.Clear();
            }
        }
        catch
        {
            throw;
        }
        finally
        {
            fileWriter.Close();
            csvData = null;
        }

I keep getting back Improper_Resource_Shutdown_or_Release vulnerability. I have tried using Automatic Implicit release using try-with-resource but that also didn't work. Can you please provide some suggestions on how to fix this kind of vulnerability?

Answer 1

Checkmarx is flagging your code because you aren't disposing of the new resource allocations properly for the StringBuilder and StreamWriter instances. You could move the resource allocation to the try { } block and then inside the finally { } block add the following code:

cvsData.Dispose(); 
fileWriter.Dispose();

The reason you need to move the resource allocations to the try { } block is because an exception can be thrown during allocation. Especially the following line:

StreamWriter fileWriter = new StreamWriter(filepath.ToString()); 

could throw an exception in the event the variable 'filepath' is invalid, for example.

Alternatively, you can wrap new resource allocations inside using(...) { } blocks such as the following:

using(var csvData = new StringBuilder())
{
    // work with csvData instance here
}

This ensures the resource is disposed of properly when it is out of scope (when you are done using it) without you needing to explicitly dispose of it.