Buffer Overflow

ID

csharp.buffer_overflow

Severity

high

Resource

Memory Access

Language

CSharp

Tags

CWE:120, NIST.SP.800-53:SI-16, PCI-DSS:6.5.1

Description

C# runs under the Common Language Runtime (CLR), where the compiler and runtime handle memory and provide checks to prevent classical buffer overflow attacks.

However, the language provides mechanisms to "escape" from managed code due to the need for interoperability with system code written in other languages. This could lead to memory corruption and other issues, such as type safety violations. One example is the unsafe keyword.

Rationale

This detector considers cases that could lead to memory block corruption:

  1. Pointer arithmetic in an unsafe context. The detector forbids dereferencing a pointer that is changed with pointer arithmetic in an unsafe context. In unmanaged code, the developer must control when the pointer escapes the proper memory area. This is error-prone and may lead to buffer overflow vulnerabilities.

    // VULNERABLE to buffer overflow, when s.Length > 10
unsafe void BufferOverflow(string s)
{
    char* ptr = stackalloc char[10];
    foreach (var c in s)
    {
      *ptr++ = c; // classic buffer overflow !
    }
}

  2. Calls to dangerous methods in System.Runtime.InteropServices.Marshal that may result in memory leaks and other memory issues. For example, the detector forbids calls to ReleaseComObject().

    using System;
using Outlook = Microsoft.Office.Interop.Outlook;
using System.Runtime.InteropServices;

static void SendEmail(string recipient, string subject, string body)
{
    Outlook.Application outlookApp = null;
    Outlook.MailItem mailItem = null;

    try
    {
        // Start Outlook application
        outlookApp = new Outlook.Application();

        // Create a new mail item
        mailItem = (Outlook.MailItem) outlookApp.CreateItem(Outlook.OlItemType.olMailItem);

        // Set properties of the mail item
        mailItem.To = recipient;
        mailItem.Subject = subject;
        mailItem.Body = body;

        mailItem.Send();
    }
    catch (Exception ex)
    {
        Console.WriteLine("Error: " + ex.Message);
    }
    finally
    {
        // Release COM objects *in reverse order of creation*
        // This is error prone and may lead to memory leaks
        if (mailItem != null)
        {
            Marshal.ReleaseComObject(mailItem);
            mailItem = null;
        }

        if (outlookApp != null)
        {
            Marshal.ReleaseComObject(outlookApp);
            outlookApp = null;
        }

        // Force garbage collection to finalize released COM objects
        GC.Collect();
        GC.WaitForPendingFinalizers();
    }
}

    The following table shows some dangerous methods in the System.Runtime.InteropServices.Marshal class:

    Method Purpose Risks

    AllocHGlobal

    Allocates unmanaged memory.

    Memory leaks if not freed, unsafe pointer usage.

    FreeHGlobal

    Frees memory allocated with AllocHGlobal.

    Double free or freeing invalid pointers can crash app.

    StructureToPtr

    Marshals a managed struct to unmanaged memory.

    Can corrupt memory if struct is not blittable or if used with wrong pointer.

    PtrToStructure

    Converts unmanaged memory to a managed struct.

    Invalid pointer or incorrect struct layout can lead to undefined behavior.

    ReadByte / ReadInt32 / ReadIntPtr

    Reads a value from unmanaged memory.

    Can read from invalid or unaligned memory, causing crashes.

    WriteByte / WriteInt32 / WriteIntPtr

    Writes a value to unmanaged memory.

    Overwrites memory, can corrupt data or crash process.

    Copy

    Copies data between managed and unmanaged memory.

    Buffer overflows if size is miscalculated.

    ReleaseComObject

    Decrements reference count of a COM object.

    Can cause crashes if released too many times or still in use.

    FinalReleaseComObject

    Releases all references to a COM object.

    Safer than ReleaseComObject, but still dangerous if object is reused elsewhere.

    GetFunctionPointerForDelegate

    Gets a function pointer for a delegate.

    Can crash or corrupt memory if the delegate is garbage collected while in use.

    GetDelegateForFunctionPointer

    Converts a native function pointer to a delegate.

    Mismatch in function signatures causes undefined behavior.

  3. Attributes with unsafe memory layouts. Annotating a C# type with [StructLayout(LayoutKind.Explicit)] is typically done when trying to emulate C unions. Type fields annotated with the [FieldOffset] attribute may also result in a memory layout that violates type safety because the CLR will not verify that memory references are within the correct range.

    using System.Runtime.InteropServices;

// VULNERABLE
// Emulates a C union with memory layout
[StructLayout(LayoutKind.Explicit, Size = 16)]
internal unsafe struct IotSensor
{
    [FieldOffset(0)]
    public fixed byte Bytes[16];

    [FieldOffset(0)]
    public UInt32 Repetitions;

    [FieldOffset(4)]
    public Int16 Amplitude;

    [FieldOffset(6)]
    public Int16 Offset;

    [FieldOffset(8)]
    public Int16 Gain;

    [FieldOffset(10)]
    public UInt16 Selection;

    [FieldOffset(12)]
    public UInt32 Step;

    public void ReadSensor(byte[] data)
    {
        for (int i = 0; i < 16; i++)
        {
            Bytes[i] = data[i];
        }
    }
}

Remediation

  1. Avoid using pointer arithmetic in unsafe contexts if possible. If the reported code cannot be converted to managed code for performance or other reasons, analyze it for potential buffer overflow issues and mute the violation.

  2. Never call the dangerous methods reported in the System.Runtime.InteropServices.Marshal class. For example, replace Marshal.ReleaseComObject() with FinalReleaseComObject().

    This will at least crash the program quicker in the event of a memory corruption error, as it releases all references to the COM object in one call (better than manually looping ReleaseComObject). But it is still risky if other parts of your code (or the interop layer itself) also reference the COM object.

    When possible, prefer SafeHandle or Span<T>-based interop introduced in newer .NET versions.

  3. Never use StructLayout(LayoutKind.Explicit) to "emulate" C unions unless it is absolutely necessary, for example for interop with native code) and thoroughly test on all target platforms. Avoid overlapping reference types: prefer LayoutKind.Sequential over LayoutKind.Explicit for most use cases, and always validate binary layouts with tests.

After carefully checking that the code is safe, you can mute the reported vulnerability.

Configuration

This detector has a dangerousMarshallMethods property with the list of dangerous methods in the System.Runtime.InteropServices.Marshal class.

dangerousMarshallMethods:
    - ReleaseComObject
    - AllocHGlobal
    - FreeHGlobal

By default, the three methods are marked as dangerous. You may uncomment other methods (or all) if you want calls to them to be reported as vulnerabilities.

References