Clipboard.GetData performs improper validation

[kevingosse]

.NET version

.NET 9

Did it work in .NET Framework?

Not tested/verified

Did it work in any of the earlier releases of .NET Core or .NET 5+?

No response

Issue description

I ran into a crash caused by an AccessViolationException with the following callstack:

at System.String.Ctor(SByte*)
   at System.Windows.Forms.DataObject+Composition+NativeToWinFormsAdapter.<GetDataFromHGLOBAL>g__ReadStringFromHGLOBAL|11_0(Windows.Win32.Foundation.HGLOBAL, Boolean)
   at System.Windows.Forms.DataObject+Composition+NativeToWinFormsAdapter.GetDataFromHGLOBAL(Windows.Win32.Foundation.HGLOBAL, System.String)
   at System.Windows.Forms.DataObject+Composition+NativeToWinFormsAdapter.<GetObjectFromDataObject>g__TryGetHGLOBALData|12_1(Windows.Win32.System.Com.IDataObject*, System.String, Boolean ByRef)
   at System.Windows.Forms.DataObject+Composition+NativeToWinFormsAdapter.GetObjectFromDataObject(Windows.Win32.System.Com.IDataObject*, System.String, Boolean ByRef)
   at System.Windows.Forms.DataObject+Composition+NativeToWinFormsAdapter.System.Windows.Forms.IDataObject.GetData(System.String, Boolean)
   at System.Windows.Forms.DataObject+Composition.System.Windows.Forms.IDataObject.GetData(System.String, Boolean)
   at System.Windows.Forms.DataObject.GetData(System.String, Boolean)
   at System.Windows.Forms.DataObject.GetData(System.String)

After investigation, it turns out that another application stored a badly formatted string in the clipboard (the null-terminator was missing), and Clipboard.GetData converts the data into a string without validation or size limit. It directly gives the pointer to the string constructor, which will read the memory past the limit until it finds a random \0 or triggers an access violation (whichever comes first).

The clipboard API should either:

• Limit the size of the string with a call to GlobalSize (I believe this is what most applications do)
• Inspect the buffer and throw an exception if it contains no null-terminator

AccessViolationException is not catchable, so applications that use Clipboard.GetData have currently no way to protect themselves from malformed data.

Steps to reproduce

Sample app:

using System.ComponentModel;
using System.Runtime.InteropServices;
using System.Text;

namespace ClipboardCorruptionTest;

internal class Program
{
    [STAThread]
    static void Main(string[] args)
    {
        if (args.Length >= 2)
        {
            if (args[0] == "set")
            {
                var nullTerminator = args[1] == "true";

                var rtfString = @"{\rtf1\ansi\deff0{\fonttbl{\f0 Arial;}}\fs24 Bad-RTF}";

                SetClipboard(rtfString, nullTerminator);

                Console.WriteLine($"Stored a string of length {rtfString.Length} with nullTerminator={nullTerminator}");
            }

            return;
        }

        // Create some memory traffic
        var ptrs = new nint[100000];
        var random = new Random();
        for (int i = 0; i < 100000; i++)
        {
            var size = random.Next(1, 1000);
            ptrs[i] = Marshal.AllocHGlobal(size);
            new Span<byte>((void*)ptrs[i], size).Fill((byte)'x');            
        }

        foreach (var ptr in ptrs)
        {
            Marshal.FreeHGlobal(ptr);
        }

        bool hasRtf = Clipboard.ContainsText(TextDataFormat.Rtf);

        if (!hasRtf)
        {
            Console.WriteLine("No RTF data on clipboard");
            return;
        }

        var result = Clipboard.GetText(TextDataFormat.Rtf);
        
        Console.WriteLine($"Retrieved RTF data of length {result.Length}");
        Console.WriteLine(result);
    }

    private static void SetClipboard(string rtfString, bool nullTerminator)
    {
        var rtfBytes = Encoding.ASCII.GetBytes(rtfString);

        const uint GMEM_MOVEABLE = 0x0002;

        uint cfRtf = RegisterClipboardFormat(DataFormats.Rtf);
        if (cfRtf == 0) throw new Win32Exception(Marshal.GetLastWin32Error(), "RegisterClipboardFormat failed.");

        if (!OpenClipboard(IntPtr.Zero))
            throw new Win32Exception(Marshal.GetLastWin32Error(), "OpenClipboard failed.");

        IntPtr hMem;

        try
        {
            if (!EmptyClipboard())
                throw new Win32Exception(Marshal.GetLastWin32Error(), "EmptyClipboard failed.");

            var length = (UIntPtr)rtfBytes.Length;

            if (nullTerminator)
            {
                length += 1;
            }

            hMem = GlobalAlloc(GMEM_MOVEABLE, length);
            if (hMem == IntPtr.Zero)
                throw new Win32Exception(Marshal.GetLastWin32Error(), "GlobalAlloc failed.");

            IntPtr p = GlobalLock(hMem);
            if (p == IntPtr.Zero)
                throw new Win32Exception(Marshal.GetLastWin32Error(), "GlobalLock failed.");

            try
            {
                Marshal.Copy(rtfBytes, 0, p, rtfBytes.Length);
            }
            finally
            {
                GlobalUnlock(hMem);
            }

            if (SetClipboardData(cfRtf, hMem) == IntPtr.Zero)
            {
                int err = Marshal.GetLastWin32Error();
                GlobalFree(hMem);
                throw new Win32Exception(err, "SetClipboardData(RTF) failed.");
            }
        }
        finally
        {
            CloseClipboard();
        }
    }

    [DllImport("user32.dll", SetLastError = true)]
    static extern bool OpenClipboard(IntPtr hWndNewOwner);
    [DllImport("user32.dll", SetLastError = true)]
    static extern bool CloseClipboard();
    [DllImport("kernel32.dll", SetLastError = true)]
    static extern IntPtr GlobalLock(IntPtr hMem);
    [DllImport("kernel32.dll", SetLastError = true)]
    static extern bool GlobalUnlock(IntPtr hMem);
    [DllImport("kernel32.dll", SetLastError = true)]
    static extern IntPtr GlobalAlloc(uint uFlags, UIntPtr dwBytes);
    [DllImport("kernel32.dll", SetLastError = true)]
    static extern IntPtr GlobalFree(IntPtr hMem);
    [DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
    static extern uint RegisterClipboardFormat(string lpszFormat);
    [DllImport("user32.dll", SetLastError = true)]
    static extern IntPtr SetClipboardData(uint uFormat, IntPtr hMem);
    [DllImport("user32.dll", SetLastError = true)]
    static extern bool EmptyClipboard();
}

Run the app with arguments set true or set false to put a string in the clipboard, respectively with or without a null terminator. Then run without arguments to read it.

> ClipboardTest.exe set true
Stored a string of length 53 with nullTerminator=True

> ClipboardTest.exe
Retrieved RTF data of length 53
{\rtf1\ansi\deff0{\fonttbl{\f0 Arial;}}\fs24 Bad-RTF}

> ClipboardTest.exe set false
Stored a string of length 53 with nullTerminator=False

> ClipboardTest.exe
Retrieved RTF data of length 64
{\rtf1\ansi\deff0{\fonttbl{\f0 Arial;}}\fs24 Bad-RTF}xxxxxxxxxxx

Since the memory is read out of bounds, the results are random. It can take a few tries to see the garbage.

[kevingosse]

Interestingly, ReadUtf8StringFromHGLOBAL is already doing exactly that:

winforms/src/System.Private.Windows.Core/src/System/Private/Windows/Ole/Composition.NativeToManagedAdapter.cs

Lines 187 to 199 in 7be5881

	private static unsafe string ReadUtf8StringFromHGLOBAL(HGLOBAL hglobal)
	{
	void* buffer = PInvokeCore.GlobalLock(hglobal);
	try
	{
	int size = (int)PInvokeCore.GlobalSize(hglobal);
	return Encoding.UTF8.GetString((byte*)buffer, size - 1);
	}
	finally
	{
	PInvokeCore.GlobalUnlock(hglobal);
	}
	}

It looks like it's just an oversight on ReadStringFromHGLOBAL.

[JeremyKuhne]

Thanks @kevingosse, looking into this. Some of the tracking is internal, will loop back here with updates.

[Zheng-Li01]

@JeremyKuhne, verified the issue with .NET SDK 10.0.100-rtm.25523.111 test pass build, for now the string return the length as 0 after executed the .exe set false command as below screenshot. please help confirm if the current result is expected or not?

cc// @kevingosse

[JeremyKuhne]

@Zheng-Li01 yes, when the data is considered corrupted, we return an empty string.