[SYS] Bound remaining MQTT/HTTP/BLE/RF inputs into fixed buffers (#2324)

Follow-up to #2321. That PR fixed five strcpy sites in XtoSYS/createOrUpdateDevice
but left adjacent attack surfaces using the same pattern (strncpy with the
destination's own size, leaving the buffer non-NUL-terminated when the source
is at the limit; or strcpy of WiFiManager/HTTP form values that bypass the
client-side maxlength). The over-long string then flows into saveConfig() and
back into a strcpy on next boot, corrupting heap/.bss on every reload.

main/main.cpp - XtoSYS: length-check before copying mqtt_topic, discovery_prefix,
  gateway_name, gw_pass (ota_pass), and ble_aes. Reject over-length with
  THEENGS_LOG_WARNING, preserving existing values.
main/main.cpp - loadConfigFromFlash: new loadStrField() helper applies the same
  guard to every SPIFFS-loaded string (mqtt_server/port/user/pass per slot,
  mqtt_topic, discovery_prefix, gateway_name, ota_pass, ble_aes). Defense in
  depth in case a pre-fix build left an over-length value in /config.json.
main/main.cpp - WiFiManager save: bound mqtt_topic against the 150-byte portal
  buffer overflowing the 66-byte destination (HTML maxlength is client-side
  only; a raw POST bypasses it).
main/webUI.cpp - handleCG /gw save: bound gateway-password form input before
  the strncpy that previously left ota_pass non-NUL-terminated.
main/blufi.cpp - ESP_BLUFI_EVENT_RECV_STA_SSID/PASSWD: clamp the wire-supplied
  ssid_len/passwd_len before the memcpy and the trailing NUL so a crafted
  BluFi provisioning frame cannot overrun gl_sta_ssid[32]/gl_sta_passwd[64].
main/gatewayBT.cpp - BLEDecryptor (Victron, encr==3): remove `nonce[16] = {0}`,
  a one-byte stack-OOB write reachable from any BLE adv. The line was dead
  (the memset two lines later zeroes the whole array); deleting it eliminates
  the OOB write.
main/gatewaySRFB.cpp - SRFBtoX: bound the _uartbuf writer so a 433 MHz frame
  with no STOP can no longer let _uartpos grow past sizeof(_uartbuf).
main/gatewaySRFB.cpp - _rfbSend: the hex output buffer was sized RF_MESSAGE_SIZE
  (9) but _rawToHex writes 2*N+2 bytes per call. Resize to fit.

Tested on a Theengs Bridge: built theengs-bridge-v11 env, OTA-flashed, and
verified that publishing {"discovery_prefix":"Z"*200} to MQTTtoSYS/config
leaves discovery_prefix unchanged (no Z*65 truncation in subsequent discovery
topics), bridge keeps decoding BLE adverts, and SYStoMQTT/LWT stay healthy.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: 1technophile

main/blufi.cpp

@@ -321,16 +321,27 @@ static void example_event_callback(esp_blufi_cb_event_t event, esp_blufi_cb_para
       THEENGS_LOG_TRACE(F("BLUFI recv slave disconnect a ble connection" CR));
       esp_blufi_disconnect();
       break;
-    case ESP_BLUFI_EVENT_RECV_STA_SSID:
-      strncpy((char*)gl_sta_ssid, (char*)param->sta_ssid.ssid, param->sta_ssid.ssid_len);
-      gl_sta_ssid[param->sta_ssid.ssid_len] = '\0';
+    case ESP_BLUFI_EVENT_RECV_STA_SSID: {
+      // ssid_len comes from an unauthenticated BLE provisioning frame; clamp it
+      // before the strncpy and the trailing NUL so a crafted long SSID cannot
+      // write past gl_sta_ssid.
+      size_t ssid_len = param->sta_ssid.ssid_len;
+      if (ssid_len > sizeof(gl_sta_ssid) - 1)
+        ssid_len = sizeof(gl_sta_ssid) - 1;
+      memcpy(gl_sta_ssid, param->sta_ssid.ssid, ssid_len);
+      gl_sta_ssid[ssid_len] = '\0';
       THEENGS_LOG_NOTICE(F("Recv STA SSID %s" CR), gl_sta_ssid);
       break;
-    case ESP_BLUFI_EVENT_RECV_STA_PASSWD:
-      strncpy((char*)gl_sta_passwd, (char*)param->sta_passwd.passwd, param->sta_passwd.passwd_len);
-      gl_sta_passwd[param->sta_passwd.passwd_len] = '\0';
+    }
+    case ESP_BLUFI_EVENT_RECV_STA_PASSWD: {
+      size_t passwd_len = param->sta_passwd.passwd_len;
+      if (passwd_len > sizeof(gl_sta_passwd) - 1)
+        passwd_len = sizeof(gl_sta_passwd) - 1;
+      memcpy(gl_sta_passwd, param->sta_passwd.passwd, passwd_len);
+      gl_sta_passwd[passwd_len] = '\0';
       THEENGS_LOG_NOTICE(F("Recv STA PASSWORD" CR));
       break;
+    }
     case ESP_BLUFI_EVENT_GET_WIFI_LIST: {
       WiFi.scanNetworks(true);
       break;

main/gatewayBT.cpp

@@ -1348,7 +1348,10 @@ void process_bledata(JsonObject& BLEdata) {
       THEENGS_LOG_TRACE(F("[BLEDecryptor] BTHomeV2 nonce %s" CR), NimBLEUtils::dataToHexString(nonce, noncelength).c_str());
 
     } else if (BLEdata["encr"].as<int>() == 3) {
-      nonce[16] = {0}; // Victron has a 16 byte zero padded nonce with IV bytes 6,7
+      // Victron has a 16-byte zero-padded nonce with IV bytes 0,1.
+      // (The line `nonce[16] = {0};` that lived here wrote a byte one past the
+      // end of the 16-byte `nonce` array on every Victron-encrypted advert;
+      // memset below already zeroes the whole buffer, so the typo was dead.)
       unsigned char iv[2];
       int ivlen = hexToBytes(BLEdata["ctr"].as<String>(), iv, 2);
       if (ivlen != 2) {

main/gatewaySRFB.cpp

@@ -60,7 +60,9 @@ void _rfbSend(byte* message) {
 }
 
 void _rfbSend(byte* message, int times) {
-  char buffer[RF_MESSAGE_SIZE];
+  // _rawToHex writes 3 bytes per input byte (two hex chars + CR) followed by a
+  // terminating NUL, so the buffer needs 2 * RF_MESSAGE_SIZE + 2 bytes.
+  char buffer[RF_MESSAGE_SIZE * 2 + 2] = {0};
   TheengsUtils::_rawToHex(message, buffer, RF_MESSAGE_SIZE);
   THEENGS_LOG_NOTICE(F("[RFBRIDGE] Sending MESSAGE" CR));
 
@@ -85,8 +87,15 @@ bool SRFBtoX() {
       if (c == RF_CODE_STOP) {
         _rfbDecode();
         receiving = false;
-      } else {
+      } else if (_uartpos < sizeof(_uartbuf)) {
         _uartbuf[_uartpos++] = c;
+      } else {
+        // Frame longer than expected — drop it rather than overrun _uartbuf.
+        // A crafted 433 MHz transmission that emits START but never STOP would
+        // otherwise let _uartpos grow up to 255 and trample adjacent globals.
+        THEENGS_LOG_WARNING(F("[RFBRIDGE] RF frame too long, dropping" CR));
+        receiving = false;
+        _uartpos = 0;
       }
     } else if (c == RF_CODE_START) {
       _uartpos = 0;

main/main.cpp

@@ -2049,6 +2049,19 @@ void saveConfig() {
   configFile.close();
 }
 
+// Length-checked copy used when loading config.json values into fixed-size
+// buffers. The SPIFFS file is implicitly trusted once written, but historical
+// MQTT/HTTP paths could land over-length values there before we hardened them;
+// this prevents one stale config from overflowing globals on every boot.
+static void loadStrField(const char* name, char* dst, size_t dstSize, const char* src) {
+  if (!src) return;
+  if (strlen(src) >= dstSize) {
+    THEENGS_LOG_WARNING(F("Config field %s too long (%u), skipping" CR), name, (unsigned)strlen(src));
+    return;
+  }
+  strcpy(dst, src);
+}
+
 bool loadConfigFromFlash() {
   THEENGS_LOG_TRACE(F("mounting FS..." CR));
   bool result = false;
@@ -2124,22 +2137,22 @@ bool loadConfigFromFlash() {
           strcpy(key, "mqtt_server");
           strcat(key, index_suffix);
           if (json.containsKey(key)) {
-            strcpy(cnt_parameters_array[i].mqtt_server, json[key].as<const char*>());
+            loadStrField(key, cnt_parameters_array[i].mqtt_server, sizeof(cnt_parameters_array[i].mqtt_server), json[key].as<const char*>());
           }
           strcpy(key, "mqtt_port");
           strcat(key, index_suffix);
           if (json.containsKey(key)) {
-            strcpy(cnt_parameters_array[i].mqtt_port, json[key].as<const char*>());
+            loadStrField(key, cnt_parameters_array[i].mqtt_port, sizeof(cnt_parameters_array[i].mqtt_port), json[key].as<const char*>());
           }
           strcpy(key, "mqtt_user");
           strcat(key, index_suffix);
           if (json.containsKey(key)) {
-            strcpy(cnt_parameters_array[i].mqtt_user, json[key].as<const char*>());
+            loadStrField(key, cnt_parameters_array[i].mqtt_user, sizeof(cnt_parameters_array[i].mqtt_user), json[key].as<const char*>());
           }
           strcpy(key, "mqtt_pass");
           strcat(key, index_suffix);
           if (json.containsKey(key)) {
-            strcpy(cnt_parameters_array[i].mqtt_pass, json[key].as<const char*>());
+            loadStrField(key, cnt_parameters_array[i].mqtt_pass, sizeof(cnt_parameters_array[i].mqtt_pass), json[key].as<const char*>());
           }
           strcpy(key, "mqtt_broker_secure");
           strcat(key, index_suffix);
@@ -2166,15 +2179,16 @@ bool loadConfigFromFlash() {
         }
 #  endif
         if (json.containsKey("mqtt_topic"))
-          strcpy(mqtt_topic, json["mqtt_topic"]);
+          loadStrField("mqtt_topic", mqtt_topic, sizeof(mqtt_topic), json["mqtt_topic"]);
 #  ifdef ZmqttDiscovery
         if (json.containsKey("discovery_prefix"))
-          strcpy(discovery_prefix, json["discovery_prefix"]);
+          // discovery_prefix is extern char[] in this TU; size is parameters_size + 1.
+          loadStrField("discovery_prefix", discovery_prefix, parameters_size + 1, json["discovery_prefix"]);
 #  endif
         if (json.containsKey("gateway_name"))
-          strcpy(gateway_name, json["gateway_name"]);
+          loadStrField("gateway_name", gateway_name, sizeof(gateway_name), json["gateway_name"]);
         if (json.containsKey("ota_pass")) {
-          strcpy(ota_pass, json["ota_pass"]);
+          loadStrField("ota_pass", ota_pass, sizeof(ota_pass), json["ota_pass"]);
 #  ifdef WM_PWD_FROM_MAC // From ESP Mac Address, last 8 digits as the password
           // Compare the existing ota_pass if ota_pass = OTAPASSWORD then replace with the last 8 digits of the mac address
           // This enable user migrating from previous version to have the same WiFi portal password as previously unless they changed it
@@ -2187,7 +2201,7 @@ bool loadConfigFromFlash() {
         }
 #  if BLEDecryptor
         if (json.containsKey("ble_aes")) {
-          strcpy(ble_aes, json["ble_aes"]);
+          loadStrField("ble_aes", ble_aes, sizeof(ble_aes), json["ble_aes"]);
           THEENGS_LOG_TRACE(F("loaded default BLE AES key %s" CR), ble_aes);
         }
         if (json.containsKey("ble_aes_keys")) {
@@ -2376,9 +2390,17 @@ void setupWiFiManager() {
       strcpy(cnt_parameters_array[cnt_index].mqtt_pass, custom_mqtt_pass.getValue());
     }
     if (strlen(custom_mqtt_topic.getValue()) > 0) {
-      strcpy(mqtt_topic, custom_mqtt_topic.getValue());
-      if (mqtt_topic[strlen(mqtt_topic) - 1] != '/' && strlen(mqtt_topic) < parameters_size) {
-        strcat(mqtt_topic, "/");
+      // The WiFiManagerParameter buffer is mqtt_topic_max_size (150) but the
+      // destination is sizeof(mqtt_topic). HTML maxlength is client-side only;
+      // a raw POST can deliver any length, so length-check before copying.
+      const char* src = custom_mqtt_topic.getValue();
+      if (strlen(src) >= sizeof(mqtt_topic)) {
+        THEENGS_LOG_WARNING(F("mqtt_topic from portal too long, ignoring" CR));
+      } else {
+        strcpy(mqtt_topic, src);
+        if (mqtt_topic[strlen(mqtt_topic) - 1] != '/' && strlen(mqtt_topic) < parameters_size) {
+          strcat(mqtt_topic, "/");
+        }
       }
     }
 
@@ -3474,20 +3496,45 @@ void XtoSYS(const char* topicOri, JsonObject& SYSdata) { // json object decoding
 #endif
         (SYSdata.containsKey("gateway_name") && SYSdata["gateway_name"].is<const char*>()) ||
         (SYSdata.containsKey("gw_pass") && SYSdata["gw_pass"].is<const char*>())) {
+      // Each of the destination buffers is sized at compile time; reject
+      // over-length MQTT-supplied values up front so a crafted publish cannot
+      // leave the destination non-NUL-terminated (which would corrupt the
+      // SPIFFS config on the next saveConfig()).
       if (SYSdata.containsKey("mqtt_topic")) {
-        strncpy(mqtt_topic, SYSdata["mqtt_topic"], parameters_size);
+        const char* src = SYSdata["mqtt_topic"];
+        if (strlen(src) >= sizeof(mqtt_topic)) {
+          THEENGS_LOG_WARNING(F("mqtt_topic too long, ignoring" CR));
+        } else {
+          strcpy(mqtt_topic, src);
+        }
       }
 #ifdef ZmqttDiscovery
       if (SYSdata.containsKey("discovery_prefix")) {
-        strncpy(discovery_prefix, SYSdata["discovery_prefix"], parameters_size);
+        const char* src = SYSdata["discovery_prefix"];
+        // discovery_prefix is defined in mqttDiscovery.cpp as char[parameters_size + 1].
+        if (strlen(src) > parameters_size) {
+          THEENGS_LOG_WARNING(F("discovery_prefix too long, ignoring" CR));
+        } else {
+          strcpy(discovery_prefix, src);
+        }
       }
 #endif
       if (SYSdata.containsKey("gateway_name")) {
-        strncpy(gateway_name, SYSdata["gateway_name"], parameters_size);
+        const char* src = SYSdata["gateway_name"];
+        if (strlen(src) >= sizeof(gateway_name)) {
+          THEENGS_LOG_WARNING(F("gateway_name too long, ignoring" CR));
+        } else {
+          strcpy(gateway_name, src);
+        }
       }
       if (SYSdata.containsKey("gw_pass")) {
-        strncpy(ota_pass, SYSdata["gw_pass"], parameters_size);
-        restartESP = true;
+        const char* src = SYSdata["gw_pass"];
+        if (strlen(src) >= sizeof(ota_pass)) {
+          THEENGS_LOG_WARNING(F("gw_pass too long, ignoring" CR));
+        } else {
+          strcpy(ota_pass, src);
+          restartESP = true;
+        }
       }
 #ifndef ESPWifiManualSetup
       saveConfig();
@@ -3498,7 +3545,12 @@ void XtoSYS(const char* topicOri, JsonObject& SYSdata) { // json object decoding
 #if BLEDecryptor
     if (SYSdata.containsKey("ble_aes") || SYSdata.containsKey("ble_aes_keys")) {
       if (SYSdata.containsKey("ble_aes")) {
-        strncpy(ble_aes, SYSdata["ble_aes"], parameters_size);
+        const char* src = SYSdata["ble_aes"];
+        if (strlen(src) >= sizeof(ble_aes)) {
+          THEENGS_LOG_WARNING(F("ble_aes too long, ignoring" CR));
+        } else {
+          strcpy(ble_aes, src);
+        }
       }
       if (SYSdata.containsKey("ble_aes_keys")) {
         THEENGS_LOG_WARNING(F("Contains ble_aes_keys" CR));

main/webUI.cpp

@@ -880,8 +880,17 @@ void handleCG() {
     }
     if (server.hasArg("save")) {
       if (server.hasArg("gp") && server.arg("gp").length() > 0 && strcmp(ota_pass, server.arg("gp").c_str())) {
-        strncpy(ota_pass, server.arg("gp").c_str(), parameters_size);
-        pwUpdate = true;
+        // HTML maxlength is client-side only — a direct POST can deliver any
+        // length. Reject anything that would not fit (and leave dst
+        // NUL-terminated) before copying into the fixed-size buffer.
+        const char* gp = server.arg("gp").c_str();
+        // ota_pass is extern char[] in this TU; size is parameters_size.
+        if (strlen(gp) >= parameters_size) {
+          THEENGS_LOG_WARNING(F("[WebUI] gateway password too long, ignoring" CR));
+        } else {
+          strcpy(ota_pass, gp);
+          pwUpdate = true;
+        }
       }
 #    ifdef ZmqttDiscovery
       if (SYSConfig.discovery != server.hasArg("dc")) {