Use strlcpy with buffer size

thinkyhead · thinkyhead · commit 51e6b5b26255 · 2023-12-08T00:08:35.000-06:00
diff --git a/Marlin/src/HAL/LINUX/include/Arduino.h b/Marlin/src/HAL/LINUX/include/Arduino.h
@@ -28,6 +28,9 @@
 
 #include <pinmapping.h>
 
+#define strlcpy(A, B, C)   strncpy(A, B, (C) - 1)
+#define strlcpy_P(A, B, C) strncpy_P(A, B, (C) - 1)
+
 #define HIGH         0x01
 #define LOW          0x00
 
diff --git a/Marlin/src/core/mstring.h b/Marlin/src/core/mstring.h
@@ -48,7 +48,7 @@
   #define DEFAULT_MSTRING_SIZE 20
 #endif
 
-//#define UNSAFE_MSTRING  // Don't initialize the string and don't terminate strncpy
+//#define UNSAFE_MSTRING  // Don't initialize the string to "" or set a terminating nul
 //#define USE_SPRINTF     // Use sprintf instead of snprintf
 //#define DJB2_HASH       // 32-bit hash with Djb2 algorithm
 //#define MSTRING_DEBUG   // Debug string operations to diagnose memory leaks
@@ -98,26 +98,20 @@ class MString {
 
   void debug(FSTR_P const f) {
     #if ENABLED(MSTRING_DEBUG)
-      SERIAL_ECHO(FTOP(f));
-      SERIAL_CHAR(':');
-      SERIAL_ECHO(uintptr_t(str));
-      SERIAL_CHAR(' ');
-      SERIAL_ECHO(length());
-      SERIAL_CHAR(' ');
-      SERIAL_ECHOLN(str);
+      SERIAL_ECHOLN(f, ':', uintptr_t(str), ' ', length(), ' ', str);
     #endif
   }
 
   void safety(const int n) { if (SAFE && n <= SIZE) str[n] = '\0'; }
 
   // Chainable String Setters
   MString& set()                          { str[0] = '\0';                            debug(F("clear"));   return *this; }
-  MString& set(char *s)                   { strncpy(str, s, SIZE);                    debug(F("string"));  return *this; }
+  MString& set(char *s)                   { strlcpy(str, s, SIZE + 1);                debug(F("string"));  return *this; }
   MString& set(const char *s)             { return set(const_cast<char*>(s)); }
-  MString& set_P(PGM_P const s)           { strncpy_P(str, s, SIZE);                  debug(F("pstring")); return *this; }
+  MString& set_P(PGM_P const s)           { strlcpy_P(str, s, SIZE + 1);              debug(F("pstring")); return *this; }
   MString& set(FSTR_P const f)            { return set_P(FTOP(f)); }
   MString& set(const bool &b)             { return set(b ? F("true") : F("false")); }
-  MString& set(const char c)              { str[0] = c; if (1 < SIZE) str[1] = '\0';  debug(F("char"));    return *this; }
+  MString& set(const char c)              { str[0] = c; str[1] = '\0';                debug(F("char"));    return *this; }
   MString& set(const int8_t &i)           { SNPRINTF_P(str, SIZE, PSTR("%d"),  i);    debug(F("int8_t"));  return *this; }
   MString& set(const short &i)            { SNPRINTF_P(str, SIZE, PSTR("%d"),  i);    debug(F("short"));   return *this; }
   MString& set(const int &i)              { SNPRINTF_P(str, SIZE, PSTR("%d"),  i);    debug(F("int"));     return *this; }
@@ -134,11 +128,11 @@ class MString {
   MString& set(const xyze_pos_t &v)       { set(); return append(v); }
 
   template <int S>
-  MString& set(const MString<S> &m)       { strncpy(str, &m, SIZE); debug(F("MString")); return *this; }
+  MString& set(const MString<S> &m)       { strlcpy(str, &m, SIZE + 1); debug(F("MString")); return *this; }
 
-  MString& setn(char *s, int len)         { int c = _MIN(len, SIZE); strncpy(str, s, c); str[c] = '\0'; debug(F("string")); return *this; }
+  MString& setn(char *s, int len)         { int c = _MIN(len, SIZE); strlcpy(str, s, c + 1); debug(F("string")); return *this; }
   MString& setn(const char *s, int len)   { return setn(const_cast<char*>(s), len); }
-  MString& setn_P(PGM_P const s, int len) { int c = _MIN(len, SIZE); strncpy_P(str, s, c); str[c] = '\0'; debug(F("pstring")); return *this; }
+  MString& setn_P(PGM_P const s, int len) { int c = _MIN(len, SIZE); strlcpy_P(str, s, c + 1); debug(F("pstring")); return *this; }
   MString& setn(FSTR_P const f, int len)  { return setn_P(FTOP(f), len); }
 
   // set(repchr_t('-', 10))
@@ -159,9 +153,9 @@ class MString {
 
   // Chainable String appenders
   MString& append()                           { debug(F("nil")); return *this; } // for macros that might emit no output
-  MString& append(char *s)                    { int sz = length(); if (sz < SIZE) strncpy(str + sz, s, SIZE - sz); debug(F("string")); return *this; }
+  MString& append(char *s)                    { int sz = length(); if (sz < SIZE) strlcpy(str + sz, s, SIZE - sz + 1); debug(F("string")); return *this; }
   MString& append(const char *s)              { return append(const_cast<char *>(s)); }
-  MString& append_P(PGM_P const s)            { int sz = length(); if (sz < SIZE) strncpy_P(str + sz, s, SIZE - sz); debug(F("pstring")); return *this; }
+  MString& append_P(PGM_P const s)            { int sz = length(); if (sz < SIZE) strlcpy_P(str + sz, s, SIZE - sz + 1); debug(F("pstring")); return *this; }
   MString& append(FSTR_P const f)             { return append_P(FTOP(f)); }
   MString& append(const bool &b)              { return append(b ? F("true") : F("false")); }
   MString& append(const char c)               { int sz = length(); if (sz < SIZE) { str[sz] = c; if (sz < SIZE - 1) str[sz + 1] = '\0'; } return *this; }
@@ -195,15 +189,15 @@ class MString {
   MString& append(const MString<S> &m)        { return append(&m); }
 
   // Append only if the given space is available
-  MString& appendn(char *s, int len)          { int sz = length(), c = _MIN(len, SIZE - sz); if (c > 0) { strncpy(str + sz, s, c); str[sz + c] = '\0'; } debug(F("string")); return *this; }
+  MString& appendn(char *s, int len)          { int sz = length(), c = _MIN(len, SIZE - sz); if (c > 0) { strlcpy(str + sz, s, c + 1); } debug(F("string")); return *this; }
   MString& appendn(const char *s, int len)    { return appendn(const_cast<char *>(s), len); }
-  MString& appendn_P(PGM_P const s, int len)  { int sz = length(), c = _MIN(len, SIZE - sz); if (c > 0) { strncpy_P(str + sz, s, c); str[sz + c] = '\0'; } debug(F("pstring")); return *this; }
+  MString& appendn_P(PGM_P const s, int len)  { int sz = length(), c = _MIN(len, SIZE - sz); if (c > 0) { strlcpy_P(str + sz, s, c + 1); } debug(F("pstring")); return *this; }
   MString& appendn(FSTR_P const f, int len)   { return appendn_P(FTOP(f), len); }
 
   // append(repchr_t('-', 10))
   MString& append(const repchr_t &s) {
     const int sz = length(), c = _MIN(s.count, SIZE - sz);
-    if (c > 0) { memset(str + sz, s.asc, c); safety(sz + c); }
+    if (c > 0) { memset(str + sz, s.asc, c); str[sz + c] = '\0'; }
     debug(F("repchr"));
     return *this;
   }
@@ -299,7 +293,7 @@ class MString {
   }
 
   void copyto(char * const dst) const { strcpy(dst, str); }
-  void copyto(char * const dst, int len) const { strncpy(dst, str, len); }
+  void copyto(char * const dst, int len) const { strlcpy(dst, str, len + 1); }
 
   MString& clear() { return set(); }
   MString& eol() { return append('\n'); }
@@ -318,6 +312,7 @@ class MString {
 
 #pragma GCC diagnostic pop
 
+// Temporary inline string typically used to compose a G-code command
 #ifndef TS_SIZE
   #define TS_SIZE 63
 #endif
diff --git a/Marlin/src/gcode/queue.h b/Marlin/src/gcode/queue.h
@@ -134,7 +134,7 @@ class GCodeQueue {
    * Aborts the current SRAM queue so only use for one or two commands.
    */
   static void inject(const char * const gcode) {
-    strncpy(injected_commands, gcode, sizeof(injected_commands) - 1);
+    strlcpy(injected_commands, gcode, sizeof(injected_commands));
   }
 
   /**
diff --git a/Marlin/src/lcd/extui/anycubic_chiron/chiron_tft.cpp b/Marlin/src/lcd/extui/anycubic_chiron/chiron_tft.cpp
@@ -433,12 +433,12 @@ void ChironTFT::sendFileList(int8_t startindex) {
 }
 
 void ChironTFT::selectFile() {
-  const size_t namelen = command_len - 4 + (panel_type <= AC_panel_new);
-  strncpy(selectedfile, panel_command + 4, namelen);
-  selectedfile[namelen] = '\0';
+  const size_t fnlen = command_len - 4 + (panel_type <= AC_panel_new);
+  strlcpy(selectedfile, panel_command + 4, fnlen + 1);
   #if ACDEBUG(AC_FILE)
     DEBUG_ECHOLNPGM(" Selected File: ", selectedfile);
   #endif
+
   switch (selectedfile[0]) {
     case '/':   // Valid file selected
       tftSendLn(AC_msg_sd_file_open_success);
@@ -449,10 +449,9 @@ void ChironTFT::selectFile() {
       tftSendLn(AC_msg_sd_file_open_failed);
       sendFileList( 0 );
       break;
-    default:   // enter sub folder
-      // for new panel remove the '.GCO' tag that was added to the end of the path
-      if (panel_type <= AC_panel_new)
-        selectedfile[strlen(selectedfile) - 4] = '\0';
+    default:    // enter subfolder
+      // For new panel remove the '.GCO' tag that was added to the end of the path
+      if (panel_type <= AC_panel_new) selectedfile[fnlen - 4] = '\0';
       filenavigator.changeDIR(selectedfile);
       tftSendLn(AC_msg_sd_file_open_failed);
       sendFileList( 0 );
diff --git a/Marlin/src/lcd/extui/anycubic_vyper/dgus_tft.cpp b/Marlin/src/lcd/extui/anycubic_vyper/dgus_tft.cpp
@@ -971,8 +971,7 @@ namespace Anycubic {
   }
 
   void DgusTFT::selectFile() {
-    strncpy(selectedfile, panel_command + 4, command_len - 4);
-    selectedfile[command_len - 5] = '\0';
+    strlcpy(selectedfile, panel_command + 4, command_len - 3);
     #if ACDEBUG(AC_FILE)
       DEBUG_ECHOLNPGM(" Selected File: ", selectedfile);
     #endif
@@ -1293,8 +1292,7 @@ namespace Anycubic {
             TERN_(CASE_LIGHT_ENABLE, setCaseLightState(true));
 
             char str_buf[20];
-            strncpy_P(str_buf, filenavigator.filelist.longFilename(), 17);
-            str_buf[17] = '\0';
+            strlcpy_P(str_buf, filenavigator.filelist.longFilename(), 18);
             sendTxtToTFT(str_buf, TXT_PRINT_NAME);
 
             #if ENABLED(POWER_LOSS_RECOVERY)
@@ -1332,8 +1330,7 @@ namespace Anycubic {
             printFile(filenavigator.filelist.shortFilename());
 
             char str_buf[20];
-            strncpy_P(str_buf, filenavigator.filelist.longFilename(), 17);
-            str_buf[17] = '\0';
+            strlcpy_P(str_buf, filenavigator.filelist.longFilename(), 18);
             sendTxtToTFT(str_buf, TXT_PRINT_NAME);
 
             sprintf(str_buf, "%5.2f", getFeedrate_percent());
diff --git a/Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp b/Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp
@@ -155,7 +155,7 @@ static void lv_kb_event_cb(lv_obj_t *kb, lv_event_t event) {
         #endif // MKS_WIFI_MODULE
         case autoLevelGcodeCommand:
           uint8_t buf[100];
-          strncpy((char *)buf, ret_ta_txt, sizeof(buf));
+          strlcpy((char *)buf, ret_ta_txt, sizeof(buf));
           update_gcode_command(AUTO_LEVELING_COMMAND_ADDR, buf);
           goto_previous_ui();
           break;
diff --git a/Marlin/src/lcd/extui/mks_ui/draw_print_file.cpp b/Marlin/src/lcd/extui/mks_ui/draw_print_file.cpp
@@ -474,7 +474,7 @@ void cutFileName(char *path, int len, int bytePerLine, char *outStr) {
         wcscpy(outStr, beginIndex);
     #else
       if ((int)strlen(beginIndex) > len)
-        strncpy(outStr, beginIndex, len);
+        strlcpy(outStr, beginIndex, len + 1);
       else
         strcpy(outStr, beginIndex);
     #endif
@@ -485,17 +485,17 @@ void cutFileName(char *path, int len, int bytePerLine, char *outStr) {
         wcsncpy(outStr, (const WCHAR *)beginIndex, len - 3);
         wcscat(outStr, (const WCHAR *)gFileTail);
       #else
-        //strncpy(outStr, beginIndex, len - 3);
-        strncpy(outStr, beginIndex, len - 4);
+        strlcpy(outStr, beginIndex, len - 3);
         strcat_P(outStr, PSTR("~.g"));
       #endif
     }
     else {
+      const size_t strsize = strIndex2 - beginIndex + 1;
       #if _LFN_UNICODE
-        wcsncpy(outStr, (const WCHAR *)beginIndex, strIndex2 - beginIndex + 1);
+        wcsncpy(outStr, (const WCHAR *)beginIndex, strsize);
         wcscat(outStr, (const WCHAR *)&gFileTail[3]);
       #else
-        strncpy(outStr, beginIndex, strIndex2 - beginIndex + 1);
+        strlcpy(outStr, beginIndex, strsize + 1);
         strcat_P(outStr, PSTR("g"));
       #endif
     }
diff --git a/Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp b/Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp
@@ -727,8 +727,7 @@ void disp_assets_update_progress(FSTR_P const fmsg) {
     static constexpr int buflen = 30;
     char buf[buflen];
     memset(buf, ' ', buflen);
-    strncpy_P(buf, FTOP(fmsg), buflen - 1);
-    buf[buflen - 1] = '\0';
+    strlcpy_P(buf, FTOP(fmsg), buflen);
     disp_string(100, 165, buf, 0xFFFF, 0x0000);
   #else
     disp_string(100, 165, FTOP(fmsg), 0xFFFF, 0x0000);
diff --git a/Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp b/Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp
@@ -237,11 +237,11 @@ void tft_lvgl_init() {
       uiCfg.print_state = REPRINTING;
 
       #if ENABLED(LONG_FILENAME_HOST_SUPPORT)
-        strncpy(public_buf_m, recovery.info.sd_filename, sizeof(public_buf_m));
+        strlcpy(public_buf_m, recovery.info.sd_filename, sizeof(public_buf_m));
         card.printLongPath(public_buf_m);
-        strncpy(list_file.long_name[sel_id], card.longFilename, sizeof(list_file.long_name[0]));
+        strlcpy(list_file.long_name[sel_id], card.longFilename, sizeof(list_file.long_name[0]));
       #else
-        strncpy(list_file.long_name[sel_id], recovery.info.sd_filename, sizeof(list_file.long_name[0]));
+        strlcpy(list_file.long_name[sel_id], recovery.info.sd_filename, sizeof(list_file.long_name[0]));
       #endif
       lv_draw_printing();
     }
diff --git a/Marlin/src/lcd/extui/nextion/nextion_tft.cpp b/Marlin/src/lcd/extui/nextion/nextion_tft.cpp
@@ -158,8 +158,7 @@ void NextionTFT::sendFileList(int8_t startindex) {
 }
 
 void NextionTFT::selectFile() {
-  strncpy(selectedfile, nextion_command + 4, command_len - 4);
-  selectedfile[command_len - 5] = '\0';
+  strlcpy(selectedfile, nextion_command + 4, command_len - 3);
   #if NEXDEBUG(N_FILE)
     DEBUG_ECHOLNPGM(" Selected File: ", selectedfile);
   #endif
diff --git a/Marlin/src/lcd/lcdprint.cpp b/Marlin/src/lcd/lcdprint.cpp
@@ -67,24 +67,24 @@ lcd_uint_t expand_u8str_P(char * const outstr, PGM_P const ptpl, const int8_t in
       }
       else {
         PGM_P const b = ind == -2 ? GET_TEXT(MSG_CHAMBER) : GET_TEXT(MSG_BED);
-        strncpy_P(o, b, n);
+        strlcpy_P(o, b, n + 1);
         n -= utf8_strlen(o);
         o += strlen(o);
       }
       if (n > 0) {
-        strncpy_P(o, (PGM_P)p, n);
+        strlcpy_P(o, (PGM_P)p, n + 1);
         n -= utf8_strlen(o);
         o += strlen(o);
         break;
       }
     }
     else if (wc == '$' && fstr) {
-      strncpy_P(o, FTOP(fstr), n);
-      n -= utf8_strlen_P(FTOP(fstr));
+      strlcpy_P(o, FTOP(fstr), n + 1);
+      n -= utf8_strlen(o);
       o += strlen(o);
     }
     else if (wc == '$' && cstr) {
-      strncpy(o, cstr, n);
+      strlcpy(o, cstr, n + 1);
       n -= utf8_strlen(o);
       o += strlen(o);
     }
diff --git a/Marlin/src/lcd/menu/menu_item.h b/Marlin/src/lcd/menu/menu_item.h
@@ -386,11 +386,11 @@ class MenuItem_bool : public MenuEditItemBase {
 
 #define PSTRING_ITEM_F_P(FLABEL, PVAL, STYL) do{ \
   constexpr int m = 20;                          \
-  char msg[m+1];                                 \
+  char msg[m + 1];                               \
   if (_menuLineNr == _thisItemNr) {              \
     msg[0] = ':'; msg[1] = ' ';                  \
-    strncpy_P(msg+2, PVAL, m-2);                 \
-    if (msg[m-1] & 0x80) msg[m-1] = '\0';        \
+    strlcpy_P(msg + 2, PVAL, m - 1);             \
+    if (msg[m - 1] & 0x80) msg[m - 1] = '\0';    \
   }                                              \
   STATIC_ITEM_F(FLABEL, STYL, msg);              \
 }while(0)
diff --git a/Marlin/src/module/settings.cpp b/Marlin/src/module/settings.cpp
@@ -2042,7 +2042,7 @@ void MarlinSettings::postprocess() {
           if (grid_max_x == (GRID_MAX_POINTS_X) && grid_max_y == (GRID_MAX_POINTS_Y)) {
             if (!validating) set_bed_leveling_enabled(false);
             bedlevel.set_grid(spacing, start);
-            EEPROM_READ(bedlevel.z_values);                 // 9 to 256 floats
+            EEPROM_READ(bedlevel.z_values);            // 9 to 256 floats
           }
           else if (grid_max_x > (GRID_MAX_POINTS_X) || grid_max_y > (GRID_MAX_POINTS_Y)) {
             eeprom_error = ERR_EEPROM_CORRUPT;
diff --git a/Marlin/src/sd/cardreader.cpp b/Marlin/src/sd/cardreader.cpp
@@ -446,8 +446,7 @@ void CardReader::ls(const uint8_t lsflags/*=0*/) {
       diveDir.close();
 
       if (longFilename[0]) {
-        strncpy_P(pathLong, longFilename, 63);
-        pathLong[63] = '\0';
+        strlcpy_P(pathLong, longFilename, 64);
         break;
       }
     }
@@ -1075,8 +1074,7 @@ const char* CardReader::diveToFile(const bool update_cwd, MediaFile* &inDirPtr,
     // Isolate the next subitem name
     const uint8_t len = name_end - atom_ptr;
     char dosSubdirname[len + 1];
-    strncpy(dosSubdirname, atom_ptr, len);
-    dosSubdirname[len] = 0;
+    strlcpy(dosSubdirname, atom_ptr, len + 1);
 
     if (echo) SERIAL_ECHOLN(dosSubdirname);
 
@@ -1181,7 +1179,7 @@ void CardReader::cdroot() {
       #endif
     #else
       // Copy filenames into the static array
-      #define _SET_SORTNAME(I) strncpy(sortnames[I], longest_filename(), SORTED_LONGNAME_MAXLEN)
+      #define _SET_SORTNAME(I) strlcpy(sortnames[I], longest_filename(), sizeof(sortnames[I]))
       #if SORTED_LONGNAME_MAXLEN == LONG_FILENAME_LENGTH
         // Short name sorting always use LONG_FILENAME_LENGTH with no trailing nul
         #define SET_SORTNAME(I) _SET_SORTNAME(I)

Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,7 @@ class GCodeQueue {`
`134`	`134`	`* Aborts the current SRAM queue so only use for one or two commands.`
`135`	`135`	`*/`
`136`	`136`	`static void inject(const char * const gcode) {`
`137`		`- strncpy(injected_commands, gcode, sizeof(injected_commands) - 1);`
	`137`	`+ strlcpy(injected_commands, gcode, sizeof(injected_commands));`
`138`	`138`	`}`
`139`	`139`
`140`	`140`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -158,8 +158,7 @@ void NextionTFT::sendFileList(int8_t startindex) {`
`158`	`158`	`}`
`159`	`159`
`160`	`160`	`void NextionTFT::selectFile() {`
`161`		`- strncpy(selectedfile, nextion_command + 4, command_len - 4);`
`162`		`- selectedfile[command_len - 5] = '\0';`
	`161`	`+ strlcpy(selectedfile, nextion_command + 4, command_len - 3);`
`163`	`162`	`#if NEXDEBUG(N_FILE)`
`164`	`163`	`DEBUG_ECHOLNPGM(" Selected File: ", selectedfile);`
`165`	`164`	`#endif`