From b87afc3bcb2190c0c2b1b19e79229197e09bc431 Mon Sep 17 00:00:00 2001 From: Laurent Dong Date: Fri, 8 Mar 2019 13:24:02 -0500 Subject: [PATCH] Code review: Copy string with strlcpy() instead of snprintf() Copying string with snprintf() is unsafy and slow because it check and replace plcaehold (%?) --- sonoff/sonoff.ino | 2 +- sonoff/support.ino | 2 +- sonoff/xdrv_02_mqtt.ino | 4 ++-- sonoff/xdrv_08_serial_bridge.ino | 2 +- sonoff/xdrv_09_timers.ino | 2 +- sonoff/xdrv_10_rules.ino | 4 ++-- sonoff/xdsp_03_matrix.ino | 2 +- sonoff/xsns_09_bmp.ino | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sonoff/sonoff.ino b/sonoff/sonoff.ino index c043dc634..52308641a 100755 --- a/sonoff/sonoff.ino +++ b/sonoff/sonoff.ino @@ -229,7 +229,7 @@ char* GetOtaUrl(char *otaurl, size_t otaurl_size) snprintf_P(otaurl, otaurl_size, Settings.ota_url, ESP.getChipId()); } else { - snprintf(otaurl, otaurl_size, Settings.ota_url); + strlcpy(otaurl, Settings.ota_url, otaurl_size); } return otaurl; } diff --git a/sonoff/support.ino b/sonoff/support.ino index 8f1e4c104..fc2a3d853 100644 --- a/sonoff/support.ino +++ b/sonoff/support.ino @@ -672,7 +672,7 @@ void SerialSendRaw(char *codes) int size = strlen(codes); while (size > 0) { - snprintf(stemp, sizeof(stemp), codes); + strlcpy(stemp, codes, sizeof(stemp)); code = strtol(stemp, &p, 16); Serial.write(code); size -= 2; diff --git a/sonoff/xdrv_02_mqtt.ino b/sonoff/xdrv_02_mqtt.ino index 4df9ca6cb..450ff252b 100644 --- a/sonoff/xdrv_02_mqtt.ino +++ b/sonoff/xdrv_02_mqtt.ino @@ -765,10 +765,10 @@ bool MqttCommand(void) if (data_len > 0) { char *mqtt_part = strtok(dataBuf, " "); if (mqtt_part) { - snprintf(stemp1, sizeof(stemp1), mqtt_part); + strlcpy(stemp1, mqtt_part, sizeof(stemp1)); mqtt_part = strtok(NULL, " "); if (mqtt_part) { - snprintf(mqtt_data, sizeof(mqtt_data), mqtt_part); + strlcpy(mqtt_data, mqtt_part, sizeof(mqtt_data)); } else { mqtt_data[0] = '\0'; } diff --git a/sonoff/xdrv_08_serial_bridge.ino b/sonoff/xdrv_08_serial_bridge.ino index ba83f11ec..0db7da58c 100644 --- a/sonoff/xdrv_08_serial_bridge.ino +++ b/sonoff/xdrv_08_serial_bridge.ino @@ -137,7 +137,7 @@ bool SerialBridgeCommand(void) int size = strlen(XdrvMailbox.data); while (size > 0) { - snprintf(stemp, sizeof(stemp), codes); + strlcpy(stemp, codes, sizeof(stemp)); code = strtol(stemp, &p, 16); SerialBridgeSerial->write(code); // "AA004566" as hex values size -= 2; diff --git a/sonoff/xdrv_09_timers.ino b/sonoff/xdrv_09_timers.ino index 094fbcd96..e80f5bc01 100644 --- a/sonoff/xdrv_09_timers.ino +++ b/sonoff/xdrv_09_timers.ino @@ -385,7 +385,7 @@ bool TimerCommand(void) uint8_t sign = 0; char time_str[10]; - snprintf(time_str, sizeof(time_str), root[parm_uc]); + strlcpy(time_str, root[parm_uc], sizeof(time_str)); const char *substr = strtok(time_str, ":"); if (substr != NULL) { if (strchr(substr, '-')) { diff --git a/sonoff/xdrv_10_rules.ino b/sonoff/xdrv_10_rules.ino index 7897a0c17..4422d92b3 100644 --- a/sonoff/xdrv_10_rules.ino +++ b/sonoff/xdrv_10_rules.ino @@ -218,7 +218,7 @@ bool RulesRuleMatch(uint8_t rule_set, String &event, String &rule) } #endif // USE_TIMERS and USE_SUNRISE rule_param.toUpperCase(); - snprintf(rule_svalue, sizeof(rule_svalue), rule_param.c_str()); + strlcpy(rule_svalue, rule_param.c_str(), sizeof(rule_svalue)); int temp_value = GetStateNumber(rule_svalue); if (temp_value > -1) { @@ -365,7 +365,7 @@ bool RuleSetProcess(uint8_t rule_set, String &event_saved) #endif // USE_TIMERS and USE_SUNRISE char command[commands.length() +1]; - snprintf(command, sizeof(command), commands.c_str()); + strlcpy(command, commands.c_str(), sizeof(command)); AddLog_P2(LOG_LEVEL_INFO, PSTR("RUL: %s performs \"%s\""), event_trigger.c_str(), command); diff --git a/sonoff/xdsp_03_matrix.ino b/sonoff/xdsp_03_matrix.ino index b63a6675e..2535c92d9 100644 --- a/sonoff/xdsp_03_matrix.ino +++ b/sonoff/xdsp_03_matrix.ino @@ -226,7 +226,7 @@ void MatrixOnOff(void) void MatrixDrawStringAt(uint16_t x, uint16_t y, char *str, uint16_t color, uint8_t flag) { - snprintf(mtx_buffer, MTX_MAX_SCREEN_BUFFER, str); + strlcpy(mtx_buffer, str, MTX_MAX_SCREEN_BUFFER); mtx_mode = x &1; // Use x for selecting scroll up (0) or scroll left (1) mtx_loop = y &1; // Use y for selecting no loop (0) or loop (1) if (!mtx_state) { mtx_state = 1; } diff --git a/sonoff/xsns_09_bmp.ino b/sonoff/xsns_09_bmp.ino index 9696a9c07..488efd7ee 100755 --- a/sonoff/xsns_09_bmp.ino +++ b/sonoff/xsns_09_bmp.ino @@ -539,7 +539,7 @@ void BmpShow(bool json) float bmp_pressure = ConvertPressure(bmp_sensors[bmp_idx].bmp_pressure); char name[10]; - snprintf(name, sizeof(name), bmp_sensors[bmp_idx].bmp_name); + strlcpy(name, bmp_sensors[bmp_idx].bmp_name, sizeof(name)); if (bmp_count > 1) { snprintf_P(name, sizeof(name), PSTR("%s-%02X"), name, bmp_sensors[bmp_idx].bmp_address); // BMXXXX-XX }