nesrak1 / AssetsTools.NET

Read and write unity assets/bundle files, based on https://github.com/SeriousCache/UABE
MIT License
509 stars 101 forks source link

Marshal.UnsafeAddrOfPinnedArrayElement() called on unpinned array #49

Open arcusmaximus opened 2 years ago

arcusmaximus commented 2 years ago

Both the example code in the README file and the code of AssetsView itself use Marshal.UnsafeAddrOfPinnedArrayElement() when passing texture data to the System.Drawing.Bitmap constructor. However, neither of them pin the data array first, which goes against the Microsoft documentation:

The array must be pinned using a GCHandle before it is passed to this method. For maximum performance, this method does not validate the array passed to it; this can result in unexpected behavior.

Indeed, if the array is not pinned, the garbage collector is free to move it around in memory at any time - and if this happens while GDI+ is busy creating the bitmap, the user will get anything from a corrupted texture to a crash.

As a side remark, neither the example code nor AssetsView dispose the Bitmap object when they're done with it.

nesrak1 commented 2 years ago

You're right, I've quit using this myself (especially since Wine doesn't even support it) but haven't updated that section of the readme in a while. I'd rather replace the entire thing with the safer version with LockBits instead of just pinning the array.