|
 |
59b6f52 |
From 79e3bc1bc4d96f48223d4667f78d599ac8905f42 Mon Sep 17 00:00:00 2001
|
|
|
59b6f52 |
From: Florent Rougon <f.rougon@free.fr>
|
|
|
59b6f52 |
Date: Sat, 26 Aug 2017 16:36:54 +0200
|
|
|
59b6f52 |
Subject: [PATCH] Security: don't allow FGLogger to overwrite arbitrary files
|
|
|
59b6f52 |
|
|
|
59b6f52 |
Since the paths of files written by FGLogger come from the property
|
|
|
59b6f52 |
tree[1], they must be validated before we decide to write to these
|
|
|
59b6f52 |
files.
|
|
|
59b6f52 |
|
|
|
59b6f52 |
[1] Except for the "empty" case, which uses the default name
|
|
|
59b6f52 |
'fg_log.csv'.
|
|
|
59b6f52 |
|
|
|
59b6f52 |
(cherry picked from commit 2a5e3d06b2c0d9f831063afe7e7260bca456d679)
|
|
|
59b6f52 |
---
|
|
|
59b6f52 |
src/Main/logger.cxx | 29 +++++++++++++++++++++++++++--
|
|
|
59b6f52 |
src/Main/logger.hxx | 4 ++--
|
|
|
59b6f52 |
2 files changed, 29 insertions(+), 4 deletions(-)
|
|
|
59b6f52 |
|
|
|
59b6f52 |
diff --git a/src/Main/logger.cxx b/src/Main/logger.cxx
|
|
|
59b6f52 |
index 6c18162c3..00b6833ca 100644
|
|
|
59b6f52 |
--- a/src/Main/logger.cxx
|
|
|
59b6f52 |
+++ b/src/Main/logger.cxx
|
|
|
59b6f52 |
@@ -9,12 +9,17 @@
|
|
|
59b6f52 |
|
|
|
59b6f52 |
#include "logger.hxx"
|
|
|
59b6f52 |
|
|
|
59b6f52 |
-#include <fstream>
|
|
|
59b6f52 |
+#include <ios>
|
|
|
59b6f52 |
#include <string>
|
|
|
59b6f52 |
+#include <cstdlib>
|
|
|
59b6f52 |
|
|
|
59b6f52 |
#include <simgear/debug/logstream.hxx>
|
|
|
59b6f52 |
+#include <simgear/io/iostreams/sgstream.hxx>
|
|
|
59b6f52 |
+#include <simgear/misc/sg_path.hxx>
|
|
|
59b6f52 |
|
|
|
59b6f52 |
#include "fg_props.hxx"
|
|
|
59b6f52 |
+#include "globals.hxx"
|
|
|
59b6f52 |
+#include "util.hxx"
|
|
|
59b6f52 |
|
|
|
59b6f52 |
using std::string;
|
|
|
59b6f52 |
using std::endl;
|
|
|
59b6f52 |
@@ -59,6 +64,25 @@ FGLogger::init ()
|
|
|
59b6f52 |
child->setStringValue("filename", filename.c_str());
|
|
|
59b6f52 |
}
|
|
|
59b6f52 |
|
|
|
59b6f52 |
+ // Security: the path comes from the global Property Tree; it *must* be
|
|
|
59b6f52 |
+ // validated before we overwrite the file.
|
|
|
59b6f52 |
+ const SGPath authorizedPath = fgValidatePath(SGPath::fromUtf8(filename),
|
|
|
59b6f52 |
+ /* write */ true);
|
|
|
59b6f52 |
+
|
|
|
59b6f52 |
+ if (authorizedPath.isNull()) {
|
|
|
59b6f52 |
+ const string propertyPath = child->getChild("filename")
|
|
|
59b6f52 |
+ ->getPath(/* simplify */ true);
|
|
|
59b6f52 |
+ const string msg =
|
|
|
59b6f52 |
+ "The FGLogger logging system, via the '" + propertyPath + "' property, "
|
|
|
59b6f52 |
+ "was asked to write to '" + filename + "', however this path is not "
|
|
|
59b6f52 |
+ "authorized for writing anymore for security reasons. " +
|
|
|
59b6f52 |
+ "Please choose another location, for instance in the $FG_HOME/Export "
|
|
|
59b6f52 |
+ "folder (" + (globals->get_fg_home() / "Export").utf8Str() + ").";
|
|
|
59b6f52 |
+
|
|
|
59b6f52 |
+ SG_LOG(SG_GENERAL, SG_ALERT, msg);
|
|
|
59b6f52 |
+ exit(EXIT_FAILURE);
|
|
|
59b6f52 |
+ }
|
|
|
59b6f52 |
+
|
|
|
59b6f52 |
string delimiter = child->getStringValue("delimiter");
|
|
|
59b6f52 |
if (delimiter.empty()) {
|
|
|
59b6f52 |
delimiter = ",";
|
|
|
59b6f52 |
@@ -68,7 +92,8 @@ FGLogger::init ()
|
|
|
59b6f52 |
log.interval_ms = child->getLongValue("interval-ms");
|
|
|
59b6f52 |
log.last_time_ms = globals->get_sim_time_sec() * 1000;
|
|
|
59b6f52 |
log.delimiter = delimiter.c_str()[0];
|
|
|
59b6f52 |
- log.output = new std::ofstream(filename.c_str());
|
|
|
59b6f52 |
+ // Security: use the return value of fgValidatePath()
|
|
|
59b6f52 |
+ log.output = new sg_ofstream(authorizedPath, std::ios_base::out);
|
|
|
59b6f52 |
if (!log.output) {
|
|
|
59b6f52 |
SG_LOG(SG_GENERAL, SG_ALERT, "Cannot write log to " << filename);
|
|
|
59b6f52 |
continue;
|
|
|
59b6f52 |
diff --git a/src/Main/logger.hxx b/src/Main/logger.hxx
|
|
|
59b6f52 |
index 3d2146a83..de6209756 100644
|
|
|
59b6f52 |
--- a/src/Main/logger.hxx
|
|
|
59b6f52 |
+++ b/src/Main/logger.hxx
|
|
|
59b6f52 |
@@ -6,10 +6,10 @@
|
|
|
59b6f52 |
#ifndef __LOGGER_HXX
|
|
|
59b6f52 |
#define __LOGGER_HXX 1
|
|
|
59b6f52 |
|
|
|
59b6f52 |
-#include <iosfwd>
|
|
|
59b6f52 |
#include <vector>
|
|
|
59b6f52 |
|
|
|
59b6f52 |
#include <simgear/compiler.h>
|
|
|
59b6f52 |
+#include <simgear/io/iostreams/sgstream.hxx>
|
|
|
59b6f52 |
#include <simgear/structure/subsystem_mgr.hxx>
|
|
|
59b6f52 |
#include <simgear/props/props.hxx>
|
|
|
59b6f52 |
|
|
|
59b6f52 |
@@ -39,7 +39,7 @@ private:
|
|
|
59b6f52 |
Log ();
|
|
|
59b6f52 |
virtual ~Log ();
|
|
|
59b6f52 |
std::vector<SGPropertyNode_ptr> nodes;
|
|
|
59b6f52 |
- std::ostream * output;
|
|
|
59b6f52 |
+ sg_ofstream * output;
|
|
|
59b6f52 |
long interval_ms;
|
|
|
59b6f52 |
double last_time_ms;
|
|
|
59b6f52 |
char delimiter;
|
|
|
59b6f52 |
--
|
|
|
59b6f52 |
2.13.5
|
|
|
59b6f52 |
|