Skip to content

Commit

Permalink
⚡️ Use strlcpy with buffer size (MarlinFirmware#26513)
Browse files Browse the repository at this point in the history
  • Loading branch information
@thinkyhead @classicrocker883
thinkyhead authored and classicrocker883 committed Dec 26, 2023
1 parent 0604079 commit 40f8f35
Show file tree
Hide file tree
Showing 14 changed files with 51 additions and 61 deletions.
3 changes: 3 additions & 0 deletions Marlin/src/HAL/LINUX/include/Arduino.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
35 changes: 15 additions & 20 deletions Marlin/src/core/mstring.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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; }
Expand All @@ -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))
Expand All @@ -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; }
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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'); }
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/gcode/queue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/**
Expand Down
13 changes: 6 additions & 7 deletions Marlin/src/lcd/extui/anycubic_chiron/chiron_tft.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 );
Expand Down
9 changes: 3 additions & 6 deletions Marlin/src/lcd/extui/anycubic_vyper/dgus_tft.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions Marlin/src/lcd/extui/mks_ui/draw_print_file.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
3 changes: 1 addition & 2 deletions Marlin/src/lcd/extui/nextion/nextion_tft.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions Marlin/src/lcd/lcdprint.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions Marlin/src/lcd/menu/menu_item.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/module/settings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 3 additions & 5 deletions Marlin/src/sd/cardreader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 40f8f35

Please sign in to comment.