Skip to content

Commit b3c36bd

Browse files
committed
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.
1 parent 0118ba6 commit b3c36bd

11 files changed

Lines changed: 70 additions & 60 deletions

File tree

docs/cli_commands.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ region save
900900

901901
---
902902

903-
#### View or change thevalue of a sensor
903+
#### View or change the value of a sensor
904904
**Usage:**
905905
- `sensor get <key>`
906906
- `sensor set <key> <value>`

examples/simple_repeater/main.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,10 @@ void setup() {
104104

105105
the_mesh.begin(fs);
106106

107-
the_mesh.begin(fs);
108-
109107
#ifdef USE_ETHERNET
110108
NodePrefs* prefs = the_mesh.getNodePrefs();
111109
if (prefs->eth_ip != 0) {
112-
board.reconfigureEthernet(prefs->eth_ip, prefs->eth_gateway, prefs->eth_subnet);
110+
board.reconfigureEthernet(prefs->eth_ip, prefs->eth_gateway, prefs->eth_subnet, prefs->eth_dns1);
113111
}
114112
#endif
115113

@@ -170,7 +168,7 @@ void loop() {
170168
#endif
171169

172170
the_mesh.loop();
173-
#if defined(ESP32) && defined(TCP_CONSOLE_PORT)
171+
#if defined(TCP_CONSOLE_PORT)
174172
tcp_console.loop(the_mesh);
175173
#endif
176174

examples/simple_room_server/main.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ void setup() {
8484
#ifdef USE_ETHERNET
8585
NodePrefs* prefs = the_mesh.getNodePrefs();
8686
if (prefs->eth_ip != 0) {
87-
board.reconfigureEthernet(prefs->eth_ip, prefs->eth_gateway, prefs->eth_subnet);
87+
board.reconfigureEthernet(prefs->eth_ip, prefs->eth_gateway, prefs->eth_subnet, prefs->eth_dns1);
8888
}
8989
#endif
9090

src/MeshCore.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class MainBoard {
6666
virtual const char* getResetReasonString(uint32_t reason) { return "Not available"; }
6767
virtual uint8_t getShutdownReason() const { return 0; }
6868
virtual const char* getShutdownReasonString(uint8_t reason) { return "Not available"; }
69-
virtual void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet) { /* no op */ }
69+
virtual void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet, uint32_t dns1 = 0) { /* no op */ }
7070
};
7171

7272
/**

src/helpers/CommonCLI.cpp

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ static bool isValidName(const char *n) {
2727
}
2828

2929
// Helper functions for IP address conversion
30+
// Returns UINT32_MAX on parse error or out-of-range octet. Returns 0 for 0.0.0.0 (DHCP/clear).
3031
static uint32_t ipStringToUint32(const char* ip_str) {
31-
uint32_t ip = 0;
32-
uint8_t parts[4] = {0, 0, 0, 0};
33-
sscanf(ip_str, "%hhu.%hhu.%hhu.%hhu", &parts[0], &parts[1], &parts[2], &parts[3]);
34-
ip = ((uint32_t)parts[0] << 24) | ((uint32_t)parts[1] << 16) | ((uint32_t)parts[2] << 8) | parts[3];
35-
return ip;
32+
unsigned int a = 0, b = 0, c = 0, d = 0;
33+
if (sscanf(ip_str, "%u.%u.%u.%u", &a, &b, &c, &d) != 4) return UINT32_MAX;
34+
if (a > 255 || b > 255 || c > 255 || d > 255) return UINT32_MAX;
35+
return ((uint32_t)a << 24) | ((uint32_t)b << 16) | ((uint32_t)c << 8) | d;
3636
}
3737

3838
static void uint32ToIPString(uint32_t ip, char* buffer, size_t size) {
@@ -141,6 +141,7 @@ void CommonCLI::loadPrefsInt(FILESYSTEM* fs, const char* filename) {
141141
// sanitise settings
142142
_prefs->rx_boosted_gain = constrain(_prefs->rx_boosted_gain, 0, 1); // boolean
143143

144+
144145
file.close();
145146
}
146147
}
@@ -311,7 +312,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch
311312
// change admin password
312313
StrHelper::strncpy(_prefs->password, &command[9], sizeof(_prefs->password));
313314
savePrefs();
314-
sprintf(reply, "password now: %s", _prefs->password); // echo back just to let admin know for sure!!
315+
strcpy(reply, "OK - password changed");
315316
} else if (memcmp(command, "clear stats", 11) == 0) {
316317
_callbacks->clearStats();
317318
strcpy(reply, "(OK - stats reset)");
@@ -764,39 +765,39 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch
764765
#ifdef USE_ETHERNET
765766
} else if (memcmp(config, "ip ", 3) == 0) {
766767
uint32_t ip = ipStringToUint32(&config[3]);
767-
if (ip != 0) {
768+
if (ip == UINT32_MAX) {
769+
strcpy(reply, "Error: invalid IP format");
770+
} else {
768771
_prefs->eth_ip = ip;
769772
savePrefs();
770-
strcpy(reply, "OK - reboot to apply");
771-
} else {
772-
strcpy(reply, "Error: invalid IP format");
773+
strcpy(reply, ip == 0 ? "OK - reboot to apply (will use DHCP)" : "OK - reboot to apply");
773774
}
774775
} else if (memcmp(config, "subnet ", 7) == 0) {
775776
uint32_t subnet = ipStringToUint32(&config[7]);
776-
if (subnet != 0) {
777+
if (subnet == UINT32_MAX) {
778+
strcpy(reply, "Error: invalid subnet format");
779+
} else {
777780
_prefs->eth_subnet = subnet;
778781
savePrefs();
779782
strcpy(reply, "OK - reboot to apply");
780-
} else {
781-
strcpy(reply, "Error: invalid subnet format");
782-
}
783+
}
783784
} else if (memcmp(config, "gw ", 3) == 0) {
784785
uint32_t gw = ipStringToUint32(&config[3]);
785-
if (gw != 0) {
786+
if (gw == UINT32_MAX) {
787+
strcpy(reply, "Error: invalid IP format");
788+
} else {
786789
_prefs->eth_gateway = gw;
787790
savePrefs();
788791
strcpy(reply, "OK - reboot to apply");
789-
} else {
790-
strcpy(reply, "Error: invalid IP format");
791792
}
792793
} else if (memcmp(config, "dns ", 4) == 0) {
793794
uint32_t dns = ipStringToUint32(&config[4]);
794-
if (dns != 0) {
795+
if (dns == UINT32_MAX) {
796+
strcpy(reply, "Error: invalid IP format");
797+
} else {
795798
_prefs->eth_dns1 = dns;
796799
savePrefs();
797800
strcpy(reply, "OK - reboot to apply");
798-
} else {
799-
strcpy(reply, "Error: invalid IP format");
800801
}
801802
#endif
802803
} else {

src/helpers/esp32/TCPConsole.h

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class TCPConsole {
3434
void disconnectClient(int i) {
3535
_clients[i].stop();
3636
_authenticated[i] = false;
37-
_cmd_buf[i][0] = 0;
37+
memset(_cmd_buf[i], 0, sizeof(_cmd_buf[i]));
3838
_cmd_len[i] = 0;
3939
}
4040

@@ -43,7 +43,7 @@ class TCPConsole {
4343
: _server(TCP_CONSOLE_PORT), _prefs(prefs) {
4444
for (int i = 0; i < TCP_CONSOLE_MAX_CLIENTS; i++) {
4545
_authenticated[i] = false;
46-
_cmd_buf[i][0] = 0;
46+
memset(_cmd_buf[i], 0, sizeof(_cmd_buf[i]));
4747
_cmd_len[i] = 0;
4848
_last_active[i] = 0;
4949
}
@@ -62,17 +62,23 @@ class TCPConsole {
6262
// Accept new clients
6363
WiFiClient newClient = _server.available();
6464
if (newClient) {
65+
bool found = false;
6566
for (int i = 0; i < TCP_CONSOLE_MAX_CLIENTS; i++) {
6667
if (!_clients[i] || !_clients[i].connected()) {
6768
_clients[i] = newClient;
6869
_authenticated[i] = false;
69-
_cmd_buf[i][0] = 0;
70+
memset(_cmd_buf[i], 0, sizeof(_cmd_buf[i]));
7071
_cmd_len[i] = 0;
7172
_last_active[i] = millis();
7273
sendToClient(i, "MeshCore Console\r\nPassword: ");
74+
found = true;
7375
break;
7476
}
7577
}
78+
if (!found) {
79+
newClient.print("Server busy. Try again later.\r\n");
80+
newClient.stop();
81+
}
7682
}
7783

7884
// Handle connected clients
@@ -109,8 +115,11 @@ class TCPConsole {
109115
_cmd_buf[i][_cmd_len[i]] = 0;
110116

111117
if (!_authenticated[i]) {
112-
// Authentication — always read from live NodePrefs, not compile-time constant
113-
if (_prefs != nullptr && strcmp(_cmd_buf[i], _prefs->password) == 0) {
118+
// Compare full password field with memcmp to avoid short-circuit timing
119+
bool ok = _prefs != nullptr &&
120+
_cmd_len[i] == (int)strnlen(_prefs->password, sizeof(_prefs->password)) &&
121+
memcmp(_cmd_buf[i], _prefs->password, sizeof(_prefs->password)) == 0;
122+
if (ok) {
114123
_authenticated[i] = true;
115124
char welcome[80];
116125
snprintf(welcome, sizeof(welcome), "Welcome to %s console.\r\n> ", _prefs->node_name);
@@ -134,7 +143,7 @@ class TCPConsole {
134143
sendToClient(i, "> ");
135144
}
136145

137-
_cmd_buf[i][0] = 0;
146+
memset(_cmd_buf[i], 0, sizeof(_cmd_buf[i]));
138147
_cmd_len[i] = 0;
139148
}
140149
}

src/helpers/esp32/TEthEliteBoard.cpp

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "TEthEliteBoard.h"
99
#include "target.h"
1010
#include "helpers/ui/MomentaryButton.h"
11+
#include <esp_task_wdt.h>
1112

1213
extern MomentaryButton user_btn;
1314

@@ -130,30 +131,26 @@ void TEthEliteBoard::startEthernet() {
130131
ETH.begin(ETH_PHY_W5500, ETH_ADDR, ETH_CS, ETH_INT, -1, SPI2_HOST, ETH_SCLK, ETH_MISO, ETH_MOSI);
131132
delay(100);
132133

133-
#ifndef USE_ETHERNET
134-
// Used only if USE_ETHERNET is not enabled
135-
#ifdef ETH_STATIC_IP
136-
IPAddress ip(ETH_STATIC_IP);
137-
IPAddress gw(ETH_GATEWAY);
138-
IPAddress mask(ETH_SUBNET);
139-
IPAddress dns(ETH_DNS);
140-
ETH.config(ip, gw, mask, dns);
141-
#endif
142-
#endif
143-
144134
unsigned long t0 = millis();
145135
while (!ETH.linkUp() && millis() - t0 < 5000) {
136+
esp_task_wdt_reset();
146137
delay(100);
147138
}
148139

149140
t0 = millis();
150141
while (ETH.localIP() == IPAddress(0, 0, 0, 0) && millis() - t0 < 5000) {
142+
esp_task_wdt_reset();
151143
delay(100);
152144
}
153145

154146
if (ETH.localIP() == IPAddress(0, 0, 0, 0)) {
147+
#ifdef ETH_STATIC_IP
148+
Serial.println("DHCP timeout, using static IP from build flags");
149+
ETH.config(IPAddress(ETH_STATIC_IP), IPAddress(ETH_GATEWAY), IPAddress(ETH_SUBNET), IPAddress(ETH_DNS));
150+
#else
155151
Serial.println("DHCP timeout, using fallback IP");
156152
ETH.config(IPAddress(192, 168, 4, 2), IPAddress(192, 168, 4, 1), IPAddress(255, 255, 255, 0));
153+
#endif
157154
}
158155

159156
eth_local_ip = ETH.localIP().toString(); // save IP for OTA use
@@ -164,30 +161,40 @@ void TEthEliteBoard::startEthernet() {
164161
Serial.print("ETH IP "); Serial.println(ETH.localIP());
165162
Serial.println(ETH.linkUp() ? "ETH LINK UP" : "ETH LINK DOWN");
166163
}
167-
void TEthEliteBoard::reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet) {
164+
void TEthEliteBoard::reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet, uint32_t dns1) {
168165
if (ip != 0) {
169166
uint8_t b1 = (ip >> 24) & 0xFF;
170167
uint8_t b2 = (ip >> 16) & 0xFF;
171168
uint8_t b3 = (ip >> 8) & 0xFF;
172169
uint8_t b4 = ip & 0xFF;
173-
170+
174171
uint8_t gw1 = (gw >> 24) & 0xFF;
175172
uint8_t gw2 = (gw >> 16) & 0xFF;
176173
uint8_t gw3 = (gw >> 8) & 0xFF;
177174
uint8_t gw4 = gw & 0xFF;
178-
175+
179176
uint8_t sub1 = (subnet >> 24) & 0xFF;
180177
uint8_t sub2 = (subnet >> 16) & 0xFF;
181178
uint8_t sub3 = (subnet >> 8) & 0xFF;
182179
uint8_t sub4 = subnet & 0xFF;
183-
184-
ETH.config(
180+
181+
uint8_t dns_1 = (dns1 >> 24) & 0xFF;
182+
uint8_t dns_2 = (dns1 >> 16) & 0xFF;
183+
uint8_t dns_3 = (dns1 >> 8) & 0xFF;
184+
uint8_t dns_4 = dns1 & 0xFF;
185+
186+
bool ok = ETH.config(
185187
IPAddress(b1, b2, b3, b4),
186188
IPAddress(gw1, gw2, gw3, gw4),
187-
IPAddress(sub1, sub2, sub3, sub4)
189+
IPAddress(sub1, sub2, sub3, sub4),
190+
IPAddress(dns_1, dns_2, dns_3, dns_4)
188191
);
189-
Serial.printf("ETH reconfigured to %d.%d.%d.%d\n", b1, b2, b3, b4);
190-
eth_local_ip = ETH.localIP().toString(); // Aggiorna IP locale per OTA
192+
if (ok) {
193+
Serial.printf("ETH reconfigured to %d.%d.%d.%d\n", b1, b2, b3, b4);
194+
} else {
195+
Serial.println("ETH reconfigure failed");
196+
}
197+
eth_local_ip = ETH.localIP().toString();
191198
}
192199
}
193200
#endif

src/helpers/esp32/TEthEliteBoard_SX1262.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class TEthEliteBoard : public ESP32Board {
7373
void startNetwork();
7474
void startEthernet();
7575
void startWifi();
76-
void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet);
76+
void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet, uint32_t dns1 = 0);
7777

7878
void enterDeepSleep(uint32_t secs, int pin_wake_btn) {
7979
esp_sleep_pd_config(ESP_PD_DOMAIN_RTC_PERIPH, ESP_PD_OPTION_ON);

src/helpers/esp32/TEthEliteBoard_SX1276.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class TEthEliteBoard : public ESP32Board {
7373
void startNetwork();
7474
void startEthernet();
7575
void startWifi();
76-
void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet);
76+
void reconfigureEthernet(uint32_t ip, uint32_t gw, uint32_t subnet, uint32_t dns1 = 0);
7777

7878
void enterDeepSleep(uint32_t secs, int pin_wake_btn) {
7979
esp_sleep_pd_config(ESP_PD_DOMAIN_RTC_PERIPH, ESP_PD_OPTION_ON);

variants/lilygo_t_eth_elite_sx1262/platformio.ini

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ lib_deps =
3030
stevemarple/MicroNMEA @ ^2.0.6
3131
adafruit/Adafruit BME280 Library @ ^2.3.0
3232

33-
; === LilyGo T-ETH-Elite with SX1262 environments ===
34-
3533
[env:LilyGo_T_ETH_Elite_SX1262_repeater]
3634
extends = LilyGo_T_ETH_Elite_SX1262
3735
build_flags =
@@ -87,7 +85,6 @@ lib_deps =
8785

8886
[env:LilyGo_T_ETH_Elite_SX1262_repeater_bridge_espnow_eth]
8987
extends = LilyGo_T_ETH_Elite_SX1262
90-
upload_speed = 115200
9188
build_flags =
9289
${LilyGo_T_ETH_Elite_SX1262.build_flags}
9390
-D ADVERT_NAME='"T-ETH Elite SX1262 ESPNow Bridge eth"'
@@ -97,7 +94,7 @@ build_flags =
9794
-D MAX_NEIGHBOURS=50
9895
-D WITH_ESPNOW_BRIDGE=1
9996
-D USE_ETHERNET
100-
-D ETH_STATIC_IP=192,168,254,21
97+
-D ETH_STATIC_IP=192,168,254,20
10198
-D ETH_GATEWAY=192,168,254,254
10299
-D ETH_SUBNET=255,255,255,0
103100
-D ETH_DNS=8,8,8,8

0 commit comments

Comments
 (0)