UnifiedEngineering / T-962-improvements

Improvements made to the cheap T-962 reflow oven utilizing the _existing_ controller HW
GNU General Public License v3.0
789 stars 193 forks source link

Fix buffer overflow in LCD_disp_str #245

Open mcapdeville opened 11 months ago

mcapdeville commented 11 months ago

This buffer overflow occur when right tc offet is >= 10.00 or <=-10.00 in this case, there is 22 chars to be displayed on the line. This also occur with long version string as v0.5.2-4-dirty-gxxxxxx. This cause reboot of the firmware.

note from snprintf(3) : snprintf return the return value is the number of characters (excluding the terminating null byte) which would have been written to the final string if enough space had been available.

borland1 commented 11 months ago

I think this is what you're describing as frame buffer overflow for the two OFFSET setup values.

T962SetupMenu1

Your proposed code changes do eliminate the buffer overflow, but don't address the source of the problem.

The reason for the OFFSET character overflow (when offset values are too positive or too negative) is that the Setup menu display format in file Setup.c, has formating errors. Here's the format code with two lines that need changes.

static setupMenuStruct setupmenu[] = {
    {"Min fan speed    %4.0f", REFLOW_MIN_FAN_SPEED, 0, 254, 0, 1.0f},
    {"Cycle done beep %4.1fs", REFLOW_BEEP_DONE_LEN, 0, 254, 0, 0.1f},
    {"Left TC gain     %1.2f", TC_LEFT_GAIN, 10, 190, 0, 0.01f},
    {"Left TC offset  %+1.2f", TC_LEFT_OFFSET, 0, 200, -100, 0.25f},       //   <---
    {"Right TC gain    %1.2f", TC_RIGHT_GAIN, 10, 190, 0, 0.01f},
    {"Right TC offset %+1.2f", TC_RIGHT_OFFSET, 0, 200, -100, 0.25f},  //   <---
};

You can see that the OFFSET input values for both left and right TCs are in two decimal places ( 0.01 degrees C ) and in increments of 0.25C. Displaying the OFFSET values with 2 decimal places is too precise.

T962SetupMenu2

If you substitute these lines in the Strut above,

    {"Left TC offset  %+1.1f", TC_LEFT_OFFSET, 0, 250, -125, 0.20f},   //  <-------

    {"Right TC offset %+1.1f", TC_RIGHT_OFFSET, 0, 250, -125, 0.20f},  //  <-------

the display will look like this..

T962SetupMenu3

while still having the same input value range, but with 0.2C increments.

T962SetupMenu4