sharpdx / SharpDX

SharpDX GitHub Repository
http://sharpdx.org
MIT License
1.69k stars 641 forks source link

[DWrite] Garbage data returned by Font1.Metrics, and probable memory overrun. #1008

Closed Jayonas closed 5 years ago

Jayonas commented 6 years ago

The FontMetrics1 struct that comes back from the Metrics property of a Font1 essentially contains garbage data. On close examination, it looks like it is a FontMetrics struct that has been hard cast to a FontMetrics1 struct.

I'm pretty sure that the problem here is that the native DWRITE_FONT_METRICS1 derives from DWRITE_FONT_METRICS but no such relation exists in managed code since managed structs can't inherit from each other. IDWriteFont1::GetMetrics expects to receive a pointer to a struct that contains both the DWRITE_FONT_METRICS fields and the DWRITE_FONT_METRICS1 fields, but the auto-generated implementation of Font1.Metrics passes it the address of a struct which contains only the DWRITE_FONT_METRICS1 fields.

That means that IDWriteFont1::GetMetrics ends up writing the DWRITE_FONT_METRICS data into the memory that the managed code will try to interpret as DWRITE_FONT_METRICS1 data, effectively resulting in garbage. Worse, it means that IDWriteFont1::GetMetrics is probably overwriting arbitrary memory past the end of the FontMetrics1 structure.

My workaround has been to not use Font1.Metrics at all, and instead implement my own native function to retrieve the data and call it via p/invoke. My function takes separate pointers to a FontMetrics and a FontMetrics1 (and a Font1, obviously), passes its own local DWRITE_FONT_METRICS1 to IDWriteFont1::GetMetrics, and copies each field from that local variable into the appropriate passed pointer.

(Note that I'm basing this off my usage of SharpDX 3.1.1, which I realize is out of date, but it also doesn't look like much has happened in the DWrite code since then so I think it's likely that this is still an issue.)

mrvux commented 6 years ago

Was looking at that issue quickly today, seems in sharpdx 4 generator does add all fields from FontMetrics in FontMetrics1, so I guess we could close that issue

Jayonas commented 5 years ago

I've recently updated SharpDX, and I can confirm that this is fixed in 4.0.1, 4.1.0, and 4.2.0. Thanks!