lighttransport / nanort

NanoRT, single header only modern ray tracing kernel.
MIT License
1.07k stars 89 forks source link

Potential issue in examples/common/imgui/imgui_widgets.cpp: Arithmetic Overflow in Expression #61

Closed monocle-ai closed 4 years ago

monocle-ai commented 4 years ago

What is a Arithmetic Overflow? When a narrow type integral value was shifted left, multiplied, added, or subtracted and the result of that arithmetic operation was cast to a wider type value. If the operation overflowed the narrow type value, then data is lost. You can prevent this loss by converting the value to a wider type before the arithmetic operation.

2 instances of this defect were found in the following locations:


Instance 1 File : examples/common/imgui/imgui_widgets.cpp Enclosing Function : $SliderBehaviorT@_J_JN@ImGui https://github.com/lighttransport/nanort/blob/135a452385a2903592db7518fbb80479d99ab9c3/examples/common/imgui/imgui_widgets.cpp#L2202 Issue in: v max, clicked t

Code extract:

                {
                    // For integer values we want the clicking position to match the grab box so we round above
                    // This code is carefully tuned to work with large values (e.g. high ranges of U64) while preserving this property..
                    FLOATTYPE v_new_off_f = (v_max - v_min) * clicked_t; <------ HERE
                    TYPE v_new_off_floor = (TYPE)(v_new_off_f);
                    TYPE v_new_off_round = (TYPE)(v_new_off_f + (FLOATTYPE)0.5);

How can I fix it? Correct reference usage found in examples/common/imgui/imgui_widgets.cpp at line 1763. https://github.com/lighttransport/nanort/blob/135a452385a2903592db7518fbb80479d99ab9c3/examples/common/imgui/imgui_widgets.cpp#L1763 Code extract:

        // Offset + round to user desired precision, with a curve on the v_min..v_max range to get more precision on one side of the range
        FLOATTYPE v_old_norm_curved = ImPow((FLOATTYPE)(v_cur - v_min) / (FLOATTYPE)(v_max - v_min), (FLOATTYPE)1.0f / power);
        FLOATTYPE v_new_norm_curved = v_old_norm_curved + (g.DragCurrentAccum / (v_max - v_min));
        v_cur = v_min + (TYPE)ImPow(ImSaturate((float)v_new_norm_curved), power) * (v_max - v_min); <------ HERE
        v_old_ref_for_accum_remainder = v_old_norm_curved;
    }

Instance 2 File : examples/common/imgui/imgui_widgets.cpp Enclosing Function : $SliderBehaviorT@_K_JN@ImGui https://github.com/lighttransport/nanort/blob/135a452385a2903592db7518fbb80479d99ab9c3/examples/common/imgui/imgui_widgets.cpp#L2202 Issue in: v max, clicked t

Code extract:

                {
                    // For integer values we want the clicking position to match the grab box so we round above
                    // This code is carefully tuned to work with large values (e.g. high ranges of U64) while preserving this property..
                    FLOATTYPE v_new_off_f = (v_max - v_min) * clicked_t; <------ HERE
                    TYPE v_new_off_floor = (TYPE)(v_new_off_f);
                    TYPE v_new_off_round = (TYPE)(v_new_off_f + (FLOATTYPE)0.5);

How can I fix it? Correct reference usage found in examples/common/imgui/imgui_widgets.cpp at line 1763. https://github.com/lighttransport/nanort/blob/135a452385a2903592db7518fbb80479d99ab9c3/examples/common/imgui/imgui_widgets.cpp#L1763 Code extract:

        // Offset + round to user desired precision, with a curve on the v_min..v_max range to get more precision on one side of the range
        FLOATTYPE v_old_norm_curved = ImPow((FLOATTYPE)(v_cur - v_min) / (FLOATTYPE)(v_max - v_min), (FLOATTYPE)1.0f / power);
        FLOATTYPE v_new_norm_curved = v_old_norm_curved + (g.DragCurrentAccum / (v_max - v_min));
        v_cur = v_min + (TYPE)ImPow(ImSaturate((float)v_new_norm_curved), power) * (v_max - v_min); <------ HERE
        v_old_ref_for_accum_remainder = v_old_norm_curved;
    }
syoyo commented 4 years ago

Please report the issue to imgui project, not here.