Merge branch 'fix-string-usage' into 'main'

dhrishi · dhrishi · commit df47859a31e3 · 2026-05-28T16:01:09.000+08:00
components: fix unsafe string handling and enable stringop warnings

See merge request app-frameworks/esp-matter!1522
diff --git a/components/esp_matter/CMakeLists.txt b/components/esp_matter/CMakeLists.txt
@@ -104,6 +104,8 @@ idf_component_register( SRC_DIRS        ${SRC_DIRS_LIST}
 idf_build_set_property(COMPILE_OPTIONS "-Wno-error=uninitialized;-Wno-error=maybe-uninitialized;-Wno-missing-field-initializers;" APPEND)
 idf_build_set_property(COMPILE_OPTIONS "-Wno-error=array-bounds" APPEND)
 
+target_compile_options(${COMPONENT_LIB} PRIVATE "-Wstringop-truncation" "-Wstringop-overflow")
+
 if (CONFIG_ESP_MATTER_ENABLE_MATTER_SERVER)
     if (CONFIG_ESP_MATTER_ENABLE_DATA_MODEL)
         target_link_libraries(${COMPONENT_LIB} INTERFACE "-u data_model_utils_impl")
diff --git a/components/esp_matter/utils/jsontlv/json_to_tlv.cpp b/components/esp_matter/utils/jsontlv/json_to_tlv.cpp
@@ -198,8 +198,7 @@ static esp_err_t parse_json_name(const char *name, element_context &element_ctx,
                         "Failed to parse json name");
     ESP_RETURN_ON_ERROR(internal_convert_tlv_tag(tag_number, element_ctx.tag, implicit_profile_id), TAG,
                         "Failed to convert TLV tag");
-    strncpy(element_ctx.json_name, name, strlen(name));
-    element_ctx.json_name[strlen(name)] = 0;
+    strlcpy(element_ctx.json_name, name, sizeof(element_ctx.json_name));
     return ESP_OK;
 }
 
diff --git a/components/esp_matter_bridge/CMakeLists.txt b/components/esp_matter_bridge/CMakeLists.txt
@@ -8,3 +8,7 @@ endif()
 idf_component_register(SRC_DIRS        ${src_dirs_list}
                        INCLUDE_DIRS    ${include_dirs_list}
                        REQUIRES        esp_matter)
+
+if (CONFIG_ESP_MATTER_ENABLE_DATA_MODEL)
+    target_compile_options(${COMPONENT_LIB} PRIVATE "-Wstringop-truncation" "-Wstringop-overflow")
+endif()
diff --git a/components/esp_matter_console/CMakeLists.txt b/components/esp_matter_console/CMakeLists.txt
@@ -13,3 +13,7 @@ idf_component_register(SRC_DIRS ${src_dirs}
                     INCLUDE_DIRS .
                     EXCLUDE_SRCS ${exclude_srcs_list}
                     PRIV_REQUIRES ${priv_req})
+
+if (CONFIG_ENABLE_CHIP_SHELL)
+    target_compile_options(${COMPONENT_LIB} PRIVATE "-Wstringop-truncation" "-Wstringop-overflow")
+endif()
diff --git a/components/esp_matter_console/esp_matter_console_otcli.cpp b/components/esp_matter_console/esp_matter_console_otcli.cpp
@@ -37,7 +37,7 @@ static esp_err_t otcli_handler(int argc, char *argv[])
     /* the beginning of command "matter esp otcli" has already been removed */
     std::unique_ptr<char[]> cli_str(new char[CLI_INPUT_BUFF_LENGTH]);
     memset(cli_str.get(), 0, CLI_INPUT_BUFF_LENGTH);
-    uint8_t len = 0;
+    size_t len = 0;
     for (size_t i = 0; i < (size_t)argc; ++i) {
         len = len + strlen(argv[i]) + 1;
         if (len > CLI_INPUT_BUFF_LENGTH - 1) {
diff --git a/components/esp_matter_controller/CMakeLists.txt b/components/esp_matter_controller/CMakeLists.txt
@@ -46,3 +46,7 @@ idf_component_register(SRC_DIRS ${src_dirs_list}
     REQUIRES ${requires_list})
 
 idf_build_set_property(COMPILE_OPTIONS "-Wno-write-strings" APPEND)
+
+if (CONFIG_ESP_MATTER_CONTROLLER_ENABLE)
+    target_compile_options(${COMPONENT_LIB} PRIVATE "-Wstringop-truncation" "-Wstringop-overflow")
+endif()
diff --git a/components/esp_matter_controller/attestation_store/esp_matter_attestation_trust_store.cpp b/components/esp_matter_controller/attestation_store/esp_matter_attestation_trust_store.cpp
@@ -39,8 +39,11 @@ static const char *get_filename_extension(const char *filename)
 
 paa_der_cert_iterator::paa_der_cert_iterator(const char *path)
 {
-    strncpy(m_path, path, strnlen(path, 16));
-    m_path[15] = 0;
+    if (path == nullptr) {
+        assert(false);
+        return;
+    }
+    strlcpy(m_path, path, sizeof(m_path));
     m_dir = opendir(path);
     if (!m_dir) {
         ESP_LOGE(TAG, "Failed to open the directory");
@@ -258,8 +261,7 @@ CHIP_ERROR dcl_attestation_trust_store::GetProductAttestationAuthorityCert(const
     http_payload.Calloc(2400);
     ESP_GOTO_ON_FALSE(http_payload.Get(), ESP_ERR_NO_MEM, close, TAG, "Failed to alloc memory for http_payload");
     if (http_status_code == HttpStatus_Ok) {
-        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize());
-        http_payload[http_len] = '\0';
+        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize() - 1);
     } else {
         ESP_LOGE(TAG, "Status = %d. Invalid response for %s", http_status_code, url);
         ret = ESP_FAIL;
diff --git a/components/esp_matter_ota_provider/CMakeLists.txt b/components/esp_matter_ota_provider/CMakeLists.txt
@@ -13,3 +13,7 @@ idf_component_register(SRCS "${srcs}"
                        INCLUDE_DIRS "${include_dirs}"
                        PRIV_INCLUDE_DIRS "${priv_include_dirs}"
                        REQUIRES esp_matter esp_http_client json_parser)
+
+if (CONFIG_ESP_MATTER_OTA_PROVIDER_ENABLED)
+    target_compile_options(${COMPONENT_LIB} PRIVATE "-Wstringop-truncation" "-Wstringop-overflow")
+endif()
diff --git a/components/esp_matter_ota_provider/include/esp_matter_ota_bdx_sender.h b/components/esp_matter_ota_provider/include/esp_matter_ota_bdx_sender.h
@@ -48,7 +48,7 @@ class OtaBdxSender : public chip::bdx::Responder {
 
     void SetOtaImageUrl(const char *otaImageUrl)
     {
-        strncpy(mOtaImageUrl, otaImageUrl, strnlen(otaImageUrl, OTA_URL_MAX_LEN));
+        strlcpy(mOtaImageUrl, otaImageUrl, sizeof(mOtaImageUrl));
     }
 
     const char *GetOtaImageUrl() const
diff --git a/components/esp_matter_ota_provider/src/esp_matter_ota_candidates.cpp b/components/esp_matter_ota_provider/src/esp_matter_ota_candidates.cpp
@@ -92,7 +92,7 @@ static int _search_ota_candidate_from_cache(uint16_t vendor_id, uint16_t product
 
 static size_t _find_empty_ota_candidates()
 {
-    uint8_t oldest_candidate_lifetime = 0;
+    uint32_t oldest_candidate_lifetime = 0;
     size_t oldest_candidate_index = 0;
     for (size_t index = 0; index < max_ota_candidate_count; ++index) {
         if (!_ota_candidates_cache[index]) {
@@ -160,11 +160,9 @@ static esp_err_t _query_software_version_array(const uint16_t vendor_id, const u
     http_payload.Calloc(1024);
     if ((http_len > 0) && (http_status_code == 200)) {
         ESP_GOTO_ON_FALSE(http_payload.Get(), ESP_ERR_NO_MEM, close, TAG, "Failed to alloc memory for http_payload");
-        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize());
-        http_payload[http_len] = '\0';
+        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize() - 1);
     } else {
-        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize());
-        http_payload[http_len] = '\0';
+        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize() - 1);
         ESP_LOGE(TAG, "Invalid response for %s", url);
         ESP_LOGE(TAG, "Status = %d, Data = %s", http_status_code, http_len > 0 ? http_payload.Get() : "None");
         ret = ESP_FAIL;
@@ -253,11 +251,9 @@ static esp_err_t _query_ota_candidate(model_version_t *model, uint32_t new_softw
     http_payload.Calloc(1024);
     if ((http_len > 0) && (http_status_code == 200)) {
         ESP_GOTO_ON_FALSE(http_payload.Get(), ESP_ERR_NO_MEM, close, TAG, "Failed to alloc memory for http_payload");
-        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize());
-        http_payload[http_len] = '\0';
+        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize() - 1);
     } else {
-        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize());
-        http_payload[http_len] = '\0';
+        http_len = esp_http_client_read_response(client, http_payload.Get(), http_payload.AllocatedSize() - 1);
         ESP_LOGE(TAG, "Invalid response for %s", url);
         ESP_LOGE(TAG, "Status = %d, Data = %s", http_status_code, http_len > 0 ? http_payload.Get() : "None");
         ret = ESP_FAIL;
@@ -290,6 +286,9 @@ static esp_err_t _query_ota_candidate(model_version_t *model, uint32_t new_softw
             }
             if (json_obj_get_strlen(&jctx, "otaUrl", &string_len) == 0 &&
                     json_obj_get_string(&jctx, "otaUrl", model->ota_url, sizeof(model->ota_url)) == 0) {
+                string_len = string_len < sizeof(model->ota_url) - 1
+                             ? string_len
+                             : sizeof(model->ota_url) - 1;
                 model->ota_url[string_len] = 0;
             }
         } else {
diff --git a/components/esp_matter_ota_provider/src/esp_matter_ota_provider.cpp b/components/esp_matter_ota_provider/src/esp_matter_ota_provider.cpp
@@ -58,6 +58,10 @@ static void GetUpdateTokenString(const ByteSpan &token, char *buf, size_t bufSiz
 {
     const uint8_t *tokenData = static_cast<const uint8_t *>(token.data());
     size_t minLength = std::min(token.size(), bufSize);
+    if (minLength < 2 && bufSize > 0) {
+        buf[0] = 0;
+        return;
+    }
     for (size_t i = 0; i < (minLength / 2) - 1; ++i) {
         snprintf(&buf[i * 2], bufSize, "%02X", tokenData[i]);
     }
@@ -116,7 +120,11 @@ void EspOtaProvider::SendQueryImageResponse(OTAQueryStatus status)
         MutableCharSpan uri(requestor->mImageUri);
         char otaFileName[128] = {0};
         char *ptr = strrchr(requestor->mOtaImageUrl, '/');
-        strncpy(otaFileName, ptr + 1, strnlen(ptr + 1, 255));
+        if (!ptr) {
+            ESP_LOGE(TAG, "Invalid OTA image URL: missing '/'");
+            return;
+        }
+        strlcpy(otaFileName, ptr + 1, sizeof(otaFileName));
         CHIP_ERROR error = chip::bdx::MakeURI(providerNodeId, CharSpan::fromCharString(otaFileName), uri);
         if (error != CHIP_NO_ERROR) {
             ESP_LOGE(TAG, "Cannot generate URI");
@@ -332,6 +340,7 @@ esp_err_t EspOtaProvider::CreateOtaRequestorEntry(const chip::ScopedNodeId &node
         if (!entry) {
             return ESP_ERR_NO_MEM;
         }
+        memset(entry, 0, sizeof(EspOtaRequestorEntry));
         entry->mNodeId = nodeId;
         entry->mOtaAllowed = mOtaAllowedDefault;
         entry->mNext = mOtaRequestorList;
diff --git a/components/esp_matter_rainmaker/CMakeLists.txt b/components/esp_matter_rainmaker/CMakeLists.txt
@@ -10,3 +10,7 @@ endif()
 idf_component_register(SRCS             ${SRCS_LIST}
                        INCLUDE_DIRS     "."
                        REQUIRES         ${REQUIRES_LIST})
+
+if (${rainmaker_enabled})
+    target_compile_options(${COMPONENT_LIB} PRIVATE "-Wstringop-truncation" "-Wstringop-overflow")
+endif()

Original file line number	Diff line number	Diff line change
`@@ -198,8 +198,7 @@ static esp_err_t parse_json_name(const char *name, element_context &element_ctx,`
`198`	`198`	`"Failed to parse json name");`
`199`	`199`	`ESP_RETURN_ON_ERROR(internal_convert_tlv_tag(tag_number, element_ctx.tag, implicit_profile_id), TAG,`
`200`	`200`	`"Failed to convert TLV tag");`
`201`		`- strncpy(element_ctx.json_name, name, strlen(name));`
`202`		`- element_ctx.json_name[strlen(name)] = 0;`
	`201`	`+ strlcpy(element_ctx.json_name, name, sizeof(element_ctx.json_name));`
`203`	`202`	`return ESP_OK;`
`204`	`203`	`}`
`205`	`204`
Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ class OtaBdxSender : public chip::bdx::Responder {`
`48`	`48`
`49`	`49`	`void SetOtaImageUrl(const char *otaImageUrl)`
`50`	`50`	`{`
`51`		`- strncpy(mOtaImageUrl, otaImageUrl, strnlen(otaImageUrl, OTA_URL_MAX_LEN));`
	`51`	`+ strlcpy(mOtaImageUrl, otaImageUrl, sizeof(mOtaImageUrl));`
`52`	`52`	`}`
`53`	`53`
`54`	`54`	`const char *GetOtaImageUrl() const`