Access Violation with P/Invoke SendMessage and Message.GetLParam

Question

I get an intermittent error when calling Message.GetLParam, getting messages sent between processes. I have two processes, both written in C# (.Net 3.5). I am using the Win32 function SendMessage() to send data from one process (the source) to the other (the target). The target process's main window (it's a Windows Forms app) overrides the WndProc() function to receive messages. The source process locates the other by using the Process.GetProcessesByName() function, then using the Process.MainWindowHandle to get the window handle that I want to send the message to. The code of the source looks like this:

Process[] procs = Process.GetProcessesByName("MyTargetProcess");
if (procs != null 
    && procs.Length > 0)
{
    IntPtr win = procs[0].MainWindowHandle;
    var someData = new Win32.COPYDATASTRUCT   // This is a struct that I defined
    {
    // Initialize fields of the struct
    };
    Win32.SendMessage(win,
        Win32.MyMsgCode,    // my custom message
        IntPtr.Zero,    // wParam = 0
        ref someData);
}

The target process code looks like this:

protected override void WndProc(ref System.Windows.Forms.Message m)
{
    if (m.Msg == Win32.MyMsgCode)
    {
    Win32.COPYDATASTRUCT ds;
        try
        {
            ds = (Win32.COPYDATASTRUCT)m.GetLParam(typeof(Win32.COPYDATASTRUCT));
        }
        catch (Exception ex)
        {
            log.ErrorFormat("Got exception in WndProc", ex);
        }
        // Do something with the message
        ....
}

Win32 is a static class I defined that gets all my P/Invoke definitions. I do not understand why I am catching an AccessViolationException in WndProc. Does anybody have an idea why? and why it only happens some of the time?

Thanks for your attention!

-------------------------------- EDIT ------------------------------------------ Another thing that baffles me: the COPYDATASTRUCT is declared as

public static readonly int WM_COPYDATA = 0x004a;
[StructLayout(LayoutKind.Sequential)]
public struct COPYDATASTRUCT
{
    // Specifies data to be passed to the receiving application.
    public string dwData;
    // Specifies the size, in bytes, of the data pointed to by the lpData member. 
    public int cbData;
    // Pointer to data to be passed to the receiving application. This member can be NULL.                 
    public string lpData;
}

It is initialized like this:

string payload = " some data ";
var someData = new Win32.COPYDATASTRUCT   // This is a struct that I defined
{
    dwData = "bogusData",
    cbData = sizeof(char) * payload.Length,
    lpData = payload
};

And in target code, I always receive dwData = null.

----------------------- 2nd edit -------------------------------------- I just tried with adding the zero terminator, and I still get the error. If I change the marshalling code to do my own marshalling as in

IntPtr pcds = Marshal.AllocHGlobal(Marshal.SizeOf(someData));
Marshal.StructureToPtr(someData, pcds, true);
Win32.SendMessage(win, (uint)Win32.WM_COPYDATA, IntPtr.Zero, pcds);

Then the error happens ALL THE TIME! However, if I repeat the GetLParam() call in the catch block, it succeeds almost all the time, on the 2nd try.

Answer 1

The thing that jumps out at me is that cbData is set incorrectly. You need to account for the zero-terminator at the end of the string.

 cbData = sizeof(char) * (payload.Length+1)

That would certainly explain the error. When you send the message, the WM_COPYDATA marshalling would not copy the zero terminator and so the recipient would then read beyond the end of the buffer into uninitialized values.

I also wonder about sizeof(char). Are you calling the Unicode version of SendMessage? If not then I'd expect to see access violations in the sending code.

You should also be wary that the recipient application opens itself to buffer overruns. Because it ignores the value of cbData and treats the lpData field as a null-terminated pointer it may be possible for an attacker to force your app to execute arbitrary code. To defend against this, you should copy the data, cbData bytes of it, into a byte array and then convert to a string.

The other issue is that you need to declare dwData as UIntPtr. The way you have defined the struct, the recipient code treats dwData as a pointer to a string which will provoke AVs because you have crossed process boundaries.

I'd also point out that cbData is unsigned and should be uint but that is a benign error here.