From 69d016fab09d7646cee3b017b0cbe38a86deb6e3 Mon Sep 17 00:00:00 2001 From: philmoz Date: Thu, 24 Sep 2026 11:56:04 +1000 Subject: [PATCH] chore(fw): replace unsafe calls to strAppend with copyToUnTerminated (#7814) Co-authored-by: Peter Feerick <5500713+pfeerick@users.noreply.github.com> --- radio/src/gui/colorlcd/themes/theme_manager.cpp | 6 +++--- radio/src/storage/yaml/yaml_datastructs_funcs.cpp | 2 +- radio/src/strhelpers.cpp | 1 + radio/src/tests/modelslist_labels.cpp | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/radio/src/gui/colorlcd/themes/theme_manager.cpp b/radio/src/gui/colorlcd/themes/theme_manager.cpp index 0bea14b4e..f471e7fa7 100644 --- a/radio/src/gui/colorlcd/themes/theme_manager.cpp +++ b/radio/src/gui/colorlcd/themes/theme_manager.cpp @@ -396,7 +396,8 @@ void ThemePersistance::loadDefaultTheme() } for (auto theme : themes) { - if (theme->getName().compare(0, SELECTED_THEME_NAME_LEN, g_eeGeneral.selectedTheme) == 0) { + if (theme->getName().compare(0, SELECTED_THEME_NAME_LEN, + stringFromNtString(g_eeGeneral.selectedTheme)) == 0) { found = true; break; } @@ -469,8 +470,7 @@ bool ThemePersistance::createNewTheme(const std::string& name, ThemeFile& theme) void ThemePersistance::setDefaultTheme(int index) { if (index >= 0 && index < (int)themes.size()) { - strAppend(g_eeGeneral.selectedTheme, themes[index]->getName().c_str(), - SELECTED_THEME_NAME_LEN); + copyToUnTerminated(g_eeGeneral.selectedTheme, themes[index]->getName()); SET_DIRTY(); currentTheme = index; } diff --git a/radio/src/storage/yaml/yaml_datastructs_funcs.cpp b/radio/src/storage/yaml/yaml_datastructs_funcs.cpp index 3ebf07d21..9f1406b6b 100644 --- a/radio/src/storage/yaml/yaml_datastructs_funcs.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_funcs.cpp @@ -2748,7 +2748,7 @@ static void r_cfs_name(void* user, uint8_t* data, uint32_t bitoffs, const char* val, uint8_t val_len) { uint16_t idx = getIdx(user); - strAppend(g_model.customSwitches[idx].name, val, LEN_SWITCH_NAME); + copyToUnTerminated(g_model.customSwitches[idx].name, val); } static const struct YamlNode struct_cfsNameConfig[] = { diff --git a/radio/src/strhelpers.cpp b/radio/src/strhelpers.cpp index 9492d87ab..099f15bfe 100644 --- a/radio/src/strhelpers.cpp +++ b/radio/src/strhelpers.cpp @@ -1192,6 +1192,7 @@ char *strAppendSigned(char *dest, int32_t value, uint8_t digits, uint8_t radix) return strAppendUnsigned(dest, (uint32_t)value, digits, radix); } +// NOTE: 'dest' buffer must be at least 'len + 1' bytes long to allow for nul terminator char *strAppend(char *dest, const char *source, int len) { if (source == nullptr) { *dest = '\0'; return dest; } diff --git a/radio/src/tests/modelslist_labels.cpp b/radio/src/tests/modelslist_labels.cpp index 138801591..50f943da7 100644 --- a/radio/src/tests/modelslist_labels.cpp +++ b/radio/src/tests/modelslist_labels.cpp @@ -99,7 +99,7 @@ class ModelMapFsTest : public ::testing::Test // as a compaction optimisation, which would leave this fixture without // a "header:" section at all. Giving it a name keeps the fixture // realistic and avoids that unrelated edge case. - strAppend(model.header.name, filename, LEN_MODEL_NAME); + copyToUnTerminated(model.header.name, filename); strAppend(model.header.labels, labels, LABELS_LENGTH - 1); model.getScreenData(0)->LayoutId = screenLayoutId; model.getTopbarData()->zones[0].widgetName = widgetName;