From 79e3bc1bc4d96f48223d4667f78d599ac8905f42 Mon Sep 17 00:00:00 2001
From: Florent Rougon <f.rougon@free.fr>
Date: Sat, 26 Aug 2017 16:36:54 +0200
Subject: [PATCH] Security: don't allow FGLogger to overwrite arbitrary files

Since the paths of files written by FGLogger come from the property
tree[1], they must be validated before we decide to write to these
files.

[1] Except for the "empty" case, which uses the default name
    'fg_log.csv'.

(cherry picked from commit 2a5e3d06b2c0d9f831063afe7e7260bca456d679)
---
 src/Main/logger.cxx | 29 +++++++++++++++++++++++++++--
 src/Main/logger.hxx |  4 ++--
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/Main/logger.cxx b/src/Main/logger.cxx
index 6c18162c3..00b6833ca 100644
--- a/src/Main/logger.cxx
+++ b/src/Main/logger.cxx
@@ -9,12 +9,17 @@
 
 #include "logger.hxx"
 
-#include <fstream>
+#include <ios>
 #include <string>
+#include <cstdlib>
 
 #include <simgear/debug/logstream.hxx>
+#include <simgear/io/iostreams/sgstream.hxx>
+#include <simgear/misc/sg_path.hxx>
 
 #include "fg_props.hxx"
+#include "globals.hxx"
+#include "util.hxx"
 
 using std::string;
 using std::endl;
@@ -59,6 +64,25 @@ FGLogger::init ()
         child->setStringValue("filename", filename.c_str());
     }
 
+    // Security: the path comes from the global Property Tree; it *must* be
+    //           validated before we overwrite the file.
+    const SGPath authorizedPath = fgValidatePath(SGPath::fromUtf8(filename),
+                                                 /* write */ true);
+
+    if (authorizedPath.isNull()) {
+      const string propertyPath = child->getChild("filename")
+                                       ->getPath(/* simplify */ true);
+      const string msg =
+        "The FGLogger logging system, via the '" + propertyPath + "' property, "
+        "was asked to write to '" + filename + "', however this path is not "
+        "authorized for writing anymore for security reasons. " +
+        "Please choose another location, for instance in the $FG_HOME/Export "
+        "folder (" + (globals->get_fg_home() / "Export").utf8Str() + ").";
+
+      SG_LOG(SG_GENERAL, SG_ALERT, msg);
+      exit(EXIT_FAILURE);
+    }
+
     string delimiter = child->getStringValue("delimiter");
     if (delimiter.empty()) {
         delimiter = ",";
@@ -68,7 +92,8 @@ FGLogger::init ()
     log.interval_ms = child->getLongValue("interval-ms");
     log.last_time_ms = globals->get_sim_time_sec() * 1000;
     log.delimiter = delimiter.c_str()[0];
-    log.output = new std::ofstream(filename.c_str());
+    // Security: use the return value of fgValidatePath()
+    log.output = new sg_ofstream(authorizedPath, std::ios_base::out);
     if (!log.output) {
       SG_LOG(SG_GENERAL, SG_ALERT, "Cannot write log to " << filename);
       continue;
diff --git a/src/Main/logger.hxx b/src/Main/logger.hxx
index 3d2146a83..de6209756 100644
--- a/src/Main/logger.hxx
+++ b/src/Main/logger.hxx
@@ -6,10 +6,10 @@
 #ifndef __LOGGER_HXX
 #define __LOGGER_HXX 1
 
-#include <iosfwd>
 #include <vector>
 
 #include <simgear/compiler.h>
+#include <simgear/io/iostreams/sgstream.hxx>
 #include <simgear/structure/subsystem_mgr.hxx>
 #include <simgear/props/props.hxx>
 
@@ -39,7 +39,7 @@ private:
     Log ();
     virtual ~Log ();
     std::vector<SGPropertyNode_ptr> nodes;
-    std::ostream * output;
+    sg_ofstream * output;
     long interval_ms;
     double last_time_ms;
     char delimiter;
-- 
2.13.5