From 05c478d1f25b67582856240ef68003a59cf15fb3 Mon Sep 17 00:00:00 2001 From: Thomas Basler Date: Thu, 14 Jul 2022 18:54:53 +0200 Subject: [PATCH] Added several guards and error messages Try to prevent heap corruptions --- lib/Hoymiles/src/HoymilesRadio.cpp | 5 +++++ lib/Hoymiles/src/inverters/InverterAbstract.cpp | 12 +++++++++++- lib/Hoymiles/src/parser/AlarmLogParser.cpp | 4 ++++ lib/Hoymiles/src/parser/StatisticsParser.cpp | 4 ++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/Hoymiles/src/HoymilesRadio.cpp b/lib/Hoymiles/src/HoymilesRadio.cpp index 32de8883..533d321e 100644 --- a/lib/Hoymiles/src/HoymilesRadio.cpp +++ b/lib/Hoymiles/src/HoymilesRadio.cpp @@ -225,6 +225,11 @@ void HoymilesRadio::sendEsbPacket(serial_u target, uint8_t mainCmd, uint8_t subC { static uint8_t txBuffer[MAX_RF_PAYLOAD_SIZE]; + if (10 + currentTransaction.len + 1 > MAX_RF_PAYLOAD_SIZE) { + Serial.printf("FATAL: (%s, %d) payload too large\n", __FILE__, __LINE__); + return; + } + if (!resend) { currentTransaction.sendCount = 0; currentTransaction.target = target; diff --git a/lib/Hoymiles/src/inverters/InverterAbstract.cpp b/lib/Hoymiles/src/inverters/InverterAbstract.cpp index e9e24794..ffc1355a 100644 --- a/lib/Hoymiles/src/inverters/InverterAbstract.cpp +++ b/lib/Hoymiles/src/inverters/InverterAbstract.cpp @@ -41,7 +41,7 @@ StatisticsParser* InverterAbstract::Statistics() void InverterAbstract::clearRxFragmentBuffer() { - memset(_rxFragmentBuffer, 0, MAX_RF_FRAGMENT_COUNT * MAX_RF_PAYLOAD_SIZE); + memset(_rxFragmentBuffer, 0, MAX_RF_FRAGMENT_COUNT * sizeof(fragment_t)); _rxFragmentMaxPacketId = 0; _rxFragmentLastPacketId = 0; _rxFragmentRetransmitCnt = 0; @@ -55,6 +55,16 @@ void InverterAbstract::clearRxFragmentBuffer() void InverterAbstract::addRxFragment(uint8_t fragment[], uint8_t len) { + if (len < 11 + 1) { + Serial.printf("FATAL: (%s, %d) fragment too short\n", __FILE__, __LINE__); + return; + } + + if (len - 11 > MAX_RF_PAYLOAD_SIZE) { + Serial.printf("FATAL: (%s, %d) fragment too large\n", __FILE__, __LINE__); + return; + } + uint8_t fragmentCount = fragment[9]; if ((fragmentCount & 0b01111111) < MAX_RF_FRAGMENT_COUNT) { // Packets with 0x81 will be seen as 1 diff --git a/lib/Hoymiles/src/parser/AlarmLogParser.cpp b/lib/Hoymiles/src/parser/AlarmLogParser.cpp index 79f40031..9d321e83 100644 --- a/lib/Hoymiles/src/parser/AlarmLogParser.cpp +++ b/lib/Hoymiles/src/parser/AlarmLogParser.cpp @@ -9,6 +9,10 @@ void AlarmLogParser::clearBuffer() void AlarmLogParser::appendFragment(uint8_t offset, uint8_t* payload, uint8_t len) { + if (offset + len > (ALARM_LOG_ENTRY_COUNT * ALARM_LOG_ENTRY_SIZE)) { + Serial.printf("FATAL: (%s, %d) stats packet too large for buffer\n", __FILE__, __LINE__); + return; + } memcpy(&_payloadAlarmLog[offset], payload, len); _alarmLogLength += len; } diff --git a/lib/Hoymiles/src/parser/StatisticsParser.cpp b/lib/Hoymiles/src/parser/StatisticsParser.cpp index 1aaaa6be..2310a42b 100644 --- a/lib/Hoymiles/src/parser/StatisticsParser.cpp +++ b/lib/Hoymiles/src/parser/StatisticsParser.cpp @@ -14,6 +14,10 @@ void StatisticsParser::clearBuffer() void StatisticsParser::appendFragment(uint8_t offset, uint8_t* payload, uint8_t len) { + if (offset + len > STATISTIC_PACKET_SIZE) { + Serial.printf("FATAL: (%s, %d) stats packet too large for buffer\n", __FILE__, __LINE__); + return; + } memcpy(&_payloadStatistic[offset], payload, len); _statisticLength += len; }