diff --git a/esphome/components/b_parasite/b_parasite.cpp b/esphome/components/b_parasite/b_parasite.cpp index 356f3964766..7be26efa7f2 100644 --- a/esphome/components/b_parasite/b_parasite.cpp +++ b/esphome/components/b_parasite/b_parasite.cpp @@ -38,6 +38,11 @@ bool BParasite::parse_device(const esp32_ble_tracker::ESPBTDevice &device) { const auto &data = service_data.data; + if (data.size() < 10) { + ESP_LOGW(TAG, "Service data too short: %zu", data.size()); + return false; + } + const uint8_t protocol_version = data[0] >> 4; if (protocol_version != 1 && protocol_version != 2) { ESP_LOGE(TAG, "Unsupported protocol version: %u", protocol_version); @@ -47,6 +52,11 @@ bool BParasite::parse_device(const esp32_ble_tracker::ESPBTDevice &device) { // Some b-parasite versions have an (optional) illuminance sensor. bool has_illuminance = data[0] & 0x1; + if (has_illuminance && data.size() < 18) { + ESP_LOGW(TAG, "Service data too short for illuminance: %zu", data.size()); + return false; + } + // Counter for deduplicating messages. uint8_t counter = data[1] & 0x0f; if (last_processed_counter_ == counter) { diff --git a/esphome/components/kamstrup_kmp/kamstrup_kmp.cpp b/esphome/components/kamstrup_kmp/kamstrup_kmp.cpp index 29de6512559..9f2557243c8 100644 --- a/esphome/components/kamstrup_kmp/kamstrup_kmp.cpp +++ b/esphome/components/kamstrup_kmp/kamstrup_kmp.cpp @@ -110,9 +110,17 @@ void KamstrupKMPComponent::send_message_(const uint8_t *msg, int msg_len) { for (int i = 0; i < buffer_len; i++) { if (buffer[i] == 0x06 || buffer[i] == 0x0d || buffer[i] == 0x1b || buffer[i] == 0x40 || buffer[i] == 0x80) { + if (tx_msg_len + 2 >= static_cast(sizeof(tx_msg))) { + ESP_LOGE(TAG, "TX message overflow"); + return; + } tx_msg[tx_msg_len++] = 0x1b; tx_msg[tx_msg_len++] = buffer[i] ^ 0xff; } else { + if (tx_msg_len + 1 >= static_cast(sizeof(tx_msg))) { + ESP_LOGE(TAG, "TX message overflow"); + return; + } tx_msg[tx_msg_len++] = buffer[i]; } } @@ -216,8 +224,8 @@ void KamstrupKMPComponent::parse_command_message_(uint16_t command, const uint8_ uint8_t unit_idx = msg[4]; uint8_t mantissa_range = msg[5]; - if (mantissa_range > 4) { - ESP_LOGE(TAG, "Received invalid message (mantissa size too large %d, expected 4)", mantissa_range); + if (mantissa_range > 4 || msg_len < 7 + mantissa_range) { + ESP_LOGE(TAG, "Received invalid message (mantissa size %d, msg_len %d)", mantissa_range, msg_len); return; } diff --git a/esphome/components/ld2412/ld2412.cpp b/esphome/components/ld2412/ld2412.cpp index ef0915d0bc0..37578dd8daf 100644 --- a/esphome/components/ld2412/ld2412.cpp +++ b/esphome/components/ld2412/ld2412.cpp @@ -413,24 +413,29 @@ void LD2412Component::handle_periodic_data_() { this->detection_distance_sensor_->publish_state_if_not_dup(new_detect_distance); } if (engineering_mode) { - /* - Moving distance range: 18th byte - Still distance range: 19th byte - Moving energy: 20~28th bytes - */ - for (uint8_t i = 0; i < TOTAL_GATES; i++) { - SAFE_PUBLISH_SENSOR(this->gate_move_sensors_[i], this->buffer_data_[MOVING_SENSOR_START + i]) + // Engineering mode needs at least LIGHT_SENSOR + 1 bytes + if (this->buffer_pos_ < LIGHT_SENSOR + 1) { + ESP_LOGW(TAG, "Engineering mode packet too short: %u", this->buffer_pos_); + } else { + /* + Moving distance range: 18th byte + Still distance range: 19th byte + Moving energy: 20~28th bytes + */ + for (uint8_t i = 0; i < TOTAL_GATES; i++) { + SAFE_PUBLISH_SENSOR(this->gate_move_sensors_[i], this->buffer_data_[MOVING_SENSOR_START + i]) + } + /* + Still energy: 29~37th bytes + */ + for (uint8_t i = 0; i < TOTAL_GATES; i++) { + SAFE_PUBLISH_SENSOR(this->gate_still_sensors_[i], this->buffer_data_[STILL_SENSOR_START + i]) + } + /* + Light sensor value + */ + SAFE_PUBLISH_SENSOR(this->light_sensor_, this->buffer_data_[LIGHT_SENSOR]) } - /* - Still energy: 29~37th bytes - */ - for (uint8_t i = 0; i < TOTAL_GATES; i++) { - SAFE_PUBLISH_SENSOR(this->gate_still_sensors_[i], this->buffer_data_[STILL_SENSOR_START + i]) - } - /* - Light sensor: 38th bytes - */ - SAFE_PUBLISH_SENSOR(this->light_sensor_, this->buffer_data_[LIGHT_SENSOR]) } else { for (auto &gate_move_sensor : this->gate_move_sensors_) { SAFE_PUBLISH_SENSOR_UNKNOWN(gate_move_sensor) diff --git a/esphome/components/nextion/nextion.cpp b/esphome/components/nextion/nextion.cpp index 7ae4d50fc80..01ceb3d765e 100644 --- a/esphome/components/nextion/nextion.cpp +++ b/esphome/components/nextion/nextion.cpp @@ -646,8 +646,8 @@ void Nextion::process_nextion_commands_() { break; } - if (to_process_length == 0) { - ESP_LOGE(TAG, "Numeric return but no data"); + if (to_process_length < 4) { + ESP_LOGE(TAG, "Numeric return but insufficient data (need 4, got %zu)", to_process_length); break; } diff --git a/esphome/components/pipsolar/pipsolar.cpp b/esphome/components/pipsolar/pipsolar.cpp index eb6d3931e05..c304d206c00 100644 --- a/esphome/components/pipsolar/pipsolar.cpp +++ b/esphome/components/pipsolar/pipsolar.cpp @@ -192,8 +192,13 @@ bool Pipsolar::send_next_command_() { if (!this->command_queue_[this->command_queue_position_].empty()) { const char *command = this->command_queue_[this->command_queue_position_].c_str(); uint8_t byte_command[16]; - uint8_t length = this->command_queue_[this->command_queue_position_].length(); - for (uint8_t i = 0; i < length; i++) { + size_t length = this->command_queue_[this->command_queue_position_].length(); + if (length > sizeof(byte_command)) { + ESP_LOGE(TAG, "Command too long: %zu", length); + this->command_queue_[this->command_queue_position_].clear(); + return false; + } + for (size_t i = 0; i < length; i++) { byte_command[i] = (uint8_t) this->command_queue_[this->command_queue_position_].at(i); } this->state_ = STATE_COMMAND; diff --git a/esphome/components/smt100/smt100.cpp b/esphome/components/smt100/smt100.cpp index 105cc06edbf..6eb6416447f 100644 --- a/esphome/components/smt100/smt100.cpp +++ b/esphome/components/smt100/smt100.cpp @@ -14,11 +14,26 @@ void SMT100Component::update() { void SMT100Component::loop() { while (this->available() != 0) { if (this->readline_(this->read(), this->readline_buffer_, MAX_LINE_LENGTH) > 0) { - int counts = (int) strtol((strtok(this->readline_buffer_, ",")), nullptr, 10); - float permittivity = (float) strtod((strtok(nullptr, ",")), nullptr); - float moisture = (float) strtod((strtok(nullptr, ",")), nullptr); - float temperature = (float) strtod((strtok(nullptr, ",")), nullptr); - float voltage = (float) strtod((strtok(nullptr, ",")), nullptr); + char *token = strtok(this->readline_buffer_, ","); + if (!token) + continue; + int counts = (int) strtol(token, nullptr, 10); + token = strtok(nullptr, ","); + if (!token) + continue; + float permittivity = (float) strtod(token, nullptr); + token = strtok(nullptr, ","); + if (!token) + continue; + float moisture = (float) strtod(token, nullptr); + token = strtok(nullptr, ","); + if (!token) + continue; + float temperature = (float) strtod(token, nullptr); + token = strtok(nullptr, ","); + if (!token) + continue; + float voltage = (float) strtod(token, nullptr); if (this->counts_sensor_ != nullptr) { counts_sensor_->publish_state(counts); diff --git a/tests/integration/fixtures/uart_mock_ld2412_engineering.yaml b/tests/integration/fixtures/uart_mock_ld2412_engineering.yaml index 103dbed132f..a69e18888e4 100644 --- a/tests/integration/fixtures/uart_mock_ld2412_engineering.yaml +++ b/tests/integration/fixtures/uart_mock_ld2412_engineering.yaml @@ -102,6 +102,18 @@ uart_mock: 0xF8, 0xF7, 0xF6, 0xF5, ] +# Common filter definitions +.sensor_filters: &sensor_filters + filters: + - timeout: + timeout: 50ms + value: last + - throttle_with_priority: 50ms + +.binary_filters: &binary_filters + filters: + - settle: 50ms + ld2412: id: ld2412_dev uart_id: mock_uart @@ -111,107 +123,56 @@ sensor: ld2412_id: ld2412_dev moving_distance: name: "Moving Distance" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters still_distance: name: "Still Distance" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters moving_energy: name: "Moving Energy" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters still_energy: name: "Still Energy" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters detection_distance: name: "Detection Distance" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters light: name: "Light" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters gate_0: move_energy: name: "Gate 0 Move Energy" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters still_energy: name: "Gate 0 Still Energy" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters gate_1: move_energy: name: "Gate 1 Move Energy" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters still_energy: name: "Gate 1 Still Energy" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters gate_2: move_energy: name: "Gate 2 Move Energy" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters still_energy: name: "Gate 2 Still Energy" - filters: - - timeout: - timeout: 50ms - value: last - - throttle_with_priority: 50ms + <<: *sensor_filters binary_sensor: - platform: ld2412 ld2412_id: ld2412_dev has_target: name: "Has Target" - filters: - - settle: 50ms + <<: *binary_filters has_moving_target: name: "Has Moving Target" - filters: - - settle: 50ms + <<: *binary_filters has_still_target: name: "Has Still Target" - filters: - - settle: 50ms + <<: *binary_filters button: - platform: template diff --git a/tests/integration/fixtures/uart_mock_ld2412_engineering_truncated.yaml b/tests/integration/fixtures/uart_mock_ld2412_engineering_truncated.yaml new file mode 100644 index 00000000000..c0bd5147629 --- /dev/null +++ b/tests/integration/fixtures/uart_mock_ld2412_engineering_truncated.yaml @@ -0,0 +1,167 @@ +esphome: + name: uart-mock-ld2412-eng-trunc + +host: +api: +logger: + level: VERBOSE + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +# Dummy uart entry to satisfy ld2412's DEPENDENCIES = ["uart"] +uart: + baud_rate: 115200 + port: /dev/null + +uart_mock: + id: mock_uart + baud_rate: 256000 + auto_start: false + injections: + # Phase 1 (t=100ms): Valid engineering mode frame (52 bytes, buffer_pos_=52) + # Establishes baseline: gate_0_move=100, light=87 + - delay: 100ms + inject_rx: + [ + 0xF4, 0xF3, 0xF2, 0xF1, + 0x2A, 0x00, + 0x01, 0xAA, + 0x03, + 0x1E, 0x00, + 0x64, + 0x1E, 0x00, + 0x64, + 0x00, 0x00, + 0x64, 0x41, 0x06, 0x0E, 0x2B, 0x16, 0x03, 0x03, 0x07, 0x05, 0x09, 0x08, 0x07, 0x06, + 0x00, 0x00, 0x64, 0x64, 0x64, 0x64, 0x64, 0x64, 0x64, 0x50, 0x40, 0x30, 0x20, 0x10, + 0x57, + 0x55, 0x00, + 0xF8, 0xF7, 0xF6, 0xF5, + ] + + # Phase 2 (t=200ms): Truncated engineering mode frame (24 bytes, buffer_pos_=24) + # This frame has data_type=0x01 (engineering) but only enough data for the + # basic target fields, not the gate energies or light sensor. + # buffer_pos_=24 passes the old check (>= 12) but fails the new check (< 46). + # Without the fix, indices 17-45 would read stale buffer data from Phase 1. + # + # Layout (24 bytes): + # [0-3] F4 F3 F2 F1 = data frame header + # [4-5] 0E 00 = length 14 + # [6] 01 = data type (engineering mode) + # [7] AA = data header marker + # [8] 03 = target states (moving+still) + # [9-10] 1E 00 = moving distance 30 + # [11] 50 = moving energy 80 + # [12-13] 1E 00 = still distance 30 + # [14] 50 = still energy 80 + # [15-16] FF FF = garbage detection distance bytes + # [17] FF = padding (would be gate data in full frame) + # [18] 55 = data footer marker (at buffer_pos_ - 6) + # [19] 00 = check byte + # [20-23] F8 F7 F6 F5 = data frame footer + - delay: 100ms + inject_rx: + [ + 0xF4, 0xF3, 0xF2, 0xF1, + 0x0E, 0x00, + 0x01, 0xAA, + 0x03, + 0x1E, 0x00, + 0x50, + 0x1E, 0x00, + 0x50, + 0xFF, 0xFF, + 0xFF, + 0x55, 0x00, + 0xF8, 0xF7, 0xF6, 0xF5, + ] + + # Phase 3 (t=300ms): Valid recovery frame with different values + # gate_0_move=50, light=42 — proves component recovered + - delay: 100ms + inject_rx: + [ + 0xF4, 0xF3, 0xF2, 0xF1, + 0x2A, 0x00, + 0x01, 0xAA, + 0x03, + 0x1E, 0x00, + 0x64, + 0x1E, 0x00, + 0x64, + 0x00, 0x00, + 0x32, 0x20, 0x06, 0x0E, 0x2B, 0x16, 0x03, 0x03, 0x07, 0x05, 0x09, 0x08, 0x07, 0x06, + 0x00, 0x00, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x28, 0x20, 0x18, 0x10, 0x08, + 0x2A, + 0x55, 0x00, + 0xF8, 0xF7, 0xF6, 0xF5, + ] + +# Common filter definitions +.sensor_filters: &sensor_filters + filters: + - timeout: + timeout: 50ms + value: last + - throttle_with_priority: 50ms + +.binary_filters: &binary_filters + filters: + - settle: 50ms + +ld2412: + id: ld2412_dev + uart_id: mock_uart + +sensor: + - platform: ld2412 + ld2412_id: ld2412_dev + moving_distance: + name: "Moving Distance" + <<: *sensor_filters + still_distance: + name: "Still Distance" + <<: *sensor_filters + moving_energy: + name: "Moving Energy" + <<: *sensor_filters + still_energy: + name: "Still Energy" + <<: *sensor_filters + detection_distance: + name: "Detection Distance" + <<: *sensor_filters + light: + name: "Light" + <<: *sensor_filters + gate_0: + move_energy: + name: "Gate 0 Move Energy" + <<: *sensor_filters + still_energy: + name: "Gate 0 Still Energy" + <<: *sensor_filters + +binary_sensor: + - platform: ld2412 + ld2412_id: ld2412_dev + has_target: + name: "Has Target" + <<: *binary_filters + has_moving_target: + name: "Has Moving Target" + <<: *binary_filters + has_still_target: + name: "Has Still Target" + <<: *binary_filters + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + - lambda: 'id(mock_uart).start_scenario();' diff --git a/tests/integration/test_uart_mock_ld2412.py b/tests/integration/test_uart_mock_ld2412.py index a964ba00738..9b928ef14f2 100644 --- a/tests/integration/test_uart_mock_ld2412.py +++ b/tests/integration/test_uart_mock_ld2412.py @@ -14,6 +14,12 @@ test_uart_mock_ld2412_engineering (engineering mode): 2. Multi-byte still distance (291cm) using high byte > 0 3. Gate energy sensor values 4. Detection distance computed from target state + +test_uart_mock_ld2412_engineering_truncated (truncated engineering mode): + 1. Valid engineering frame establishes baseline sensor values + 2. Truncated engineering frame (24 bytes) is rejected — gate/light sensors + must not receive garbage from stale buffer data or frame footer bytes + 3. Recovery frame with different values proves the component survived """ from __future__ import annotations @@ -273,3 +279,122 @@ async def test_uart_mock_ld2412_engineering( ) assert pytest.approx(291.0) in collector.sensor_states["detection_distance"] + + +@pytest.mark.asyncio +async def test_uart_mock_ld2412_engineering_truncated( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that truncated engineering mode frames don't corrupt sensor values. + + Without the fix, a 24-byte engineering mode frame passes the old buffer_pos_ >= 12 + check but reads indices 17-45 from stale buffer data, publishing garbage values + (e.g. frame footer bytes 0xF8=248 as gate energy). + """ + external_components_path = str( + Path(__file__).parent / "fixtures" / "external_components" + ) + yaml_config = yaml_config.replace( + "EXTERNAL_COMPONENT_PATH", external_components_path + ) + + loop = asyncio.get_running_loop() + + # Track the truncated frame warning + truncated_warning_seen = loop.create_future() + + def line_callback(line: str) -> None: + if ( + "Engineering mode packet too short" in line + and not truncated_warning_seen.done() + ): + truncated_warning_seen.set_result(True) + + collector = SensorStateCollector( + sensor_names=[ + "moving_distance", + "still_distance", + "moving_energy", + "still_energy", + "detection_distance", + "light", + "gate_0_move_energy", + "gate_0_still_energy", + ], + binary_sensor_names=[ + "has_target", + "has_moving_target", + "has_still_target", + ], + ) + + # Signal when we see Phase 3 recovery values (gate_0_move=50) + recovery_received = collector.add_waiter( + lambda: pytest.approx(50.0) in collector.sensor_states["gate_0_move_energy"] + ) + + async with ( + run_compiled(yaml_config, line_callback=line_callback), + api_client_connected() as client, + ): + entities, _ = await client.list_entities_services() + collector.build_key_mapping(entities) + + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states( + initial_state_helper.on_state_wrapper(collector.on_state) + ) + + try: + await initial_state_helper.wait_for_initial_states() + except TimeoutError: + pytest.fail("Timeout waiting for initial states") + + start_btn = find_entity(entities, "start_scenario", ButtonInfo) + assert start_btn is not None, "Start Scenario button not found" + client.button_command(start_btn.key) + + # Wait for Phase 1 — valid engineering frame establishes baseline + try: + await collector.wait_for_all(timeout=3.0) + except TimeoutError: + pytest.fail( + f"Timeout waiting for Phase 1 frame. Received:\n" + f" sensor_states: {collector.sensor_states}\n" + f" binary_states: {collector.binary_states}" + ) + + # Phase 1 baseline: gate_0_move=100, light=87 + assert collector.sensor_states["gate_0_move_energy"][0] == pytest.approx(100.0) + assert collector.sensor_states["light"][0] == pytest.approx(87.0) + + # Wait for Phase 3 recovery frame (gate_0_move=50) + try: + await asyncio.wait_for(recovery_received, timeout=3.0) + except TimeoutError: + pytest.fail( + f"Timeout waiting for recovery frame. Received:\n" + f" gate_0_move_energy: {collector.sensor_states['gate_0_move_energy']}\n" + f" light: {collector.sensor_states['light']}" + ) + + # Verify the truncated frame warning was logged + assert truncated_warning_seen.done(), ( + "Expected 'Engineering mode packet too short' warning in logs" + ) + + # Phase 3 recovery: gate_0_move=50, light=42 + assert pytest.approx(50.0) in collector.sensor_states["gate_0_move_energy"] + assert pytest.approx(42.0) in collector.sensor_states["light"] + + # The critical assertion: gate_0_move_energy must never have received + # garbage values from the truncated frame. Without the fix, + # buffer_data_[17] = 0xFF = 255 would be published as gate_0_move. + for value in collector.sensor_states["gate_0_move_energy"]: + assert value == pytest.approx(100.0) or value == pytest.approx(50.0), ( + f"gate_0_move_energy got unexpected value {value} — " + f"truncated frame likely leaked stale buffer data. " + f"All values: {collector.sensor_states['gate_0_move_energy']}" + )