From 8d99569643a14b1b09ad0abce7881c873569270f Mon Sep 17 00:00:00 2001 From: Jacob Dahl Date: Fri, 6 Mar 2026 14:36:34 -0900 Subject: [PATCH] fix(mavlink): bound recursion depth in delete_all_logs Prevent potential stack overflow from symlink loops or deeply nested directories by capping recursion to 3 levels. Also fixes dot-entry skipping to use strcmp instead of prefix check, and deduplicates the filepath construction. --- src/modules/mavlink/mavlink_log_handler.cpp | 50 +++++++++++---------- src/modules/mavlink/mavlink_log_handler.h | 2 +- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/modules/mavlink/mavlink_log_handler.cpp b/src/modules/mavlink/mavlink_log_handler.cpp index 1afa32382b..8aa8673f7a 100644 --- a/src/modules/mavlink/mavlink_log_handler.cpp +++ b/src/modules/mavlink/mavlink_log_handler.cpp @@ -522,9 +522,16 @@ bool MavlinkLogHandler::log_entry_from_id(uint16_t log_id, LogEntry *entry) return found_entry; } -void MavlinkLogHandler::delete_all_logs(const char *dir) +void MavlinkLogHandler::delete_all_logs(const char *dir, unsigned depth) { - //-- Open log directory + // Log structure is log/yyyy-mm-dd/file.ulg (2 levels). Cap recursion to prevent stack overflow. + static constexpr unsigned MAX_DEPTH = 3; + + if (depth > MAX_DEPTH) { + PX4_DEBUG("Max depth reached: %s", dir); + return; + } + DIR *dp = opendir(dir); if (dp == nullptr) { @@ -534,34 +541,29 @@ void MavlinkLogHandler::delete_all_logs(const char *dir) struct dirent *result = nullptr; while ((result = readdir(dp))) { - // no more entries? - if (result == nullptr) { - break; + + if (strcmp(result->d_name, ".") == 0 || strcmp(result->d_name, "..") == 0) { + continue; } - if (result->d_type == PX4LOG_DIRECTORY && result->d_name[0] != '.') { - char filepath[PX4_MAX_FILEPATH]; - int ret = snprintf(filepath, sizeof(filepath), "%s/%s", dir, result->d_name); - bool path_is_ok = (ret > 0) && (ret < (int)sizeof(filepath)); + char filepath[PX4_MAX_FILEPATH]; + int ret = snprintf(filepath, sizeof(filepath), "%s/%s", dir, result->d_name); + bool path_is_ok = (ret > 0) && (ret < (int)sizeof(filepath)); - if (path_is_ok) { - delete_all_logs(filepath); + if (!path_is_ok) { + continue; + } - if (rmdir(filepath)) { - PX4_DEBUG("Error removing %s", filepath); - } + if (result->d_type == PX4LOG_DIRECTORY) { + delete_all_logs(filepath, depth + 1); + + if (rmdir(filepath)) { + PX4_DEBUG("Error removing %s", filepath); } - } - if (result->d_type == PX4LOG_REGULAR_FILE) { - char filepath[PX4_MAX_FILEPATH]; - int ret = snprintf(filepath, sizeof(filepath), "%s/%s", dir, result->d_name); - bool path_is_ok = (ret > 0) && (ret < (int)sizeof(filepath)); - - if (path_is_ok) { - if (unlink(filepath)) { - PX4_DEBUG("Error unlinking %s", filepath); - } + } else { + if (unlink(filepath)) { + PX4_DEBUG("Error unlinking %s", filepath); } } } diff --git a/src/modules/mavlink/mavlink_log_handler.h b/src/modules/mavlink/mavlink_log_handler.h index eb521a61ee..176bba09fa 100644 --- a/src/modules/mavlink/mavlink_log_handler.h +++ b/src/modules/mavlink/mavlink_log_handler.h @@ -96,7 +96,7 @@ private: bool log_entry_from_id(uint16_t log_id, LogEntry *entry); // Log erase - void delete_all_logs(const char *dir); + void delete_all_logs(const char *dir, unsigned depth = 0); private: