From 7575db0214941f8670eabe32f20c4eb4d069e81b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D1=83=D0=B1=D0=B8=D0=BD=D0=B8=D0=BD=20=D0=94=D0=BC?= =?UTF-8?q?=D0=B8=D1=82=D1=80=D0=B8=D0=B9?= Date: Tue, 9 Sep 2025 02:04:59 +0300 Subject: [PATCH] Add comprehensive documentation for safety measures - Document memory protection mechanisms in getFilesRecursive() - Explain file count, path length, and recursion depth limits - Improve code maintainability with clear safety constraints --- src/FSCommon.cpp | 39 +++++++++++++++++++++++++++++---------- src/mesh/PhoneAPI.cpp | 3 ++- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/FSCommon.cpp b/src/FSCommon.cpp index 0e04ecbf4..d08e6a443 100644 --- a/src/FSCommon.cpp +++ b/src/FSCommon.cpp @@ -104,11 +104,36 @@ bool renameFile(const char *pathFrom, const char *pathTo) #define MAX_FILES_IN_MANIFEST 50 // Reduced to be more conservative with memory #define MAX_PATH_LENGTH 200 // Maximum allowed path length to prevent overflow +/** + * @brief Helper function to validate and get file path for current platform + * + * @param file The file object + * @return const char* Valid path or nullptr if invalid + */ +static const char *getValidFilePath(File &file) +{ +#ifdef ARCH_ESP32 + const char *filePath = file.path(); + return (filePath && strlen(filePath) < MAX_PATH_LENGTH) ? filePath : nullptr; +#else + const char *fileName = file.name(); + return (fileName && strlen(fileName) < MAX_PATH_LENGTH) ? fileName : nullptr; +#endif +} + /** * @brief Get the list of files in a directory (internal recursive helper). * + * This function recursively lists files in the specified directory, subject to safety constraints + * to protect memory and stack usage: + * - Limits the total number of files collected to MAX_FILES_IN_MANIFEST (currently 50) to prevent memory overflow. + * - Limits the maximum allowed path length to MAX_PATH_LENGTH (currently 200) to prevent buffer overflow. + * - Limits recursion depth via the 'levels' parameter to avoid stack exhaustion. + * + * If any of these limits are reached, the function will stop collecting further files or recursing into subdirectories. + * * @param dirname The name of the directory. - * @param levels The number of levels of subdirectories to list. + * @param levels The number of levels of subdirectories to list (recursion depth). * @param filenames Reference to vector to populate with file info. */ static void getFilesRecursive(const char *dirname, uint8_t levels, std::vector &filenames) @@ -137,16 +162,10 @@ static void getFilesRecursive(const char *dirname, uint8_t levels, std::vector 0) { // Limit recursion depth -#ifdef ARCH_ESP32 - const char *filePath = file.path(); - if (filePath && strlen(filePath) < MAX_PATH_LENGTH) { - getFilesRecursive(filePath, levels - 1, filenames); + const char *validPath = getValidFilePath(file); + if (validPath) { + getFilesRecursive(validPath, levels - 1, filenames); } -#else - if (strlen(fileName) < MAX_PATH_LENGTH) { - getFilesRecursive(fileName, levels - 1, filenames); - } -#endif } file.close(); } else { diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index e2064206c..ba864fc2b 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -4,6 +4,7 @@ #endif #ifdef ARCH_ESP32 #include +#define MIN_HEAP_FOR_FILE_MANIFEST 8192 // Minimum heap required before generating file manifest #endif #include "Channels.h" @@ -72,7 +73,7 @@ void PhoneAPI::handleStartConfig() // Check available heap before getting files to prevent crash #ifdef ARCH_ESP32 size_t freeHeap = ESP.getFreeHeap(); - if (freeHeap < 8192) { // Require at least 8KB free heap + if (freeHeap < MIN_HEAP_FOR_FILE_MANIFEST) { LOG_WARN("Low memory (%d bytes), skipping file manifest", freeHeap); filesManifest.clear(); } else {