diff --git a/src/modules/mavlink/mavlink_log_handler.cpp b/src/modules/mavlink/mavlink_log_handler.cpp index 78249824af..ba7b92392e 100644 --- a/src/modules/mavlink/mavlink_log_handler.cpp +++ b/src/modules/mavlink/mavlink_log_handler.cpp @@ -54,9 +54,13 @@ static const char *kLogDir = PX4_STORAGEDIR "/log"; #define PX4_MAX_FILEPATH PATH_MAX #endif +// Size for log file path buffers - must match MavlinkLogHandler::LOG_FILEPATH_SIZE +#define LOG_FILEPATH_SIZE 256 +#define LOG_FILEPATH_MAX_CHARS (LOG_FILEPATH_SIZE - 1) // Max chars that can be read (leave room for null terminator) + // Ensure PX4_MAX_FILEPATH is large enough for our buffer sizes -// LogEntry.filepath is 256 bytes, so PX4_MAX_FILEPATH must be at least 256 -static_assert(PX4_MAX_FILEPATH >= 256, "PX4_MAX_FILEPATH must be at least 256 bytes for log file paths"); +static_assert(PX4_MAX_FILEPATH >= LOG_FILEPATH_SIZE, + "PX4_MAX_FILEPATH must be at least LOG_FILEPATH_SIZE bytes for log file paths"); MavlinkLogHandler::MavlinkLogHandler(Mavlink &mavlink) : _mavlink(mavlink) @@ -178,8 +182,9 @@ void MavlinkLogHandler::state_listing() char filepath[PX4_MAX_FILEPATH]; // If parsed lined successfully, send the entry - // Note: Using %255s to safely read into filepath buffer (max 255 chars + null terminator) - // This is conservative but safe across all platforms regardless of PX4_MAX_FILEPATH value + // Note: Using width specifier to prevent buffer overflow + // %255s reads max 255 chars + null terminator = 256 bytes (LOG_FILEPATH_SIZE) + // This is safe across all platforms (static_assert ensures PX4_MAX_FILEPATH >= LOG_FILEPATH_SIZE) if (sscanf(line, "%" PRIu32 " %" PRIu32 " %255s", &time_utc, &size_bytes, filepath) != 3) { PX4_DEBUG("sscanf failed"); continue; @@ -512,6 +517,8 @@ bool MavlinkLogHandler::log_entry_from_id(uint16_t log_id, LogEntry *entry) continue; } + // Parse log entry with width specifier to prevent buffer overflow + // %255s = max 255 chars + null = 256 bytes (LOG_FILEPATH_SIZE) if (sscanf(line, "%" PRIu32 " %" PRIu32 " %255s", &(entry->time_utc), &(entry->size_bytes), entry->filepath) != 3) { PX4_DEBUG("sscanf failed"); continue; diff --git a/src/modules/mavlink/mavlink_log_handler.h b/src/modules/mavlink/mavlink_log_handler.h index 0af53cd2c4..f7bff89cdb 100644 --- a/src/modules/mavlink/mavlink_log_handler.h +++ b/src/modules/mavlink/mavlink_log_handler.h @@ -48,12 +48,14 @@ public: void handle_message(const mavlink_message_t *msg); private: + static constexpr size_t LOG_FILEPATH_SIZE = 256; // Buffer size for log file paths + struct LogEntry { uint16_t id{0xffff}; uint32_t time_utc{}; uint32_t size_bytes{}; FILE *fp{nullptr}; - char filepath[256]; + char filepath[LOG_FILEPATH_SIZE]; uint32_t offset{}; };