Merge pull request #3395 from jbrazio/fix/serial-command-clamp

[HIGH] Ensure command buffers stay NUL-terminated to prevent overflow

examples/companion_radio/MyMesh.cpp

@@ -2243,6 +2243,13 @@ bool MyMesh::handleCommand(const char* command, uint32_t sender_timestamp, char*
 
 void MyMesh::checkCLIRescueCmd() {
   int len = strlen(cli_command);
+  // `cli_command` must stay NUL-terminated within its bounds. If it ever isn't,
+  // strlen() above can return >= sizeof(cli_command) and the loop below would
+  // then index past the buffer, so clamp defensively.
+  if (len >= (int)sizeof(cli_command)) {
+    cli_command[0] = 0;
+    len = 0;
+  }
   while (Serial.available() && len < sizeof(cli_command)-1) {
     char c = Serial.read();
     if (c != '\n') {
@@ -2251,8 +2258,9 @@ void MyMesh::checkCLIRescueCmd() {
     }
     Serial.print(c);  // echo
   }
-  if (len == sizeof(cli_command)-1) {  // command buffer full
+  if (len == sizeof(cli_command)-1) {  // buffer full: treat as a completed line
-    cli_command[sizeof(cli_command)-1] = '\r';
+    cli_command[sizeof(cli_command)-2] = '\r';  // place end-of-line marker inside the buffer
+    cli_command[sizeof(cli_command)-1] = 0;     // keep the buffer NUL-terminated
   }
 
   if (len > 0 && cli_command[len - 1] == '\r') {  // received complete line

examples/simple_repeater/main.cpp

@@ -125,6 +125,13 @@ void setup() {
 void loop() {
   // Handle Serial CLI
   int len = strlen(command);
+  // `command` must stay NUL-terminated within its bounds. If it ever isn't,
+  // strlen() above can return >= sizeof(command) and the loop below would then
+  // index past the buffer, so clamp defensively.
+  if (len >= (int)sizeof(command)) {
+    command[0] = 0;
+    len = 0;
+  }
   while (Serial.available() && len < sizeof(command)-1) {
     char c = Serial.read();
     if (c != '\n') {
@@ -134,8 +141,9 @@ void loop() {
     }
     if (c == '\r') break;
   }
-  if (len == sizeof(command)-1) {  // command buffer full
+  if (len == sizeof(command)-1) {  // buffer full: treat as a completed line
-    command[sizeof(command)-1] = '\r';
+    command[sizeof(command)-2] = '\r';  // place end-of-line marker inside the buffer
+    command[sizeof(command)-1] = 0;     // keep the buffer NUL-terminated
   }
 
   if (len > 0 && command[len - 1] == '\r') {  // received complete line

examples/simple_room_server/main.cpp

@@ -105,6 +105,13 @@ void setup() {
 
 void loop() {
   int len = strlen(command);
+  // `command` must stay NUL-terminated within its bounds. If it ever isn't,
+  // strlen() above can return >= sizeof(command) and the loop below would then
+  // index past the buffer, so clamp defensively.
+  if (len >= (int)sizeof(command)) {
+    command[0] = 0;
+    len = 0;
+  }
   while (Serial.available() && len < sizeof(command)-1) {
     char c = Serial.read();
     if (c != '\n') {
@@ -113,8 +120,9 @@ void loop() {
     }
     Serial.print(c);
   }
-  if (len == sizeof(command)-1) {  // command buffer full
+  if (len == sizeof(command)-1) {  // buffer full: treat as a completed line
-    command[sizeof(command)-1] = '\r';
+    command[sizeof(command)-2] = '\r';  // place end-of-line marker inside the buffer
+    command[sizeof(command)-1] = 0;     // keep the buffer NUL-terminated
   }
 
   if (len > 0 && command[len - 1] == '\r') {  // received complete line

examples/simple_secure_chat/main.cpp

@@ -530,6 +530,13 @@ public:
     BaseChatMesh::loop();
 
     int len = strlen(command);
+    // `command` must stay NUL-terminated within its bounds. If it ever isn't,
+    // strlen() above can return >= sizeof(command) and the loop below would then
+    // index past the buffer, so clamp defensively.
+    if (len >= (int)sizeof(command)) {
+      command[0] = 0;
+      len = 0;
+    }
     while (Serial.available() && len < sizeof(command)-1) {
       char c = Serial.read();
       if (c != '\n') { 
@@ -538,8 +545,9 @@ public:
       }
       Serial.print(c);
     }
-    if (len == sizeof(command)-1) {  // command buffer full
+    if (len == sizeof(command)-1) {  // buffer full: treat as a completed line
-      command[sizeof(command)-1] = '\r';
+      command[sizeof(command)-2] = '\r';  // place end-of-line marker inside the buffer
+      command[sizeof(command)-1] = 0;     // keep the buffer NUL-terminated
     }
 
     if (len > 0 && command[len - 1] == '\r') {  // received complete line

examples/simple_sensor/main.cpp

@@ -122,6 +122,13 @@ void setup() {
 
 void loop() {
   int len = strlen(command);
+  // `command` must stay NUL-terminated within its bounds. If it ever isn't,
+  // strlen() above can return >= sizeof(command) and the loop below would then
+  // index past the buffer, so clamp defensively.
+  if (len >= (int)sizeof(command)) {
+    command[0] = 0;
+    len = 0;
+  }
   while (Serial.available() && len < sizeof(command)-1) {
     char c = Serial.read();
     if (c != '\n') {
@@ -130,8 +137,9 @@ void loop() {
     }
     Serial.print(c);
   }
-  if (len == sizeof(command)-1) {  // command buffer full
+  if (len == sizeof(command)-1) {  // buffer full: treat as a completed line
-    command[sizeof(command)-1] = '\r';
+    command[sizeof(command)-2] = '\r';  // place end-of-line marker inside the buffer
+    command[sizeof(command)-1] = 0;     // keep the buffer NUL-terminated
   }
 
   if (len > 0 && command[len - 1] == '\r') {  // received complete line