From 414e75deb118cba8ec9be54cec8589a117251614 Mon Sep 17 00:00:00 2001 From: Crank-Git Date: Sun, 8 Jun 2025 03:32:12 -0400 Subject: [PATCH 1/5] Say issue #6867 about adding validation for long_name and short_name. Firmware should expect at least 1 non-whitespace character for both long_name and short_name. added the USERPREFS_CONFIG_DEVICE_ROLE example to userPrefs.jsonc --- src/modules/AdminModule.cpp | 25 +++++++++++++++++++++---- userPrefs.jsonc | 1 + 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 4005222dc80..4202db30301 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -485,13 +485,30 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) { int changed = 0; + // Validate long_name and short names have meaningful content if (*o.long_name) { - changed |= strcmp(owner.long_name, o.long_name); - strncpy(owner.long_name, o.long_name, sizeof(owner.long_name)); + const char *start = o.long_name; + while (*start == ' ' || *start == '\t') + start++; // Skip whitespace + + if (strlen(start) >= 1) { + changed |= strcmp(owner.long_name, o.long_name); + strncpy(owner.long_name, o.long_name, sizeof(owner.long_name)); + } else { + LOG_WARN("Rejected long_name: must contain at least 1 non-whitespace character"); + } } if (*o.short_name) { - changed |= strcmp(owner.short_name, o.short_name); - strncpy(owner.short_name, o.short_name, sizeof(owner.short_name)); + const char *start = o.short_name; + while (*start == ' ' || *start == '\t') + start++; + + if (strlen(start) >= 1) { + changed |= strcmp(owner.short_name, o.short_name); + strncpy(owner.short_name, o.short_name, sizeof(owner.short_name)); + } else { + LOG_WARN("Rejected short_name: must contain at least 1 non-whitespace character"); + } } if (*o.id) { changed |= strcmp(owner.id, o.id); diff --git a/userPrefs.jsonc b/userPrefs.jsonc index a349a57006f..318729938d8 100644 --- a/userPrefs.jsonc +++ b/userPrefs.jsonc @@ -21,6 +21,7 @@ // "USERPREFS_CONFIG_LORA_REGION": "meshtastic_Config_LoRaConfig_RegionCode_US", // "USERPREFS_CONFIG_OWNER_LONG_NAME": "My Long Name", // "USERPREFS_CONFIG_OWNER_SHORT_NAME": "MLN", + // "USERPREFS_CONFIG_DEVICE_ROLE": "meshtastic_Config_DeviceConfig_Role_CLIENT", // Defaults to CLIENT, ROUTER*, LOST AND FOUND, and REPEATER roles are restricted. // "USERPREFS_EVENT_MODE": "1", // "USERPREFS_FIXED_BLUETOOTH": "121212", // "USERPREFS_FIXED_GPS": "", From 952f0aa663c5a9a73002e880d030aaf4ee066152 Mon Sep 17 00:00:00 2001 From: Crank-Git Date: Sun, 8 Jun 2025 04:36:17 -0400 Subject: [PATCH 2/5] Validation for user long_name and short_name implemented. No longer can use whitespace characters. Return BAD_REQUEST error responses when validation fails and warning logs when validation rejects invalid names. --- src/modules/AdminModule.cpp | 46 ++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 4202db30301..f43617a1edc 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -155,6 +155,27 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta */ case meshtastic_AdminMessage_set_owner_tag: LOG_DEBUG("Client set owner"); + // Validate names before processing + if (*r->set_owner.long_name) { + const char *start = r->set_owner.long_name; + while (*start == ' ' || *start == '\t') + start++; // Skip whitespace + if (strlen(start) < 1) { + LOG_WARN("Rejected long_name: must contain at least 1 non-whitespace character"); + myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); + break; + } + } + if (*r->set_owner.short_name) { + const char *start = r->set_owner.short_name; + while (*start == ' ' || *start == '\t') + start++; // Skip whitespace + if (strlen(start) < 1) { + LOG_WARN("Rejected short_name: must contain at least 1 non-whitespace character"); + myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); + break; + } + } handleSetOwner(r->set_owner); break; @@ -485,30 +506,13 @@ void AdminModule::handleSetOwner(const meshtastic_User &o) { int changed = 0; - // Validate long_name and short names have meaningful content if (*o.long_name) { - const char *start = o.long_name; - while (*start == ' ' || *start == '\t') - start++; // Skip whitespace - - if (strlen(start) >= 1) { - changed |= strcmp(owner.long_name, o.long_name); - strncpy(owner.long_name, o.long_name, sizeof(owner.long_name)); - } else { - LOG_WARN("Rejected long_name: must contain at least 1 non-whitespace character"); - } + changed |= strcmp(owner.long_name, o.long_name); + strncpy(owner.long_name, o.long_name, sizeof(owner.long_name)); } if (*o.short_name) { - const char *start = o.short_name; - while (*start == ' ' || *start == '\t') - start++; - - if (strlen(start) >= 1) { - changed |= strcmp(owner.short_name, o.short_name); - strncpy(owner.short_name, o.short_name, sizeof(owner.short_name)); - } else { - LOG_WARN("Rejected short_name: must contain at least 1 non-whitespace character"); - } + changed |= strcmp(owner.short_name, o.short_name); + strncpy(owner.short_name, o.short_name, sizeof(owner.short_name)); } if (*o.id) { changed |= strcmp(owner.id, o.id); From da0e35568a3f0e033658c6f9b9ffe1f851a5bed6 Mon Sep 17 00:00:00 2001 From: Crank-Git Date: Sun, 8 Jun 2025 17:33:17 -0400 Subject: [PATCH 3/5] Improve whitespace validation for user names with ctype.h, ensure logging works --- src/modules/AdminModule.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index f43617a1edc..11e75e4c7e9 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -7,6 +7,7 @@ #include "SPILock.h" #include "meshUtils.h" #include +#include // for better whitespace handling #if defined(ARCH_ESP32) && !MESHTASTIC_EXCLUDE_BLUETOOTH #include "BleOta.h" #endif @@ -155,12 +156,13 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta */ case meshtastic_AdminMessage_set_owner_tag: LOG_DEBUG("Client set owner"); - // Validate names before processing + // Validate names if (*r->set_owner.long_name) { const char *start = r->set_owner.long_name; - while (*start == ' ' || *start == '\t') - start++; // Skip whitespace - if (strlen(start) < 1) { + // Skip all whitespace (space, tab, newline, unicode, etc) + while (*start && isspace((unsigned char)*start)) + start++; + if (*start == '\0') { LOG_WARN("Rejected long_name: must contain at least 1 non-whitespace character"); myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); break; @@ -168,9 +170,9 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta } if (*r->set_owner.short_name) { const char *start = r->set_owner.short_name; - while (*start == ' ' || *start == '\t') - start++; // Skip whitespace - if (strlen(start) < 1) { + while (*start && isspace((unsigned char)*start)) + start++; + if (*start == '\0') { LOG_WARN("Rejected short_name: must contain at least 1 non-whitespace character"); myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); break; From 8ab5ee94e26ef47f321e1645eabe2ffdc77c8452 Mon Sep 17 00:00:00 2001 From: Crank-Git Date: Sun, 8 Jun 2025 18:03:05 -0400 Subject: [PATCH 4/5] Add whitespace validation to ham mode to prevent validation bypass and to match python cli command --- src/modules/AdminModule.cpp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 11e75e4c7e9..ea4c46949d1 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -159,7 +159,7 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta // Validate names if (*r->set_owner.long_name) { const char *start = r->set_owner.long_name; - // Skip all whitespace (space, tab, newline, unicode, etc) + // Skip all whitespace (space, tab, newline, etc) while (*start && isspace((unsigned char)*start)) start++; if (*start == '\0') { @@ -1176,6 +1176,27 @@ void AdminModule::handleStoreDeviceUIConfig(const meshtastic_DeviceUIConfig &uic void AdminModule::handleSetHamMode(const meshtastic_HamParameters &p) { + // Validate ham parameters before setting since this would bypass validation in the owner struct + if (*p.call_sign) { + const char *start = p.call_sign; + // Skip all whitespace + while (*start && isspace((unsigned char)*start)) + start++; + if (*start == '\0') { + LOG_WARN("Rejected ham call_sign: must contain at least 1 non-whitespace character"); + return; + } + } + if (*p.short_name) { + const char *start = p.short_name; + while (*start && isspace((unsigned char)*start)) + start++; + if (*start == '\0') { + LOG_WARN("Rejected ham short_name: must contain at least 1 non-whitespace character"); + return; + } + } + // Set call sign and override lora limitations for licensed use strncpy(owner.long_name, p.call_sign, sizeof(owner.long_name)); strncpy(owner.short_name, p.short_name, sizeof(owner.short_name)); From 73f930d172f2e660f46eb3d2909f70ae9b95f7c9 Mon Sep 17 00:00:00 2001 From: Crank-Git Date: Sun, 8 Jun 2025 18:11:02 -0400 Subject: [PATCH 5/5] punctuation change --- userPrefs.jsonc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/userPrefs.jsonc b/userPrefs.jsonc index 318729938d8..49732747886 100644 --- a/userPrefs.jsonc +++ b/userPrefs.jsonc @@ -21,7 +21,7 @@ // "USERPREFS_CONFIG_LORA_REGION": "meshtastic_Config_LoRaConfig_RegionCode_US", // "USERPREFS_CONFIG_OWNER_LONG_NAME": "My Long Name", // "USERPREFS_CONFIG_OWNER_SHORT_NAME": "MLN", - // "USERPREFS_CONFIG_DEVICE_ROLE": "meshtastic_Config_DeviceConfig_Role_CLIENT", // Defaults to CLIENT, ROUTER*, LOST AND FOUND, and REPEATER roles are restricted. + // "USERPREFS_CONFIG_DEVICE_ROLE": "meshtastic_Config_DeviceConfig_Role_CLIENT", // Defaults to CLIENT. ROUTER*, LOST AND FOUND, and REPEATER roles are restricted. // "USERPREFS_EVENT_MODE": "1", // "USERPREFS_FIXED_BLUETOOTH": "121212", // "USERPREFS_FIXED_GPS": "",