Assimp GetMaterialString binding can cause AccessViolationException

[Georgiks]

Summary

For certain overload of the Assimp.GetMaterialString, there is a code that copies managed string to global memory and passes the pointer to native method. However when it returns, it attempts to convert to string (SilkMarshal.PtrToString) which is useless since the resulting string is not use anywhere.

The aforementioned PtrToString (or rather Utf8PtrToString used for the used LPUTF8Str encoding) then can cause AccessViolationException because the way it is implemented is it scans for null-terminator in Span<byte>(ptr, int.MaxValue) which can use vectorization instructions and "look ahead". However when the pointer is near the edge of accessible memory, this can fail.

So technically there are two issues: unnecessary pointer to string conversion, and Access Violation risk when using Utf8PtrToString

Steps to reproduce

• Platform: Desktop
• Framework Version: .NET 8
• Tested Nuget Versions: 2.13.0, 2.20.0

I wasn't able to create easy repro, but the bug was encountered after using Silk.NET.Assimp like this:

string GetMaterialName(Material* material)
{
    AssimpString materialName = new AssimpString();
    _assimp.GetMaterialString(material, Assimp.MaterialNameBase, 0, 0, ref materialName);
...

Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
    at System.SpanHelpers.NonPackedIndexOfValueType[[System.Byte, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.SpanHelpers+DontNegate`1[[System.Byte, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](Byte ByRef, Byte, Int32)
    at System.MemoryExtensions.IndexOf[[System.Byte, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.Span`1<Byte>, Byte)
    at Silk.NET.Core.Native.SilkMarshal.Utf8PtrToString(IntPtr)
    at Silk.NET.Core.Native.SilkMarshal.PtrToString(IntPtr, Silk.NET.Core.Native.NativeStringEncoding)
    at Silk.NET.Assimp.Assimp.GetMaterialString(Silk.NET.Assimp.Material*, System.String, UInt32, UInt32, Silk.NET.Assimp.AssimpString ByRef)

Comments

In the following image, the highlighted code shouldn't be necessary at all.

[Perksey]

Thanks, I've changed the codegen to no longer have this pointless reassignment (below is .NET Standard 2.0 code hence the StCall):

        public unsafe partial global::Silk.NET.Assimp.Return GetMaterialProperty(global::Silk.NET.Assimp.Material* pMat, string pKey, uint type, uint index, global::Silk.NET.Assimp.MaterialProperty** pPropOut)
        {
            global::Silk.NET.Assimp.Return n6;
            byte* n5;
            global::Silk.NET.Assimp.Return StCall(global::Silk.NET.Assimp.Material* arg0, byte* arg1, uint arg2, uint arg3, global::Silk.NET.Assimp.MaterialProperty** arg4)
            {
                if (Silk.NET.Core.Native.SilkMarshal.IsWinapiStdcall)
                {
                    return ((delegate* unmanaged[Stdcall]<global::Silk.NET.Assimp.Material*, byte*, uint, uint, global::Silk.NET.Assimp.MaterialProperty**, global::Silk.NET.Assimp.Return> )(CurrentVTable as _B)._DM)(arg0, arg1, arg2, arg3, arg4);
                }
                else
                {
                    return ((delegate* unmanaged[Cdecl]<global::Silk.NET.Assimp.Material*, byte*, uint, uint, global::Silk.NET.Assimp.MaterialProperty**, global::Silk.NET.Assimp.Return> )(CurrentVTable as _B)._DM)(arg0, arg1, arg2, arg3, arg4);
                }
            };
            n5 = (byte*)Silk.NET.Core.Native.SilkMarshal.StringToPtr((pKey), (Silk.NET.Core.Native.NativeStringEncoding)48);
            n6 = StCall((pMat), (n5), (type), (index), (pPropOut));
            Silk.NET.Core.Native.SilkMarshal.FreeString((global::System.IntPtr)n5, (Silk.NET.Core.Native.NativeStringEncoding)48);
            return n6;
        }

And I've also fixed the bug you uncovered in PtrToString on .NET 8.

These will be released in 2.21

Fixed in #1990