1

Closed

Bug in RtlGetLastErrorString

description

You have a wrong declaration of RtlGetLastErrorString in your PInvoke signature which leads to heap corruptions.
In your C implementation you return the unicode string pointer which is wrong because .NET assumes that it was allocated with CoTaskMemAlloc and will try to free it which will lead to process terminations.
The fix is easy. Return a copy with the right allocator:
easyhook\src\DriverShared\Rtl\error.c
PWCHAR RtlGetLastErrorString()
{
    int len = (wcslen(LastError)+1)*sizeof(TCHAR);
    PWCHAR pBuffer = (PWCHAR) CoTaskMemAlloc(len);
    CopyMemory(pBuffer, LastError, len);

    return pBuffer;
}
That will fix the issue.
Closed Feb 6, 2014 at 9:50 PM by spazzarama

comments

spazzarama wrote Dec 26, 2013 at 2:35 AM

Thanks for the bug report, do you have a sample to reproduce the error so that we can easily check the fix?

Alois wrote Dec 26, 2013 at 11:30 AM

I have tried the file monitor sample and I did specify an invalid process id to attach to. That does trigger this memory corruption.
I have noticed you are using this method for unmanaged hooking as well. The best would be therefore to leave the C method as is and provide a RtlGetLastErrorString_Copy version for .NET clients to use this one.
Or if you are sure that much more people are only using the managed hooking Apis then you could provide another method for C client like RtlGetLastErrorString_NoCopy

Yours,
Alois Kraus

wrote Feb 6, 2014 at 9:49 PM

Fixed on changeset 73760

wrote Feb 6, 2014 at 9:50 PM

Alois wrote Feb 13, 2014 at 8:50 PM

Very nice. Thanks!