epezent / implot

Immediate Mode Plotting
MIT License
4.64k stars 516 forks source link

ImPlotPoint::operator[] bounds checking #429

Closed dcnieho closed 1 year ago

dcnieho commented 1 year ago

In implot.h, you have:

// Double precision version of ImVec2 used by ImPlot. Extensible by end users.
struct ImPlotPoint {
    double x, y;
    ....
    double  operator[] (size_t idx) const { return (&x)[idx]; }
    double& operator[] (size_t idx)       { return (&x)[idx]; }
    ....
};

These functions do not do bounds checking and may thus read outside the struct's values. ImGui's ImVec2 does the following:

struct ImVec2
{
    float                                   x, y;
    ....
    float  operator[] (size_t idx) const    { IM_ASSERT(idx <= 1); return (&x)[idx]; }    // We very rarely use this [] operator, the assert overhead is fine.
    float& operator[] (size_t idx)          { IM_ASSERT(idx <= 1); return (&x)[idx]; }    // We very rarely use this [] operator, the assert overhead is fine.
    ....
};

And additionally has a pair of macros IM_MSVC_RUNTIME_CHECKS_OFF and IM_MSVC_RUNTIME_CHECKS_RESTORE that it wraps the struct with. Since implot.h imports imgui.h, you could directly use IM_ASSERT as well as the above other macros. Could you consider doing so?

epezent commented 1 year ago

https://github.com/epezent/implot/commit/4f47cdf6c4618b43c82ea16337a703b4f2583d6f