security/fix: Final review corrections for Ethernet runtime config

Security fixes:
- IP validation: bounds checking for octets (0-255)
- ETH.config() return value now checked with distinct logging
- set ip 0.0.0.0 now enables DHCP (was rejected before)

Documentation:
- Fixed typo: 'thevalue' → 'the value'
- Added missing: advert.zerohop command documentation
- Clarified IP configuration behavior (DHCP, ETH_STATIC_IP fallback, reset to DHCP)

All identified issues addressed or documented as out-of-scope.
PR #2260 ready for maintainer review.

Author: ndrpri

docs/cli_commands.md

@@ -64,6 +64,12 @@ This document provides an overview of CLI commands that can be sent to MeshCore
 
 ---
 
+### Send a zero-hop advert
+**Usage:**
+- `advert.zerohop`
+
+---
+
 ### Start an Over-The-Air (OTA) firmware update
 **Usage:**
 - `start ota`
@@ -792,7 +798,7 @@ region save
 
 ---
 
-#### View or change thevalue of a sensor
+#### View or change the value of a sensor
 **Usage:** 
 - `sensor get <key>`
 - `sensor set <key> <value>`
@@ -910,7 +916,7 @@ region save
 
 **Default:** `0.0.0.0` (uses DHCP if not configured)
 
-**Note:** Requires reboot to apply. If no valid IP is set, device will use DHCP at boot.
+**Note:** Requires reboot to apply. Set to `0.0.0.0` to revert to DHCP. If `ETH_STATIC_IP` is defined in build flags and DHCP fails, that compile-time address is used as fallback.
 
 ---
 

examples/simple_repeater/main.cpp

@@ -99,12 +99,10 @@ void setup() {
 
   the_mesh.begin(fs);
 
-the_mesh.begin(fs);
-
 #ifdef USE_ETHERNET
     NodePrefs* prefs = the_mesh.getNodePrefs();
     if (prefs->eth_ip != 0) {
-      board.reconfigureEthernet(prefs->eth_ip, prefs->eth_gateway, prefs->eth_subnet);
+      board.reconfigureEthernet(prefs->eth_ip, prefs->eth_gateway, prefs->eth_subnet, prefs->eth_dns1);
    }
 #endif
 
@@ -150,7 +148,7 @@ void loop() {
   }
 
   the_mesh.loop();
-  #if defined(ESP32) && defined(TCP_CONSOLE_PORT)
+  #if defined(TCP_CONSOLE_PORT)
     tcp_console.loop(the_mesh);
   #endif
 

examples/simple_room_server/main.cpp

@@ -84,7 +84,7 @@ void setup() {
 #ifdef USE_ETHERNET
     NodePrefs* prefs = the_mesh.getNodePrefs();
     if (prefs->eth_ip != 0) {
-      board.reconfigureEthernet(prefs->eth_ip, prefs->eth_gateway, prefs->eth_subnet);
+      board.reconfigureEthernet(prefs->eth_ip, prefs->eth_gateway, prefs->eth_subnet, prefs->eth_dns1);
     }
 #endif
 

src/MeshCore.h

@@ -65,7 +65,7 @@ class MainBoard {
   virtual const char* getResetReasonString(uint32_t reason) { return "Not available"; }
   virtual uint8_t getShutdownReason() const { return 0; }
   virtual const char* getShutdownReasonString(uint8_t reason) { return "Not available"; }
-  virtual void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet) { /* no op */ }
+  virtual void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet, uint32_t dns1 = 0) { /* no op */ }
 };
 
 /**

src/helpers/CommonCLI.cpp

@@ -23,12 +23,12 @@ static bool isValidName(const char *n) {
 }
 
 // Helper functions for IP address conversion
+// Returns UINT32_MAX on parse error or out-of-range octet. Returns 0 for 0.0.0.0 (DHCP/clear).
 static uint32_t ipStringToUint32(const char* ip_str) {
-  uint32_t ip = 0;
-  uint8_t parts[4] = {0, 0, 0, 0};
-  sscanf(ip_str, "%hhu.%hhu.%hhu.%hhu", &parts[0], &parts[1], &parts[2], &parts[3]);
-  ip = ((uint32_t)parts[0] << 24) | ((uint32_t)parts[1] << 16) | ((uint32_t)parts[2] << 8) | parts[3];
-  return ip;
+  unsigned int a = 0, b = 0, c = 0, d = 0;
+  if (sscanf(ip_str, "%u.%u.%u.%u", &a, &b, &c, &d) != 4) return UINT32_MAX;
+  if (a > 255 || b > 255 || c > 255 || d > 255) return UINT32_MAX;
+  return ((uint32_t)a << 24) | ((uint32_t)b << 16) | ((uint32_t)c << 8) | d;
 }
 
 static void uint32ToIPString(uint32_t ip, char* buffer, size_t size) {
@@ -134,14 +134,6 @@ void CommonCLI::loadPrefsInt(FILESYSTEM* fs, const char* filename) {
     _prefs->gps_enabled = constrain(_prefs->gps_enabled, 0, 1);
     _prefs->advert_loc_policy = constrain(_prefs->advert_loc_policy, 0, 2);
 
-    // sanitise bad ethernet pref values
-    // If values are 0 (not set), they will be initialized from platformio.ini defaults at boot
-    if (_prefs->eth_ip == 0) _prefs->eth_ip = 0;          // 0 = use platformio.ini default
-    if (_prefs->eth_gateway == 0) _prefs->eth_gateway = 0;
-    if (_prefs->eth_subnet == 0) _prefs->eth_subnet = 0;
-    if (_prefs->eth_dns1 == 0) _prefs->eth_dns1 = 0;
-    if (_prefs->eth_dns2 == 0) _prefs->eth_dns2 = 0;
-
     file.close();
   }
 }
@@ -310,7 +302,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch
       // change admin password
       StrHelper::strncpy(_prefs->password, &command[9], sizeof(_prefs->password));
       savePrefs();
-      sprintf(reply, "password now: %s", _prefs->password);   // echo back just to let admin know for sure!!
+      strcpy(reply, "OK - password changed");
     } else if (memcmp(command, "clear stats", 11) == 0) {
       _callbacks->clearStats();
       strcpy(reply, "(OK - stats reset)");
@@ -735,39 +727,39 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch
 #ifdef USE_ETHERNET
       } else if (memcmp(config, "ip ", 3) == 0) {
         uint32_t ip = ipStringToUint32(&config[3]);
-        if (ip != 0) {
+        if (ip == UINT32_MAX) {
+          strcpy(reply, "Error: invalid IP format");
+        } else {
           _prefs->eth_ip = ip;
           savePrefs();
-          strcpy(reply, "OK - reboot to apply");
-        } else {
-          strcpy(reply, "Error: invalid IP format");
+          strcpy(reply, ip == 0 ? "OK - reboot to apply (will use DHCP)" : "OK - reboot to apply");
         }
       } else if (memcmp(config, "subnet ", 7) == 0) {
         uint32_t subnet = ipStringToUint32(&config[7]);
-        if (subnet != 0) {
+        if (subnet == UINT32_MAX) {
+          strcpy(reply, "Error: invalid subnet format");
+        } else {
           _prefs->eth_subnet = subnet;
           savePrefs();
           strcpy(reply, "OK - reboot to apply");
-        } else {
-          strcpy(reply, "Error: invalid subnet format");
-        }  
+        }
       } else if (memcmp(config, "gw ", 3) == 0) {
         uint32_t gw = ipStringToUint32(&config[3]);
-        if (gw != 0) {
+        if (gw == UINT32_MAX) {
+          strcpy(reply, "Error: invalid IP format");
+        } else {
           _prefs->eth_gateway = gw;
           savePrefs();
           strcpy(reply, "OK - reboot to apply");
-        } else {
-          strcpy(reply, "Error: invalid IP format");
         }
       } else if (memcmp(config, "dns ", 4) == 0) {
         uint32_t dns = ipStringToUint32(&config[4]);
-        if (dns != 0) {
+        if (dns == UINT32_MAX) {
+          strcpy(reply, "Error: invalid IP format");
+        } else {
           _prefs->eth_dns1 = dns;
           savePrefs();
           strcpy(reply, "OK - reboot to apply");
-        } else {
-          strcpy(reply, "Error: invalid IP format");
         }
 #endif
       } else {

src/helpers/esp32/TCPConsole.h

@@ -34,7 +34,7 @@ class TCPConsole {
   void disconnectClient(int i) {
     _clients[i].stop();
     _authenticated[i] = false;
-    _cmd_buf[i][0] = 0;
+    memset(_cmd_buf[i], 0, sizeof(_cmd_buf[i]));
     _cmd_len[i] = 0;
   }
 
@@ -43,7 +43,7 @@ class TCPConsole {
     : _server(TCP_CONSOLE_PORT), _prefs(prefs) {
     for (int i = 0; i < TCP_CONSOLE_MAX_CLIENTS; i++) {
       _authenticated[i] = false;
-      _cmd_buf[i][0] = 0;
+      memset(_cmd_buf[i], 0, sizeof(_cmd_buf[i]));
       _cmd_len[i] = 0;
       _last_active[i] = 0;
     }
@@ -62,17 +62,23 @@ class TCPConsole {
     // Accept new clients
     WiFiClient newClient = _server.available();
     if (newClient) {
+      bool found = false;
       for (int i = 0; i < TCP_CONSOLE_MAX_CLIENTS; i++) {
         if (!_clients[i] || !_clients[i].connected()) {
           _clients[i] = newClient;
           _authenticated[i] = false;
-          _cmd_buf[i][0] = 0;
+          memset(_cmd_buf[i], 0, sizeof(_cmd_buf[i]));
           _cmd_len[i] = 0;
           _last_active[i] = millis();
           sendToClient(i, "MeshCore Console\r\nPassword: ");
+          found = true;
           break;
         }
       }
+      if (!found) {
+        newClient.print("Server busy. Try again later.\r\n");
+        newClient.stop();
+      }
     }
 
     // Handle connected clients
@@ -109,8 +115,11 @@ class TCPConsole {
         _cmd_buf[i][_cmd_len[i]] = 0;
 
         if (!_authenticated[i]) {
-          // Authentication — always read from live NodePrefs, not compile-time constant
-          if (_prefs != nullptr && strcmp(_cmd_buf[i], _prefs->password) == 0) {
+          // Compare full password field with memcmp to avoid short-circuit timing
+          bool ok = _prefs != nullptr &&
+                    _cmd_len[i] == (int)strnlen(_prefs->password, sizeof(_prefs->password)) &&
+                    memcmp(_cmd_buf[i], _prefs->password, sizeof(_prefs->password)) == 0;
+          if (ok) {
             _authenticated[i] = true;
             char welcome[80];
             snprintf(welcome, sizeof(welcome), "Welcome to %s console.\r\n> ", _prefs->node_name);
@@ -134,7 +143,7 @@ class TCPConsole {
           sendToClient(i, "> ");
         }
 
-        _cmd_buf[i][0] = 0;
+        memset(_cmd_buf[i], 0, sizeof(_cmd_buf[i]));
         _cmd_len[i] = 0;
       }
     }

src/helpers/esp32/TEthEliteBoard.cpp

@@ -8,6 +8,7 @@
 #include "TEthEliteBoard.h"
 #include "target.h"
 #include "helpers/ui/MomentaryButton.h"
+#include <esp_task_wdt.h>
 
 extern MomentaryButton user_btn;
 
@@ -130,30 +131,26 @@ void TEthEliteBoard::startEthernet() {
     ETH.begin(ETH_PHY_W5500, ETH_ADDR, ETH_CS, ETH_INT, -1, SPI2_HOST, ETH_SCLK, ETH_MISO, ETH_MOSI);
     delay(100);
 
-#ifndef USE_ETHERNET
-  // Used only if USE_ETHERNET is not enabled
-  #ifdef ETH_STATIC_IP
-    IPAddress ip(ETH_STATIC_IP);
-    IPAddress gw(ETH_GATEWAY);
-    IPAddress mask(ETH_SUBNET);
-    IPAddress dns(ETH_DNS);
-    ETH.config(ip, gw, mask, dns);
-  #endif
-#endif
-
     unsigned long t0 = millis();
     while (!ETH.linkUp() && millis() - t0 < 5000) {
+        esp_task_wdt_reset();
         delay(100);
     }
 
     t0 = millis();
     while (ETH.localIP() == IPAddress(0, 0, 0, 0) && millis() - t0 < 5000) {
+        esp_task_wdt_reset();
         delay(100);
     }
 
     if (ETH.localIP() == IPAddress(0, 0, 0, 0)) {
+#ifdef ETH_STATIC_IP
+        Serial.println("DHCP timeout, using static IP from build flags");
+        ETH.config(IPAddress(ETH_STATIC_IP), IPAddress(ETH_GATEWAY), IPAddress(ETH_SUBNET), IPAddress(ETH_DNS));
+#else
         Serial.println("DHCP timeout, using fallback IP");
         ETH.config(IPAddress(192, 168, 4, 2), IPAddress(192, 168, 4, 1), IPAddress(255, 255, 255, 0));
+#endif
     }
 
     eth_local_ip = ETH.localIP().toString();  // save IP for OTA use
@@ -164,30 +161,40 @@ void TEthEliteBoard::startEthernet() {
     Serial.print("ETH IP "); Serial.println(ETH.localIP());
     Serial.println(ETH.linkUp() ? "ETH LINK UP" : "ETH LINK DOWN");
 }
-void TEthEliteBoard::reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet) {
+void TEthEliteBoard::reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet, uint32_t dns1) {
   if (ip != 0) {
     uint8_t b1 = (ip >> 24) & 0xFF;
     uint8_t b2 = (ip >> 16) & 0xFF;
     uint8_t b3 = (ip >> 8) & 0xFF;
     uint8_t b4 = ip & 0xFF;
-    
+
     uint8_t gw1 = (gw >> 24) & 0xFF;
     uint8_t gw2 = (gw >> 16) & 0xFF;
     uint8_t gw3 = (gw >> 8) & 0xFF;
     uint8_t gw4 = gw & 0xFF;
-    
+
     uint8_t sub1 = (subnet >> 24) & 0xFF;
     uint8_t sub2 = (subnet >> 16) & 0xFF;
     uint8_t sub3 = (subnet >> 8) & 0xFF;
     uint8_t sub4 = subnet & 0xFF;
-    
-    ETH.config(
+
+    uint8_t dns_1 = (dns1 >> 24) & 0xFF;
+    uint8_t dns_2 = (dns1 >> 16) & 0xFF;
+    uint8_t dns_3 = (dns1 >> 8) & 0xFF;
+    uint8_t dns_4 = dns1 & 0xFF;
+
+    bool ok = ETH.config(
       IPAddress(b1, b2, b3, b4),
       IPAddress(gw1, gw2, gw3, gw4),
-      IPAddress(sub1, sub2, sub3, sub4)
+      IPAddress(sub1, sub2, sub3, sub4),
+      IPAddress(dns_1, dns_2, dns_3, dns_4)
     );
-    Serial.printf("ETH reconfigured to %d.%d.%d.%d\n", b1, b2, b3, b4);
-    eth_local_ip = ETH.localIP().toString();  // Aggiorna IP locale per OTA
+    if (ok) {
+      Serial.printf("ETH reconfigured to %d.%d.%d.%d\n", b1, b2, b3, b4);
+    } else {
+      Serial.println("ETH reconfigure failed");
+    }
+    eth_local_ip = ETH.localIP().toString();
   }
 }
 #endif

src/helpers/esp32/TEthEliteBoard_SX1262.h

@@ -73,7 +73,7 @@ class TEthEliteBoard : public ESP32Board {
   void startNetwork();
   void startEthernet();
   void startWifi();
-  void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet);
+  void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet, uint32_t dns1 = 0);
 
   void enterDeepSleep(uint32_t secs, int pin_wake_btn) {
     esp_sleep_pd_config(ESP_PD_DOMAIN_RTC_PERIPH, ESP_PD_OPTION_ON);

src/helpers/esp32/TEthEliteBoard_SX1276.h

@@ -73,7 +73,7 @@ class TEthEliteBoard : public ESP32Board {
   void startNetwork();
   void startEthernet();
   void startWifi();
-  void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet);
+  void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet, uint32_t dns1 = 0);
 
   void enterDeepSleep(uint32_t secs, int pin_wake_btn) {
     esp_sleep_pd_config(ESP_PD_DOMAIN_RTC_PERIPH, ESP_PD_OPTION_ON);