From 4c9be53120c18e0addbfb2dd8926a7be306bb4ad Mon Sep 17 00:00:00 2001 From: zwx Date: Thu, 2 Jul 2026 15:31:46 +0800 Subject: [PATCH] fix(openthread): fix null pointer deref, uninitialized struct, and unbounded strcpy in spinel/RCP code --- components/openthread/include/esp_radio_spinel.h | 3 ++- components/openthread/src/ncp/esp_openthread_ncp.cpp | 4 ++++ .../openthread/src/port/esp_openthread_spi_slave.c | 12 +++++++----- .../openthread/src/spinel/esp_radio_spinel.cpp | 11 +++++++++-- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/components/openthread/include/esp_radio_spinel.h b/components/openthread/include/esp_radio_spinel.h index 9cf3db0079a..18fff09d3fa 100644 --- a/components/openthread/include/esp_radio_spinel.h +++ b/components/openthread/include/esp_radio_spinel.h @@ -414,7 +414,8 @@ esp_err_t esp_radio_spinel_rcp_deinit(esp_radio_spinel_idx_t idx); /** * @brief Get the version of RCP. * - * @param[in] running_rcp_version A pointer to the RCP version string. + * @param[in] running_rcp_version A pointer to a caller-allocated buffer of at least 128 bytes, + * to be filled with the null-terminated RCP version string. * @param[in] idx The index of 802.15.4 related protocol stack. * * @return diff --git a/components/openthread/src/ncp/esp_openthread_ncp.cpp b/components/openthread/src/ncp/esp_openthread_ncp.cpp index eb0bad9c508..84b0c5da9de 100644 --- a/components/openthread/src/ncp/esp_openthread_ncp.cpp +++ b/components/openthread/src/ncp/esp_openthread_ncp.cpp @@ -180,6 +180,10 @@ otError NcpBase::VendorSetPropertyHandler(spinel_prop_key_t aPropKey) int32_t pending_mode = 0; mDecoder.ReadInt32(pending_mode); + if (pending_mode < ESP_IEEE802154_AUTO_PENDING_DISABLE || pending_mode > ESP_IEEE802154_AUTO_PENDING_ZIGBEE) { + error = OT_ERROR_INVALID_ARGS; + break; + } esp_ieee802154_set_pending_mode(static_cast(pending_mode)); break; } diff --git a/components/openthread/src/port/esp_openthread_spi_slave.c b/components/openthread/src/port/esp_openthread_spi_slave.c index 787daadd2ea..02ea5f41994 100644 --- a/components/openthread/src/port/esp_openthread_spi_slave.c +++ b/components/openthread/src/port/esp_openthread_spi_slave.c @@ -135,14 +135,16 @@ err: void esp_openthread_spi_slave_deinit(void) { - spi_slave_free(s_spi_config->host_device); - s_spi_config->slave_config.post_setup_cb = NULL; - s_spi_config->slave_config.post_trans_cb = NULL; - heap_caps_free(s_spi_config); + if (s_spi_config != NULL) { + spi_slave_free(s_spi_config->host_device); + s_spi_config->slave_config.post_setup_cb = NULL; + s_spi_config->slave_config.post_trans_cb = NULL; + heap_caps_free(s_spi_config); + s_spi_config = NULL; + } heap_caps_free(s_spi_transaction); heap_caps_free(s_pending_transaction); heap_caps_free(s_rx_dma_buf); - s_spi_config = NULL; s_spi_transaction = NULL; s_pending_transaction = NULL; s_rx_dma_buf = NULL; diff --git a/components/openthread/src/spinel/esp_radio_spinel.cpp b/components/openthread/src/spinel/esp_radio_spinel.cpp index f7de1edb489..64840ad97f7 100644 --- a/components/openthread/src/spinel/esp_radio_spinel.cpp +++ b/components/openthread/src/spinel/esp_radio_spinel.cpp @@ -19,6 +19,8 @@ #define SPINEL_VENDOR_PROPERTY_BIT_PENDINGMODE BIT(0) #define SPINEL_VENDOR_PROPERTY_BIT_COORDINATOR BIT(1) +// Must match ot::Spinel::SpinelDriver::kVersionStringSize (private, spinel_driver.hpp). +#define ESP_RADIO_SPINEL_RCP_VERSION_MAX_SIZE 128 static esp_ieee802154_pending_mode_t s_spinel_vendor_property_pendingmode[ot::Spinel::kSpinelHeaderMaxNumIid] = {ESP_IEEE802154_AUTO_PENDING_DISABLE}; static bool s_spinel_vendor_property_coordinator[ot::Spinel::kSpinelHeaderMaxNumIid] = {false}; static uint64_t s_spinel_vendor_property_mask[ot::Spinel::kSpinelHeaderMaxNumIid] = {0}; @@ -128,7 +130,7 @@ void TransmitDone(otInstance *aInstance, otRadioFrame *aFrame, otRadioFrame *aAc uint8_t *frame = (uint8_t *)calloc(1, aFrame->mLength + 1); uint8_t *ack = nullptr; if (frame) { - esp_ieee802154_frame_info_t ack_info; + esp_ieee802154_frame_info_t ack_info = {}; frame[0] = aFrame->mLength; memcpy((void *)(frame + 1), aFrame->mPsdu, frame[0]); if (aAckFrame) { @@ -136,6 +138,11 @@ void TransmitDone(otInstance *aInstance, otRadioFrame *aFrame, otRadioFrame *aAc if (ack) { ack[0] = aAckFrame->mLength; memcpy((void *)(ack + 1), aAckFrame->mPsdu, ack[0]); + ack_info.rssi = aAckFrame->mInfo.mRxInfo.mRssi; + ack_info.channel = aAckFrame->mChannel; + ack_info.lqi = aAckFrame->mInfo.mRxInfo.mLqi; + ack_info.timestamp = aAckFrame->mInfo.mRxInfo.mTimestamp; + ack_info.pending = aAckFrame->mInfo.mRxInfo.mAckedWithFramePending; } else { ESP_LOGE(ESP_SPINEL_LOG_TAG, "Fail to alloc memory for ack"); } @@ -389,7 +396,7 @@ esp_err_t esp_radio_spinel_rcp_version_get(char *running_rcp_version, esp_radio_ { const char *rcp_version = s_radio[idx].GetVersion(); ESP_RETURN_ON_FALSE(rcp_version != nullptr, ESP_FAIL, ESP_SPINEL_LOG_TAG, "Fail to get rcp version"); - strcpy(running_rcp_version, rcp_version); + strlcpy(running_rcp_version, rcp_version, ESP_RADIO_SPINEL_RCP_VERSION_MAX_SIZE); return ESP_OK; }