CWE-401: Missing Release of Memory After Effective Lifetime - C#

Overview

Memory leaks in C# typically involve unmanaged resources (files, database connections, streams, unmanaged memory) rather than managed objects, since the garbage collector handles most memory management. Failing to dispose of IDisposable resources causes resource exhaustion, connection pool depletion, file handle leaks, and denial of service.

Primary Defence: Use using statements or using declarations for all IDisposable resources, implement IDisposable pattern correctly for classes managing unmanaged resources, unsubscribe from events when objects are no longer needed, dispose of resources in finally blocks if using statements aren't available, and avoid static collections that grow unbounded.

Common Vulnerable Patterns

Undisposed Database Connections

public List<User> GetUsers() {
    var conn = new SqlConnection(connectionString);
    var cmd = new SqlCommand("SELECT * FROM users", conn);

    conn.Open();
    var reader = cmd.ExecuteReader();

    var users = new List<User>();
    while (reader.Read()) {
        users.Add(new User(reader.GetString(0)));
    }

    return users;
    // No Dispose() - connection, command, reader all leaked!
}

Why is this vulnerable: Database connections are backed by network sockets and connection pool entries. When Dispose() isn't called, the connection remains in the "in use" state in the pool, even though the method has returned and can't access it anymore. After 10-50 calls (typical pool size), the pool is exhausted and new requests block indefinitely waiting for available connections, causing application-wide denial of service. The garbage collector doesn't help - finalizers run non-deterministically, far too slowly for high-throughput applications. Connection, command, and reader all hold unmanaged resources (native handles, memory buffers) that won't be released until GC finalizes them, which could be minutes or never in a long-running process. This pattern causes the most common production outages in .NET applications.

Undisposed File Streams

public string ReadFile(string path) {
    var stream = new FileStream(path, FileMode.Open);
    var reader = new StreamReader(stream);
    string content = reader.ReadToEnd();
    return content;
    // No Dispose() - file handle leaked!
}

// After many calls, file descriptors exhausted
for (int i = 0; i < 10000; i++) {
    ReadFile($"data_{i}.txt");
    // "Too many open files" error
}

Why is this vulnerable: Each FileStream opens a file handle from the operating system's limited pool (typically 1024-4096 per process). Without calling Dispose(), the handle remains open until the garbage collector finalizes the object - but finalization is non-deterministic and slow. A web application handling hundreds of requests per second can exhaust all file handles in seconds. Open file handles also prevent file deletion, modification, and can cause sharing violations when other processes try to access the files. Even though .NET has a garbage collector, it only manages memory - unmanaged resources like file handles, sockets, and database connections must be explicitly released via Dispose().

Event Handler Leaks

public class SubscriberManager {
    private List<Subscriber> subscribers = new List<Subscriber>();

    public void AddSubscriber(Subscriber sub) {
        subscribers.Add(sub);

        // Subscribe to event
        SomeStaticPublisher.DataReceived += sub.HandleData;
    }

    public void RemoveSubscriber(Subscriber sub) {
        subscribers.Remove(sub);
        // Event handler NOT unsubscribed - sub can't be GC'd!
    }
}

// Usage in ASP.NET Core
public class MyController : Controller {
    public MyController() {
        // Subscribe to static event
        GlobalEvents.OnUpdate += HandleUpdate;
        // Controller instances never unsubscribe
        // Each request creates new controller -> all leaked!
    }

    private void HandleUpdate(object sender, EventArgs e) { }
}

Why is this vulnerable: When an object subscribes to an event (especially on static/long-lived publishers), the publisher holds a delegate reference to the subscriber's method, preventing garbage collection of the subscriber. Even when RemoveSubscriber() removes the subscriber from the list, the event subscription keeps it alive. In ASP.NET applications where controllers are created per request, failing to unsubscribe means every controller instance ever created remains in memory indefinitely. After thousands of requests, memory is exhausted. This pattern is particularly insidious because the leak isn't obvious - the subscriber appears unused but is kept alive through the event delegate chain.

Secure Patterns

Using Statements

public async Task<string> ReadFileAsync(string path) {
    // using statement: calls Dispose() automatically
    using (var stream = new FileStream(path, FileMode.Open))
    using (var reader = new StreamReader(stream)) {
        return await reader.ReadToEndAsync();
    }
    // stream and reader disposed here, even on exception
}

// C# 8.0+ using declarations (simpler syntax)
public List<User> GetUsers() {
    using var conn = new SqlConnection(connectionString);
    using var cmd = new SqlCommand("SELECT * FROM users", conn);

    conn.Open();
    using var reader = cmd.ExecuteReader();

    var users = new List<User>();
    while (reader.Read()) {
        users.Add(new User(reader.GetString(0)));
    }

    return users;
    // All resources disposed at end of method scope
}

Why this works: The using statement automatically calls Dispose() on IDisposable objects when they go out of scope, whether the block exits normally or via exception. The compiler generates a try-finally block that guarantees disposal even if exceptions are thrown. This eliminates the most common source of resource leaks - forgetting to dispose in all code paths, especially error handlers. Using declarations (C# 8.0+) provide cleaner syntax by disposing at the end of the enclosing scope rather than requiring nested braces, making the code more readable while maintaining the same safety guarantees. Any class implementing IDisposable works with this pattern, including streams, database connections, HTTP clients, and custom resources.

Implementing IDisposable Correctly

using System;
using System.Runtime.InteropServices;

public class ResourceHolder : IDisposable {
    // Managed resource
    private SqlConnection connection;

    // Unmanaged resource
    private IntPtr unmanagedBuffer;

    private bool disposed = false;

    public ResourceHolder(string connString) {
        connection = new SqlConnection(connString);
        unmanagedBuffer = Marshal.AllocHGlobal(1024);
    }

    // Public Dispose method
    public void Dispose() {
        Dispose(true);
        GC.SuppressFinalize(this);  // Prevent finalizer from running
    }

    // Protected virtual Dispose for derived classes
    protected virtual void Dispose(bool disposing) {
        if (!disposed) {
            if (disposing) {
                // Dispose managed resources
                connection?.Dispose();
            }

            // Free unmanaged resources
            if (unmanagedBuffer != IntPtr.Zero) {
                Marshal.FreeHGlobal(unmanagedBuffer);
                unmanagedBuffer = IntPtr.Zero;
            }

            disposed = true;
        }
    }

    // Finalizer - only if class has unmanaged resources
    ~ResourceHolder() {
        Dispose(false);
    }
}

// Usage
using (var resource = new ResourceHolder(connString)) {
    // Use resource
}
// Dispose() called automatically

Why this works: This implements the standard IDisposable pattern correctly. The public Dispose() method releases both managed and unmanaged resources and suppresses finalization (since resources are already cleaned up). The protected virtual Dispose(bool) allows derived classes to extend cleanup. The disposing parameter indicates whether we're in a Dispose() call (true) or finalizer (false) - managed resources should only be accessed when disposing is true because they might already be finalized. The finalizer provides a safety net for unmanaged resources if Dispose() is never called, but it's only needed if the class directly holds unmanaged resources (most classes don't need a finalizer). The disposed flag prevents double-disposal. This pattern ensures resources are released deterministically via Dispose() while providing fallback cleanup via finalization.

Unsubscribing from Events

public class EventSubscriber : IDisposable {
    private readonly EventHandler<DataEventArgs> handler;

    public EventSubscriber() {
        handler = HandleData;
        GlobalEvents.DataReceived += handler;
    }

    private void HandleData(object sender, DataEventArgs e) {
        // Process data
    }

    public void Dispose() {
        // Unsubscribe from event
        GlobalEvents.DataReceived -= handler;
    }
}

// Usage
using (var subscriber = new EventSubscriber()) {
    // Subscriber receives events
}
// Automatically unsubscribed and eligible for GC

// ASP.NET Core controller with proper cleanup
public class MyController : Controller, IDisposable {
    public MyController() {
        GlobalEvents.OnUpdate += HandleUpdate;
    }

    private void HandleUpdate(object sender, EventArgs e) { }

    protected override void Dispose(bool disposing) {
        if (disposing) {
            GlobalEvents.OnUpdate -= HandleUpdate;
        }
        base.Dispose(disposing);
    }
}

Why this works: Explicitly unsubscribing from events breaks the reference from the event publisher to the subscriber, allowing the subscriber to be garbage collected. Implementing IDisposable and unsubscribing in Dispose() ensures cleanup happens deterministically. In ASP.NET applications, controllers are automatically disposed at the end of each request, so overriding Dispose() to unsubscribe ensures each controller instance is eligible for GC. Storing the delegate in a field (rather than using a lambda) is important for unsubscription - you must pass the exact same delegate instance to -= that you passed to +=. This pattern is essential for any object that subscribes to events on longer-lived publishers, especially static events or application-scoped services.

Weak Event Pattern

using System;

public class WeakEventManager<TEventArgs> where TEventArgs : EventArgs {
    private readonly List<WeakReference<EventHandler<TEventArgs>>> handlers 
        = new List<WeakReference<EventHandler<TEventArgs>>>();

    public void AddHandler(EventHandler<TEventArgs> handler) {
        handlers.Add(new WeakReference<EventHandler<TEventArgs>>(handler));
    }

    public void RemoveHandler(EventHandler<TEventArgs> handler) {
        handlers.RemoveAll(wr => {
            if (wr.TryGetTarget(out var target)) {
                return target == handler;
            }
            return true;  // Remove dead references
        });
    }

    public void RaiseEvent(object sender, TEventArgs args) {
        foreach (var weakRef in handlers.ToList()) {
            if (weakRef.TryGetTarget(out var handler)) {
                handler(sender, args);
            }
        }

        // Clean up dead references periodically
        handlers.RemoveAll(wr => !wr.TryGetTarget(out _));
    }
}

// Usage
public class Publisher {
    private readonly WeakEventManager<DataEventArgs> eventManager 
        = new WeakEventManager<DataEventArgs>();

    public event EventHandler<DataEventArgs> DataReceived {
        add => eventManager.AddHandler(value);
        remove => eventManager.RemoveHandler(value);
    }

    protected virtual void OnDataReceived(DataEventArgs e) {
        eventManager.RaiseEvent(this, e);
    }
}

// Subscribers can be GC'd even if they forget to unsubscribe

Why this works: Weak event managers use WeakReference to hold event handler delegates, allowing subscribers to be garbage collected even if they remain subscribed. When a subscriber is no longer strongly referenced elsewhere, the GC reclaims it, and the weak reference in the event manager becomes invalid. The next time the event is raised, dead references are detected (TryGetTarget returns false) and cleaned up. This prevents event subscription leaks without requiring explicit unsubscription. The pattern is particularly useful for framework-level components or libraries where you can't control subscriber cleanup, or in WPF/XAML scenarios with complex object graphs. However, it adds overhead (WeakReference allocations, cleanup logic) and shouldn't be used everywhere - only where long-lived publishers subscribe to short-lived subscribers.