From 41501c4f23aa26d1218dc064270a88eca0de565c Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Wed, 1 Dec 2021 12:16:49 +0800 Subject: [PATCH] bugfix (nvs): Fixed issues found by Coverity * Fixed potential memory leak * Fixed wrong strncpy usage * Fixed potential out of bounds access --- components/nvs_flash/include/nvs.h | 21 ++++++----------- components/nvs_flash/src/nvs_page.cpp | 27 ++++++++++----------- components/nvs_flash/src/nvs_storage.cpp | 30 +++++++++++------------- 3 files changed, 33 insertions(+), 45 deletions(-) diff --git a/components/nvs_flash/include/nvs.h b/components/nvs_flash/include/nvs.h index 79c851bb33..55d74f0636 100644 --- a/components/nvs_flash/include/nvs.h +++ b/components/nvs_flash/include/nvs.h @@ -1,16 +1,8 @@ -// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #ifndef ESP_NVS_H #define ESP_NVS_H @@ -65,6 +57,7 @@ typedef nvs_handle_t nvs_handle IDF_DEPRECATED("Replace with nvs_handle_t"); #define NVS_DEFAULT_PART_NAME "nvs" /*!< Default partition name of the NVS partition in the partition table */ #define NVS_PART_NAME_MAX_SIZE 16 /*!< maximum length of partition name (excluding null terminator) */ +#define NVS_KEY_NAME_MAX_SIZE 16 /*!< Maximal length of NVS key name (including null terminator) */ /** * @brief Mode of opening the non-volatile storage @@ -103,7 +96,7 @@ typedef enum { */ typedef struct { char namespace_name[16]; /*!< Namespace to which key-value belong */ - char key[16]; /*!< Key of stored key-value pair */ + char key[NVS_KEY_NAME_MAX_SIZE]; /*!< Key of stored key-value pair */ nvs_type_t type; /*!< Type of stored key-value pair */ } nvs_entry_info_t; diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index 72aee09091..98f459f946 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -1,16 +1,8 @@ -// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #include "nvs_page.hpp" #if defined(ESP_PLATFORM) #include @@ -195,6 +187,10 @@ esp_err_t Page::writeItem(uint8_t nsIndex, ItemType datatype, const char* key, c return ESP_ERR_NVS_VALUE_TOO_LONG; } + if ((!isVariableLengthType(datatype)) && dataSize > 8) { + return ESP_ERR_INVALID_ARG; + } + size_t totalSize = ENTRY_SIZE; size_t entriesCount = 1; if (isVariableLengthType(datatype)) { @@ -235,7 +231,8 @@ esp_err_t Page::writeItem(uint8_t nsIndex, ItemType datatype, const char* key, c return err; } - size_t left = dataSize / ENTRY_SIZE * ENTRY_SIZE; + size_t rest = dataSize % ENTRY_SIZE; + size_t left = dataSize - rest; if (left > 0) { err = writeEntryData(static_cast(data), left); if (err != ESP_OK) { @@ -243,7 +240,7 @@ esp_err_t Page::writeItem(uint8_t nsIndex, ItemType datatype, const char* key, c } } - size_t tail = dataSize - left; + size_t tail = rest; if (tail > 0) { std::fill_n(item.rawData, ENTRY_SIZE, 0xff); memcpy(item.rawData, static_cast(data) + left, tail); diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 62d61b86ac..f9882825a6 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -1,16 +1,8 @@ -// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at - -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #include "nvs_storage.hpp" #ifndef ESP_PLATFORM @@ -402,7 +394,11 @@ esp_err_t Storage::createOrOpenNamespace(const char* nsName, bool canCreate, uin mNamespaceUsage.set(ns, true); nsIndex = ns; - NamespaceEntry* entry = new NamespaceEntry; + NamespaceEntry* entry = new (std::nothrow) NamespaceEntry; + if (!entry) { + return ESP_ERR_NO_MEM; + } + entry->mIndex = ns; strncpy(entry->mName, nsName, sizeof(entry->mName) - 1); entry->mName[sizeof(entry->mName) - 1] = 0; @@ -707,11 +703,13 @@ esp_err_t Storage::calcEntriesInNamespace(uint8_t nsIndex, size_t& usedEntries) void Storage::fillEntryInfo(Item &item, nvs_entry_info_t &info) { info.type = static_cast(item.datatype); - strncpy(info.key, item.key, sizeof(info.key)); + strncpy(info.key, item.key, sizeof(info.key) - 1); + info.key[sizeof(info.key) - 1] = '\0'; for (auto &name : mNamespaces) { if(item.nsIndex == name.mIndex) { - strncpy(info.namespace_name, name.mName, sizeof(info.namespace_name)); + strncpy(info.namespace_name, name.mName, sizeof(info.namespace_name) - 1); + info.namespace_name[sizeof(info.namespace_name) -1] = '\0'; break; } }