diff --git a/esphome/components/modbus/modbus_helpers.cpp b/esphome/components/modbus/modbus_helpers.cpp index 77190b28463..89dc3c08bc1 100644 --- a/esphome/components/modbus/modbus_helpers.cpp +++ b/esphome/components/modbus/modbus_helpers.cpp @@ -5,6 +5,29 @@ namespace esphome::modbus::helpers { static const char *const TAG = "modbus_helpers"; +static size_t required_payload_size(SensorValueType sensor_value_type) { + switch (sensor_value_type) { + case SensorValueType::U_WORD: + case SensorValueType::S_WORD: + return 2; + case SensorValueType::U_DWORD: + case SensorValueType::FP32: + case SensorValueType::U_DWORD_R: + case SensorValueType::FP32_R: + case SensorValueType::S_DWORD: + case SensorValueType::S_DWORD_R: + return 4; + case SensorValueType::U_QWORD: + case SensorValueType::S_QWORD: + case SensorValueType::U_QWORD_R: + case SensorValueType::S_QWORD_R: + return 8; + case SensorValueType::RAW: + default: + return 0; + } +} + void number_to_payload(std::vector &data, int64_t value, SensorValueType value_type) { switch (value_type) { case SensorValueType::U_WORD: @@ -47,93 +70,70 @@ int64_t payload_to_number(const std::vector &data, SensorValueType sens uint32_t bitmask) { int64_t value = 0; // int64_t because it can hold signed and unsigned 32 bits - if (offset > data.size()) { - ESP_LOGE(TAG, "not enough data for value"); + // Validate offset against the buffer for all types, including RAW/unsupported, so + // a malformed or misconfigured frame still produces an error log. + if (static_cast(offset) > data.size()) { + ESP_LOGE(TAG, "not enough data for value type=%u offset=%u size=%zu", static_cast(sensor_value_type), + static_cast(offset), data.size()); + return value; + } + + const size_t required_size = required_payload_size(sensor_value_type); + if (required_size == 0) { + return value; + } + + if (data.size() - offset < required_size) { + ESP_LOGE(TAG, "not enough data for value type=%u offset=%u size=%zu required=%zu", + static_cast(sensor_value_type), static_cast(offset), data.size(), + required_size); return value; } - size_t size = data.size() - offset; - bool error = false; switch (sensor_value_type) { case SensorValueType::U_WORD: - if (size >= 2) { - value = mask_and_shift_by_rightbit(get_data(data, offset), - bitmask); // default is 0xFFFF ; - } else { - error = true; - } + value = mask_and_shift_by_rightbit(get_data(data, offset), bitmask); // default is 0xFFFF ; break; case SensorValueType::U_DWORD: case SensorValueType::FP32: - if (size >= 4) { - value = get_data(data, offset); - value = mask_and_shift_by_rightbit((uint32_t) value, bitmask); - } else { - error = true; - } + value = get_data(data, offset); + value = mask_and_shift_by_rightbit((uint32_t) value, bitmask); break; case SensorValueType::U_DWORD_R: case SensorValueType::FP32_R: - if (size >= 4) { - value = get_data(data, offset); - value = static_cast(value & 0xFFFF) << 16 | (value & 0xFFFF0000) >> 16; - value = mask_and_shift_by_rightbit((uint32_t) value, bitmask); - } else { - error = true; - } + value = get_data(data, offset); + value = static_cast(value & 0xFFFF) << 16 | (value & 0xFFFF0000) >> 16; + value = mask_and_shift_by_rightbit((uint32_t) value, bitmask); break; case SensorValueType::S_WORD: - if (size >= 2) { - value = mask_and_shift_by_rightbit(get_data(data, offset), - bitmask); // default is 0xFFFF ; - } else { - error = true; - } + value = mask_and_shift_by_rightbit(get_data(data, offset), bitmask); // default is 0xFFFF ; break; case SensorValueType::S_DWORD: - if (size >= 4) { - value = mask_and_shift_by_rightbit(get_data(data, offset), bitmask); - } else { - error = true; - } + value = mask_and_shift_by_rightbit(get_data(data, offset), bitmask); break; case SensorValueType::S_DWORD_R: { - if (size >= 4) { - value = get_data(data, offset); - // Currently the high word is at the low position - // the sign bit is therefore at low before the switch - uint32_t sign_bit = (value & 0x8000) << 16; - value = mask_and_shift_by_rightbit( - static_cast(((value & 0x7FFF) << 16 | (value & 0xFFFF0000) >> 16) | sign_bit), bitmask); - } else { - error = true; - } + value = get_data(data, offset); + // Currently the high word is at the low position + // the sign bit is therefore at low before the switch + uint32_t sign_bit = (value & 0x8000) << 16; + value = mask_and_shift_by_rightbit( + static_cast(((value & 0x7FFF) << 16 | (value & 0xFFFF0000) >> 16) | sign_bit), bitmask); } break; case SensorValueType::U_QWORD: case SensorValueType::S_QWORD: // Ignore bitmask for QWORD - if (size >= 8) { - value = get_data(data, offset); - } else { - error = true; - } + value = get_data(data, offset); break; case SensorValueType::U_QWORD_R: case SensorValueType::S_QWORD_R: { // Ignore bitmask for QWORD - if (size >= 8) { - uint64_t tmp = get_data(data, offset); - value = (tmp << 48) | (tmp >> 48) | ((tmp & 0xFFFF0000) << 16) | ((tmp >> 16) & 0xFFFF0000); - } else { - error = true; - } + uint64_t tmp = get_data(data, offset); + value = (tmp << 48) | (tmp >> 48) | ((tmp & 0xFFFF0000) << 16) | ((tmp >> 16) & 0xFFFF0000); } break; case SensorValueType::RAW: default: break; } - if (error) - ESP_LOGE(TAG, "not enough data for value"); return value; } } // namespace esphome::modbus::helpers diff --git a/tests/components/modbus/modbus_helpers_test.cpp b/tests/components/modbus/modbus_helpers_test.cpp new file mode 100644 index 00000000000..e1b4fb2aa6e --- /dev/null +++ b/tests/components/modbus/modbus_helpers_test.cpp @@ -0,0 +1,22 @@ +#include + +#include "esphome/components/modbus/modbus_helpers.h" + +namespace esphome::modbus::helpers { + +TEST(ModbusHelpersTest, PayloadToNumberRejectsOffsetAtEndOfBuffer) { + const std::vector data{0x12, 0x34}; + EXPECT_EQ(payload_to_number(data, SensorValueType::U_WORD, 2, 0xFFFFFFFF), 0); +} + +TEST(ModbusHelpersTest, PayloadToNumberRejectsTruncatedMultiRegisterValue) { + const std::vector data{0x12, 0x34, 0x56}; + EXPECT_EQ(payload_to_number(data, SensorValueType::U_DWORD, 0, 0xFFFFFFFF), 0); +} + +TEST(ModbusHelpersTest, PayloadToNumberDecodesValidWord) { + const std::vector data{0x12, 0x34}; + EXPECT_EQ(payload_to_number(data, SensorValueType::U_WORD, 0, 0xFFFFFFFF), 0x1234); +} + +} // namespace esphome::modbus::helpers