Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive on stream leak in HttpClient StreamContent and CryptoStream? #187

Open
akatakritos opened this issue Nov 4, 2022 · 1 comment

Comments

@akatakritos
Copy link

Found this in a real project. We're uploading files to a rather interesting HTTP API that expects its content to be base64 encoded. Rather than loading large files into memory and running Convert.ToBase64String I found some guidance on how to use CryptoStream to create a stream that base64 encodes on the fly.

using System.Security.Cryptography;

await MakeApiCall();

async Task MakeApiCall()
{
    using var client = new HttpClient(); // get this from IHttpClientFactory in the real world
    
    using var request = new HttpRequestMessage(HttpMethod.Post, "https://example.com/")
    {
        Content = GetBase64FileContent()
    };


    using var response = await client.SendAsync(request);
    var responseContent = await response.Content.ReadAsStringAsync();
    Console.WriteLine(responseContent);
}

StreamContent GetBase64FileContent()
{
    // Leak reported here, but I think StreamContent will Dispose CryptoStream which will Dispose the FileStream
    return new StreamContent(new CryptoStream(File.OpenRead("C:\\temp\\test.txt"), new ToBase64Transform(), CryptoStreamMode.Read));
}

Ran with docker:

docker run -v /Users/matt.burke/projects/ConsoleApp3/ConsoleApp3/bin/Debug/net6.0:/infersharp/binary_path --rm mcr.microsoft.com/infersharp:v1.4 /bin/bash -c "./run_infersharp.sh binary_path; cp infer-out/report.txt /infersharp/binary_path/report.txt"

Output

/Users/matt.burke/projects/ConsoleApp3/ConsoleApp3/Program.cs:23: error: Pulse Resource Leak
  Resource dynamically allocated by constructor System.Security.Cryptography.CryptoStream() on line 23 is not closed after the last access at line 23, column 5.


Found 1 issue
                Issue Type(ISSUED_TYPE_ID): #
  Pulse Resource Leak(PULSE_RESOURCE_LEAK): 1

Reading through all the framework code, I think that when HttpRequestMessage is disposed, it will dispose the StreamContent which will dispose the CryptoStream which will dispose the FileStream.

@matjin
Copy link
Contributor

matjin commented Nov 4, 2022

Thanks for posting this issue! It looks like you're correct -- the Httprequest message/streamcontent/cryptostream all in turn dispose of the underlying. We'll add this into our next batch of infer backend model updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants