Closed Aeledfyr closed 2 years ago
168244e makes the change to snprintf. Regarding the second issue, ImGui
allows users to provide custom format strings throughout its API for various widgets. Do those functions also present the same issue, or is ImGui taking special precauations we should also be taking?
Curious, I followed ImGui's format strings all the way through and it doesn't seem ImGui is doing anything special -- all format strings go to vsnprintf
without additional checks.
In doing so, I discovered that ImGui wraps raw calls to vsnprintf
inside of ImFormatString
, which can be override by the user using imconfig.h
. I have decided to replace all of our calls to snprintf
with ImFormatString
so that ImPlot will also honor the user's override (86f4dd6). I suppose one could intercept malicious format arguements there if desired.
PlotPieChart
andRenderHeatmap
callsprintf
with a provided format string, which is unsafe. https://github.com/epezent/implot/blob/4fcc6e01aca406ef17d5a2dabdcbc9e1bd962c0d/implot_items.cpp#L1909 https://github.com/epezent/implot/blob/4fcc6e01aca406ef17d5a2dabdcbc9e1bd962c0d/implot_items.cpp#L2063This can lead to a simple buffer overflow, if the provided format string causes >32 characters of output, but it may also allow writing to arbitrary memory locations by using
%n
and reading local stack addresses using%p
.I don't know if there are ways to handle the second two issues, but using
snprintf
instead ofsprintf
should prevent potential buffer overflows.