From c4cb916afaebe457ee86806fb4cafbd942469419 Mon Sep 17 00:00:00 2001
From: Julian Oes <julian@oes.ch>
Date: Wed, 27 Jul 2016 09:55:29 +0200
Subject: [PATCH] Fix sdlog2/logger path/file name overflows. (#5138)

* logger: prevent logpath buffer overflows

The handling of the log path had the potential to cause buffer
overflows, especially on POSIX platforms where the paths are often much
longer than just 64 chars.

* sdlog2: prevent logpath buffer overflows

When the log folder path was created, this was done with the unsafe
sprintf function instead of snprintf. This caused buffer overflows in
SITL but the overflow was usually not detected until recent testing of
some work in progress.
---
 src/modules/logger/logger.cpp | 16 ++++++++++++++--
 src/modules/logger/logger.h   |  8 +++++++-
 src/modules/sdlog2/sdlog2.c   | 15 ++++++++++++++-
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/src/modules/logger/logger.cpp b/src/modules/logger/logger.cpp
index 034580d99b..0331ebf2ba 100644
--- a/src/modules/logger/logger.cpp
+++ b/src/modules/logger/logger.cpp
@@ -860,6 +860,12 @@ int Logger::create_log_dir(tm *tt)
 
 	if (tt) {
 		int n = snprintf(_log_dir, sizeof(_log_dir), "%s/", LOG_ROOT);
+
+		if (n >= sizeof(_log_dir)) {
+			PX4_ERR("log path too long");
+			return -1;
+		}
+
 		strftime(_log_dir + n, sizeof(_log_dir) - n, "%Y-%m-%d", tt);
 		mkdir_ret = mkdir(_log_dir, S_IRWXU | S_IRWXG | S_IRWXO);
 
@@ -874,7 +880,13 @@ int Logger::create_log_dir(tm *tt)
 		/* look for the next dir that does not exist */
 		while (!_has_log_dir && dir_number <= MAX_NO_LOGFOLDER) {
 			/* format log dir: e.g. /fs/microsd/sess001 */
-			sprintf(_log_dir, "%s/sess%03u", LOG_ROOT, dir_number);
+			int n = snprintf(_log_dir, sizeof(_log_dir), "%s/sess%03u", LOG_ROOT, dir_number);
+
+			if (n >= sizeof(_log_dir)) {
+				PX4_ERR("log path too long");
+				return -1;
+			}
+
 			mkdir_ret = mkdir(_log_dir, S_IRWXU | S_IRWXG | S_IRWXO);
 
 			if (mkdir_ret == 0) {
@@ -1030,7 +1042,7 @@ void Logger::start_log()
 
 	PX4_INFO("start log");
 
-	char file_name[64] = "";
+	char file_name[LOG_DIR_LEN] = "";
 
 	if (get_log_file_name(file_name, sizeof(file_name))) {
 		PX4_ERR("logger: failed to get log file name");
diff --git a/src/modules/logger/logger.h b/src/modules/logger/logger.h
index eb11ed56bd..44ad42ebbf 100644
--- a/src/modules/logger/logger.h
+++ b/src/modules/logger/logger.h
@@ -47,6 +47,12 @@ extern "C" __EXPORT int logger_main(int argc, char *argv[]);
 #define TRY_SUBSCRIBE_INTERVAL 1000*1000	// interval in microseconds at which we try to subscribe to a topic
 // if we haven't succeeded before
 
+#ifdef __PX4_NUTTX
+#define LOG_DIR_LEN 64
+#else
+#define LOG_DIR_LEN 256
+#endif
+
 namespace px4
 {
 namespace logger
@@ -210,7 +216,7 @@ private:
 	uint8_t						*_msg_buffer = nullptr;
 	int						_msg_buffer_len = 0;
 	bool						_task_should_exit = true;
-	char 						_log_dir[64];
+	char 						_log_dir[LOG_DIR_LEN];
 	bool						_has_log_dir = false;
 	bool						_enabled = false;
 	bool						_was_armed = false;
diff --git a/src/modules/sdlog2/sdlog2.c b/src/modules/sdlog2/sdlog2.c
index f1fc820d81..a787a76e15 100644
--- a/src/modules/sdlog2/sdlog2.c
+++ b/src/modules/sdlog2/sdlog2.c
@@ -170,7 +170,11 @@ struct logbuffer_s lb;
 static pthread_mutex_t logbuffer_mutex;
 static pthread_cond_t logbuffer_cond;
 
+#ifdef __PX4_NUTTX
 #define LOG_BASE_PATH_LEN	64
+#else
+#define LOG_BASE_PATH_LEN	256
+#endif
 
 static char log_dir[LOG_BASE_PATH_LEN];
 
@@ -452,6 +456,10 @@ int create_log_dir()
 
 	if (log_name_timestamp && time_ok) {
 		int n = snprintf(log_dir, sizeof(log_dir), "%s/", log_root);
+		if (n >= sizeof(log_dir)) {
+			PX4_ERR("log path too long");
+			return -1;
+		}
 		strftime(log_dir + n, sizeof(log_dir) - n, "%Y-%m-%d", &tt);
 		mkdir_ret = mkdir(log_dir, S_IRWXU | S_IRWXG | S_IRWXO);
 
@@ -466,7 +474,12 @@ int create_log_dir()
 		 * let's re-use it. */
 		while (dir_number <= MAX_NO_LOGFOLDER && !sess_folder_created) {
 			/* format log dir: e.g. /fs/microsd/sess001 */
-			sprintf(log_dir, "%s/sess%03u", log_root, dir_number);
+			int n = snprintf(log_dir, sizeof(log_dir), "%s/sess%03u", log_root, dir_number);
+			if (n >= sizeof(log_dir)) {
+				PX4_ERR("log path too long");
+				return -1;
+			}
+
 			mkdir_ret = mkdir(log_dir, S_IRWXU | S_IRWXG | S_IRWXO);
 
 			if (mkdir_ret == 0) {