`PlotHeatmap::with_label_format` is (probably) unsound
Aeledfyr opened this issue · 2 comments
Allowing user control of the label format string is unsound, as implot passes it directly into sprintf with a 32 char buffer. (See epezent/implot#310)
Plot heatmap uses the provided format string here:
https://github.com/epezent/implot/blob/4fcc6e01aca406ef17d5a2dabdcbc9e1bd962c0d/implot_items.cpp#L2063
Setting the format string to "%100f" overflows the 32 character buffer and writes into stack memory (though on my system, glibc detects it and aborts).
The same issue is present in PlotPieChart, but that does not currently have a binding in this library.
For now, I think the safest options would be either removing it or manually generating it on the rust side (ie. generate a string `"%.{n}f" with a given level of precision, while ensuring that it is still safe)
Good find, I agree that this is a problem. Probably disabling custom formatting is the best bet at the moment - likely the best API to have would be to let the user provide a formatter callback, but this won't be possible with the way implot exposes the formatting functionality.
I've now marked the label-setting function as unsafe and added a note about the buffer. This does not remove functionality, but makes the danger of setting a wrong format string much clearer and has people explicitly opt into it with unsafe.