Author: Mingjie Shen <shen497@purdue.edu>
Fix potential buffer overflows when calling sprintf 1. Replace sprintf with snprintf, which is safer 2. An unsigned int or unsigned long int requires 11 bytes to print (including the null terminator) 3. Use PRIu16 macro to print uint16_t 4. Format string "#%2d %2d:%02d:%02d.%02d\n" in StopWatch::stopLapBtnEventHandler() requires at least 17 bytes. The 16-byte buffer would clearly be overrun if sprintf were used.
src/components/datetime/DateTimeController.cpp | 4 ++-- src/displayapp/screens/StopWatch.cpp | 6 +++--- src/displayapp/screens/SystemInfo.cpp | 8 ++++---- src/displayapp/screens/Twos.cpp | 2 +- src/displayapp/screens/settings/SettingDisplay.cpp | 2 +-
diff --git a/src/components/datetime/DateTimeController.cpp b/src/components/datetime/DateTimeController.cpp
index 9e9fb6e4fe03a9fc715e164a032dbea14c9163d1..8d4a834e063b676dd8b68bec57c1d7407ebdc21d 100644
--- a/src/components/datetime/DateTimeController.cpp
+++ b/src/components/datetime/DateTimeController.cpp
@@ -140,9 +140,9 @@ } else {
hour12 = (hour == 12) ? 12 : hour - 12;
amPmStr = "PM";
}
- sprintf(buff, "%i:%02i %s", hour12, minute, amPmStr);
+ snprintf(buff, sizeof(buff), "%i:%02i %s", hour12, minute, amPmStr);
} else {
- sprintf(buff, "%02i:%02i", hour, minute);
+ snprintf(buff, sizeof(buff), "%02i:%02i", hour, minute);
}
return std::string(buff);
}
diff --git a/src/displayapp/screens/StopWatch.cpp b/src/displayapp/screens/StopWatch.cpp
index 239ebe393486d30af26b10369224c2edfe8399e4..d2b811542f233ab7a88097d482edce988d02e958 100644
--- a/src/displayapp/screens/StopWatch.cpp
+++ b/src/displayapp/screens/StopWatch.cpp
@@ -198,11 +198,11 @@ lv_label_ins_text(lapText, LV_LABEL_POS_LAST, "\n");
continue;
}
TimeSeparated_t times = convertTicksToTimeSegments(laps[i]);
- char buffer[16];
+ char buffer[17];
if (times.hours == 0) {
- sprintf(buffer, "#%2d %2d:%02d.%02d\n", i + 1, times.mins, times.secs, times.hundredths);
+ snprintf(buffer, sizeof(buffer), "#%2d %2d:%02d.%02d\n", i + 1, times.mins, times.secs, times.hundredths);
} else {
- sprintf(buffer, "#%2d %2d:%02d:%02d.%02d\n", i + 1, times.hours, times.mins, times.secs, times.hundredths);
+ snprintf(buffer, sizeof(buffer), "#%2d %2d:%02d:%02d.%02d\n", i + 1, times.hours, times.mins, times.secs, times.hundredths);
}
lv_label_ins_text(lapText, LV_LABEL_POS_LAST, buffer);
}
diff --git a/src/displayapp/screens/SystemInfo.cpp b/src/displayapp/screens/SystemInfo.cpp
index eea7593800e752c59bdb69b82bccc9cf5a6ef7b9..18fb7ad2083446fd392a928c3e5e4965df051b7b 100644
--- a/src/displayapp/screens/SystemInfo.cpp
+++ b/src/displayapp/screens/SystemInfo.cpp
@@ -236,9 +236,9 @@
auto nb = uxTaskGetSystemState(tasksStatus, maxTaskCount, nullptr);
std::sort(tasksStatus, tasksStatus + nb, sortById);
for (uint8_t i = 0; i < nb && i < maxTaskCount; i++) {
- char buffer[7] = {0};
+ char buffer[11] = {0};
- sprintf(buffer, "%lu", tasksStatus[i].xTaskNumber);
+ snprintf(buffer, sizeof(buffer), "%lu", tasksStatus[i].xTaskNumber);
lv_table_set_cell_value(infoTask, i + 1, 0, buffer);
switch (tasksStatus[i].eCurrentState) {
case eReady:
@@ -262,9 +262,9 @@ buffer[1] = '\0';
lv_table_set_cell_value(infoTask, i + 1, 1, buffer);
lv_table_set_cell_value(infoTask, i + 1, 2, tasksStatus[i].pcTaskName);
if (tasksStatus[i].usStackHighWaterMark < 20) {
- sprintf(buffer, "%d low", tasksStatus[i].usStackHighWaterMark);
+ snprintf(buffer, sizeof(buffer), "%" PRIu16 " low", tasksStatus[i].usStackHighWaterMark);
} else {
- sprintf(buffer, "%d", tasksStatus[i].usStackHighWaterMark);
+ snprintf(buffer, sizeof(buffer), "%" PRIu16, tasksStatus[i].usStackHighWaterMark);
}
lv_table_set_cell_value(infoTask, i + 1, 3, buffer);
}
diff --git a/src/displayapp/screens/Twos.cpp b/src/displayapp/screens/Twos.cpp
index 8157a160b9987a45a3014ac42128b4d56afa93ce..6f2eff40c4d7129366a65209405995b6909f1698 100644
--- a/src/displayapp/screens/Twos.cpp
+++ b/src/displayapp/screens/Twos.cpp
@@ -242,7 +242,7 @@ const unsigned int row = i / nCols;
const unsigned int col = i % nCols;
if (grid[row][col].value > 0) {
char buffer[7];
- sprintf(buffer, "%d", grid[row][col].value);
+ snprintf(buffer, sizeof(buffer), "%u", grid[row][col].value);
lv_table_set_cell_value(gridDisplay, row, col, buffer);
} else {
lv_table_set_cell_value(gridDisplay, row, col, "");
diff --git a/src/displayapp/screens/settings/SettingDisplay.cpp b/src/displayapp/screens/settings/SettingDisplay.cpp
index 91f4d5902f50fedc45f6ff84b6251d5e61933b95..9abb4c74cfe695b371c0cb4119a929dc29f01452 100644
--- a/src/displayapp/screens/settings/SettingDisplay.cpp
+++ b/src/displayapp/screens/settings/SettingDisplay.cpp
@@ -46,7 +46,7 @@
char buffer[12];
for (unsigned int i = 0; i < options.size(); i++) {
cbOption[i] = lv_checkbox_create(container1, nullptr);
- sprintf(buffer, "%2ds", options[i] / 1000);
+ snprintf(buffer, sizeof(buffer), "%2" PRIu16 "s", options[i] / 1000);
lv_checkbox_set_text(cbOption[i], buffer);
cbOption[i]->user_data = this;
lv_obj_set_event_cb(cbOption[i], event_handler);