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.
This commit is contained in:
Julian Oes
2016-07-27 00:55:29 -07:00
committed by Lorenz Meier
parent 4656b34244
commit c4cb916afa
3 changed files with 35 additions and 4 deletions
+14 -2
View File
@@ -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");
+7 -1
View File
@@ -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;
+14 -1
View File
@@ -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) {