From aca1241a7f7483b5459c9ca1712aa3ebcbed0378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Mon, 16 Jan 2023 10:55:40 +0100 Subject: [PATCH] Having a first stab at flawfinder errors --- src/FSCommon.cpp | 12 +++++------ src/gps/GPS.cpp | 2 +- src/gps/NMEAWPL.cpp | 8 ++++---- src/graphics/Screen.cpp | 24 +++++++++++----------- src/main.cpp | 6 +++--- src/mesh/Channels.cpp | 2 +- src/mesh/NodeDB.cpp | 6 +++--- src/mesh/http/WiFiAPClient.cpp | 2 +- src/modules/AdminModule.cpp | 8 ++++---- src/modules/CannedMessageModule.cpp | 8 ++++---- src/modules/ExternalNotificationModule.cpp | 4 ++-- 11 files changed, 41 insertions(+), 41 deletions(-) diff --git a/src/FSCommon.cpp b/src/FSCommon.cpp index 2541aa8b5..e072e54fb 100644 --- a/src/FSCommon.cpp +++ b/src/FSCommon.cpp @@ -80,7 +80,7 @@ void listDir(const char * dirname, uint8_t levels, boolean del = false) listDir(file.path(), levels -1, del); if(del) { LOG_DEBUG("Removing %s\n", file.path()); - strcpy(buffer, file.path()); + strncpy(buffer, file.path(), sizeof(buffer)); file.close(); FSCom.rmdir(buffer); } else { @@ -90,7 +90,7 @@ void listDir(const char * dirname, uint8_t levels, boolean del = false) listDir(file.name(), levels -1, del); if(del) { LOG_DEBUG("Removing %s\n", file.name()); - strcpy(buffer, file.name()); + strncpy(buffer, file.name(), sizeof(buffer)); file.close(); FSCom.rmdir(buffer); } else { @@ -105,7 +105,7 @@ void listDir(const char * dirname, uint8_t levels, boolean del = false) #ifdef ARCH_ESP32 if(del) { LOG_DEBUG("Deleting %s\n", file.path()); - strcpy(buffer, file.path()); + strncpy(buffer, file.path(), sizeof(buffer)); file.close(); FSCom.remove(buffer); } else { @@ -115,7 +115,7 @@ void listDir(const char * dirname, uint8_t levels, boolean del = false) #elif (defined(ARCH_RP2040) || defined(ARCH_PORTDUINO)) if(del) { LOG_DEBUG("Deleting %s\n", file.name()); - strcpy(buffer, file.name()); + strncpy(buffer, file.name(), sizeof(buffer)); file.close(); FSCom.remove(buffer); } else { @@ -132,7 +132,7 @@ void listDir(const char * dirname, uint8_t levels, boolean del = false) #ifdef ARCH_ESP32 if(del) { LOG_DEBUG("Removing %s\n", root.path()); - strcpy(buffer, root.path()); + strncpy(buffer, root.path(), sizeof(buffer)); root.close(); FSCom.rmdir(buffer); } else { @@ -141,7 +141,7 @@ void listDir(const char * dirname, uint8_t levels, boolean del = false) #elif (defined(ARCH_RP2040) || defined(ARCH_PORTDUINO)) if(del) { LOG_DEBUG("Removing %s\n", root.name()); - strcpy(buffer, root.name()); + strncpy(buffer, root.name(), sizeof(buffer)); root.close(); FSCom.rmdir(buffer); } else { diff --git a/src/gps/GPS.cpp b/src/gps/GPS.cpp index e5791223d..1814261a4 100644 --- a/src/gps/GPS.cpp +++ b/src/gps/GPS.cpp @@ -623,7 +623,7 @@ GnssModel_t GPS::probe() //tips: extensionNo field is 0 on some 6M GNSS modules for (int i = 0; i < info.extensionNo; ++i) { if (!strncmp(info.extension[i], "OD=", 3)) { - strcpy((char *)buffer, &(info.extension[i][3])); + strncpy((char *)buffer, &(info.extension[i][3]), sizeof(buffer)); LOG_DEBUG("GetModel:%s\n",(char *)buffer); } } diff --git a/src/gps/NMEAWPL.cpp b/src/gps/NMEAWPL.cpp index 70812e87b..842fc0760 100644 --- a/src/gps/NMEAWPL.cpp +++ b/src/gps/NMEAWPL.cpp @@ -19,7 +19,7 @@ uint32_t printWPL(char *buf, const Position &pos, const char *name) { GeoCoord geoCoord(pos.latitude_i,pos.longitude_i,pos.altitude); - uint32_t len = sprintf(buf, "$GNWPL,%02d%07.4f,%c,%03d%07.4f,%c,%s", + uint32_t len = snprintf(buf, sizeof(buf), "$GNWPL,%02d%07.4f,%c,%03d%07.4f,%c,%s", geoCoord.getDMSLatDeg(), (abs(geoCoord.getLatitude()) - geoCoord.getDMSLatDeg() * 1e+7) * 6e-6, geoCoord.getDMSLatCP(), @@ -31,7 +31,7 @@ uint32_t printWPL(char *buf, const Position &pos, const char *name) for (uint32_t i = 1; i < len; i++) { chk ^= buf[i]; } - len += sprintf(buf + len, "*%02X\r\n", chk); + len += snprintf(buf + len, sizeof(buf) + len, "*%02X\r\n", chk); return len; } @@ -62,7 +62,7 @@ uint32_t printWPL(char *buf, const Position &pos, const char *name) uint32_t printGGA(char *buf, const Position &pos) { GeoCoord geoCoord(pos.latitude_i,pos.longitude_i,pos.altitude); - uint32_t len = sprintf(buf, "$GNGGA,%06u.%03u,%02d%07.4f,%c,%03d%07.4f,%c,%u,%02u,%04u,%04d,%c,%04d,%c,%d,%04d", + uint32_t len = snprintf(buf, sizeof(buf), "$GNGGA,%06u.%03u,%02d%07.4f,%c,%03d%07.4f,%c,%u,%02u,%04u,%04d,%c,%04d,%c,%d,%04d", pos.time / 1000, pos.time % 1000, geoCoord.getDMSLatDeg(), @@ -85,6 +85,6 @@ uint32_t printGGA(char *buf, const Position &pos) for (uint32_t i = 1; i < len; i++) { chk ^= buf[i]; } - len += sprintf(buf + len, "*%02X\r\n", chk); + len += snprintf(buf + len, sizeof(buf) + len, "*%02X\r\n", chk); return len; } \ No newline at end of file diff --git a/src/graphics/Screen.cpp b/src/graphics/Screen.cpp index 53ca75a51..034ba3307 100644 --- a/src/graphics/Screen.cpp +++ b/src/graphics/Screen.cpp @@ -470,7 +470,7 @@ static void drawBattery(OLEDDisplay *display, int16_t x, int16_t y, uint8_t *img static void drawNodes(OLEDDisplay *display, int16_t x, int16_t y, NodeStatus *nodeStatus) { char usersString[20]; - sprintf(usersString, "%d/%d", nodeStatus->getNumOnline(), nodeStatus->getNumTotal()); + snprintf(usersString, sizeof(usersString), "%d/%d", nodeStatus->getNumOnline(), nodeStatus->getNumTotal()); #if defined(USE_EINK) || defined(ILI9341_DRIVER) || defined(ST7735_CS) display->drawFastImage(x, y + 3, 8, 8, imgUser); #else @@ -521,7 +521,7 @@ static void drawGPS(OLEDDisplay *display, int16_t x, int16_t y, const GPSStatus display->drawFastImage(x + 24, y, 8, 8, imgSatellite); // Draw the number of satellites - sprintf(satsString, "%u", gps->getNumSatellites()); + snprintf(satsString, sizeof(satsString), "%u", gps->getNumSatellites()); display->drawString(x + 34, y - 2, satsString); if(config.display.heading_bold) display->drawString(x + 35, y - 2, satsString); @@ -582,21 +582,21 @@ static void drawGPScoordinates(OLEDDisplay *display, int16_t x, int16_t y, const if (gpsFormat != Config_DisplayConfig_GpsCoordinateFormat_DMS) { char coordinateLine[22]; if (gpsFormat == Config_DisplayConfig_GpsCoordinateFormat_DEC) { // Decimal Degrees - sprintf(coordinateLine, "%f %f", geoCoord.getLatitude() * 1e-7, geoCoord.getLongitude() * 1e-7); + snprintf(coordinateLine, sizeof(coordinateLine), "%f %f", geoCoord.getLatitude() * 1e-7, geoCoord.getLongitude() * 1e-7); } else if (gpsFormat == Config_DisplayConfig_GpsCoordinateFormat_UTM) { // Universal Transverse Mercator - sprintf(coordinateLine, "%2i%1c %06u %07u", geoCoord.getUTMZone(), geoCoord.getUTMBand(), + snprintf(coordinateLine, sizeof(coordinateLine), "%2i%1c %06u %07u", geoCoord.getUTMZone(), geoCoord.getUTMBand(), geoCoord.getUTMEasting(), geoCoord.getUTMNorthing()); } else if (gpsFormat == Config_DisplayConfig_GpsCoordinateFormat_MGRS) { // Military Grid Reference System - sprintf(coordinateLine, "%2i%1c %1c%1c %05u %05u", geoCoord.getMGRSZone(), geoCoord.getMGRSBand(), + snprintf(coordinateLine, sizeof(coordinateLine), "%2i%1c %1c%1c %05u %05u", geoCoord.getMGRSZone(), geoCoord.getMGRSBand(), geoCoord.getMGRSEast100k(), geoCoord.getMGRSNorth100k(), geoCoord.getMGRSEasting(), geoCoord.getMGRSNorthing()); } else if (gpsFormat == Config_DisplayConfig_GpsCoordinateFormat_OLC) { // Open Location Code geoCoord.getOLCCode(coordinateLine); } else if (gpsFormat == Config_DisplayConfig_GpsCoordinateFormat_OSGR) { // Ordnance Survey Grid Reference if (geoCoord.getOSGRE100k() == 'I' || geoCoord.getOSGRN100k() == 'I') // OSGR is only valid around the UK region - sprintf(coordinateLine, "%s", "Out of Boundary"); + snprintf(coordinateLine, sizeof(coordinateLine), "%s", "Out of Boundary"); else - sprintf(coordinateLine, "%1c%1c %05u %05u", geoCoord.getOSGRE100k(), geoCoord.getOSGRN100k(), + snprintf(coordinateLine, sizeof(coordinateLine), "%1c%1c %05u %05u", geoCoord.getOSGRE100k(), geoCoord.getOSGRN100k(), geoCoord.getOSGREasting(), geoCoord.getOSGRNorthing()); } @@ -614,9 +614,9 @@ static void drawGPScoordinates(OLEDDisplay *display, int16_t x, int16_t y, const } else { char latLine[22]; char lonLine[22]; - sprintf(latLine, "%2i° %2i' %2u\" %1c", geoCoord.getDMSLatDeg(), geoCoord.getDMSLatMin(), geoCoord.getDMSLatSec(), + snprintf(latLine, sizeof(latLine), "%2i° %2i' %2u\" %1c", geoCoord.getDMSLatDeg(), geoCoord.getDMSLatMin(), geoCoord.getDMSLatSec(), geoCoord.getDMSLatCP()); - sprintf(lonLine, "%3i° %2i' %2u\" %1c", geoCoord.getDMSLonDeg(), geoCoord.getDMSLonMin(), geoCoord.getDMSLonSec(), + snprintf(lonLine, sizeof(lonLine), "%3i° %2i' %2u\" %1c", geoCoord.getDMSLonDeg(), geoCoord.getDMSLonMin(), geoCoord.getDMSLonSec(), geoCoord.getDMSLonCP()); display->drawString(x + (SCREEN_WIDTH - (display->getStringWidth(latLine))) / 2, y - FONT_HEIGHT_SMALL * 1, latLine); display->drawString(x + (SCREEN_WIDTH - (display->getStringWidth(lonLine))) / 2, y, lonLine); @@ -831,7 +831,7 @@ static void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_ } static char distStr[20]; - strcpy(distStr, "? km"); // might not have location data + strncpy(distStr, "? km", sizeof(distStr)); // might not have location data NodeInfo *ourNode = nodeDB.getNode(nodeDB.getNodeNum()); const char *fields[] = {username, distStr, signalStr, lastStr, NULL}; int16_t compassX = 0, compassY = 0; @@ -1010,7 +1010,7 @@ void Screen::setup() // Get our hardware ID uint8_t dmac[6]; getMacAddr(dmac); - sprintf(ourId, "%02x%02x", dmac[4], dmac[5]); + snprintf(ourId, sizeof(ourId), "%02x%02x", dmac[4], dmac[5]); // Turn on the display. handleSetOn(true); @@ -1738,7 +1738,7 @@ void DebugInfo::drawFrameSettings(OLEDDisplay *display, OLEDDisplayUiState *stat // Display Channel Utilization char chUtil[13]; - sprintf(chUtil, "ChUtil %2.0f%%", airTime->channelUtilizationPercent()); + snprintf(chUtil, sizeof(chUtil), "ChUtil %2.0f%%", airTime->channelUtilizationPercent()); display->drawString(x + SCREEN_WIDTH - display->getStringWidth(chUtil), y + FONT_HEIGHT_SMALL * 1, chUtil); if (config.position.gps_enabled) { // Line 3 diff --git a/src/main.cpp b/src/main.cpp index 2631e68df..76a6a6b6f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -110,12 +110,12 @@ const char *getDeviceName() // Meshtastic_ab3c or Shortname_abcd static char name[20]; - sprintf(name, "%02x%02x", dmac[4], dmac[5]); + snprintf(name, sizeof(name), "%02x%02x", dmac[4], dmac[5]); // if the shortname exists and is NOT the new default of ab3c, use it for BLE name. if ((owner.short_name != NULL) && (strcmp(owner.short_name, name) != 0)) { - sprintf(name, "%s_%02x%02x", owner.short_name, dmac[4], dmac[5]); + snprintf(name, sizeof(name), "%s_%02x%02x", owner.short_name, dmac[4], dmac[5]); } else { - sprintf(name, "Meshtastic_%02x%02x", dmac[4], dmac[5]); + snprintf(name, sizeof(name), "Meshtastic_%02x%02x", dmac[4], dmac[5]); } return name; } diff --git a/src/mesh/Channels.cpp b/src/mesh/Channels.cpp index 337fa66e0..f9b3eb9b8 100644 --- a/src/mesh/Channels.cpp +++ b/src/mesh/Channels.cpp @@ -84,7 +84,7 @@ void Channels::initDefaultChannel(ChannelIndex chIndex) uint8_t defaultpskIndex = 1; channelSettings.psk.bytes[0] = defaultpskIndex; channelSettings.psk.size = 1; - strcpy(channelSettings.name, ""); + strncpy(channelSettings.name, "", sizeof(channelSettings.name)); ch.has_settings = true; ch.role = Channel_Role_PRIMARY; diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index c19beadca..076f56af6 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -265,10 +265,10 @@ void NodeDB::installDefaultDeviceState() // Set default owner name pickNewNodeNum(); // based on macaddr now - sprintf(owner.long_name, "Meshtastic %02x%02x", ourMacAddr[4], ourMacAddr[5]); - sprintf(owner.short_name, "%02x%02x", ourMacAddr[4], ourMacAddr[5]); + snprintf(owner.long_name, sizeof(owner.long_name), "Meshtastic %02x%02x", ourMacAddr[4], ourMacAddr[5]); + snprintf(owner.short_name, sizeof(owner.short_name), "%02x%02x", ourMacAddr[4], ourMacAddr[5]); - sprintf(owner.id, "!%08x", getNodeNum()); // Default node ID now based on nodenum + snprintf(owner.id, sizeof(owner.id), "!%08x", getNodeNum()); // Default node ID now based on nodenum memcpy(owner.macaddr, ourMacAddr, sizeof(owner.macaddr)); } diff --git a/src/mesh/http/WiFiAPClient.cpp b/src/mesh/http/WiFiAPClient.cpp index 733e4c1f1..1139c305e 100644 --- a/src/mesh/http/WiFiAPClient.cpp +++ b/src/mesh/http/WiFiAPClient.cpp @@ -164,7 +164,7 @@ bool initWifi() if (*wifiName) { uint8_t dmac[6]; getMacAddr(dmac); - sprintf(ourHost, "Meshtastic-%02x%02x", dmac[4], dmac[5]); + snprintf(ourHost, sizeof(ourHost), "Meshtastic-%02x%02x", dmac[4], dmac[5]); WiFi.mode(WIFI_MODE_STA); WiFi.setHostname(ourHost); diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index b6ea8826a..f790eba71 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -27,7 +27,7 @@ static const char *secretReserved = "sekrit"; static void writeSecret(char *buf, const char *currentVal) { if (strcmp(buf, secretReserved) == 0) { - strcpy(buf, currentVal); + strncpy(buf, currentVal, sizeof(buf)); } } @@ -199,15 +199,15 @@ void AdminModule::handleSetOwner(const User &o) if (*o.long_name) { changed |= strcmp(owner.long_name, o.long_name); - strcpy(owner.long_name, o.long_name); + strncpy(owner.long_name, o.long_name, sizeof(owner.long_name)); } if (*o.short_name) { changed |= strcmp(owner.short_name, o.short_name); - strcpy(owner.short_name, o.short_name); + strncpy(owner.short_name, o.short_name, sizeof(owner.short_name)); } if (*o.id) { changed |= strcmp(owner.id, o.id); - strcpy(owner.id, o.id); + strncpy(owner.id, o.id, sizeof(owner.id)); } if (owner.is_licensed != o.is_licensed) { changed = 1; diff --git a/src/modules/CannedMessageModule.cpp b/src/modules/CannedMessageModule.cpp index f1cbc576d..9e42f8467 100644 --- a/src/modules/CannedMessageModule.cpp +++ b/src/modules/CannedMessageModule.cpp @@ -75,7 +75,7 @@ int CannedMessageModule::splitConfiguredMessages() int i = 0; // collect all the message parts - strcpy(this->messageStore, cannedMessageModuleConfig.messages); + strncpy(this->messageStore, cannedMessageModuleConfig.messages, sizeof(this->messageStore)); // The first message points to the beginning of the store. this->messages[messageIndex++] = this->messageStore; @@ -454,7 +454,7 @@ void CannedMessageModule::drawFrame(OLEDDisplay *display, OLEDDisplayUiState *st } display->drawStringf(0 + x, 0 + y, buffer, "To: %s", cannedMessageModule->getNodeName(this->dest)); // used chars right aligned - sprintf(buffer, "%d left", Constants_DATA_PAYLOAD_LEN - this->freetext.length()); + snprintf(buffer, sizeof(buffer), "%d left", Constants_DATA_PAYLOAD_LEN - this->freetext.length()); display->drawString(x + display->getWidth() - display->getStringWidth(buffer), y + 0, buffer); if (this->destSelect) { display->drawString(x + display->getWidth() - display->getStringWidth(buffer) - 1, y + 0, buffer); @@ -551,7 +551,7 @@ void CannedMessageModule::handleGetCannedMessageModuleMessages(const MeshPacket LOG_DEBUG("*** handleGetCannedMessageModuleMessages\n"); if(req.decoded.want_response) { response->which_payload_variant = AdminMessage_get_canned_message_module_messages_response_tag; - strcpy(response->get_canned_message_module_messages_response, cannedMessageModuleConfig.messages); + strncpy(response->get_canned_message_module_messages_response, cannedMessageModuleConfig.messages, sizeof(response->get_canned_message_module_messages_response)); } // Don't send anything if not instructed to. Better than asserting. } @@ -562,7 +562,7 @@ void CannedMessageModule::handleSetCannedMessageModuleMessages(const char *from_ if (*from_msg) { changed |= strcmp(cannedMessageModuleConfig.messages, from_msg); - strcpy(cannedMessageModuleConfig.messages, from_msg); + strncpy(cannedMessageModuleConfig.messages, from_msg, sizeof(cannedMessageModuleConfig.messages)); LOG_DEBUG("*** from_msg.text:%s\n", from_msg); } diff --git a/src/modules/ExternalNotificationModule.cpp b/src/modules/ExternalNotificationModule.cpp index a8f112c94..3f2137ec8 100644 --- a/src/modules/ExternalNotificationModule.cpp +++ b/src/modules/ExternalNotificationModule.cpp @@ -349,7 +349,7 @@ void ExternalNotificationModule::handleGetRingtone(const MeshPacket &req, AdminM LOG_INFO("*** handleGetRingtone\n"); if(req.decoded.want_response) { response->which_payload_variant = AdminMessage_get_ringtone_response_tag; - strcpy(response->get_ringtone_response, rtttlConfig.ringtone); + strncpy(response->get_ringtone_response, rtttlConfig.ringtone, sizeof(response->get_ringtone_response)); } // Don't send anything if not instructed to. Better than asserting. } @@ -360,7 +360,7 @@ void ExternalNotificationModule::handleSetRingtone(const char *from_msg) if (*from_msg) { changed |= strcmp(rtttlConfig.ringtone, from_msg); - strcpy(rtttlConfig.ringtone, from_msg); + strncpy(rtttlConfig.ringtone, from_msg, sizeof(rtttlConfig.ringtone)); LOG_INFO("*** from_msg.text:%s\n", from_msg); }