bellet pushed to FlightGear (master). "Stop using property listener for fgValidatePath (..more)"
notifications at fedoraproject.org
notifications at fedoraproject.org
Sun Apr 19 14:25:12 UTC 2015
>From 5b152e7d5ef9382418344e91a12071219f57288a Mon Sep 17 00:00:00 2001
From: Fabrice Bellet <fabrice at bellet.info>
Date: Sun, 19 Apr 2015 11:52:14 +0200
Subject: Stop using property listener for fgValidatePath
- Normalize the allowed paths as well (fix Windows breakage)
diff --git a/0007-Stop-using-property-listener-for-fgValidatePath.patch b/0007-Stop-using-property-listener-for-fgValidatePath.patch
new file mode 100644
index 0000000..71754af
--- /dev/null
+++ b/0007-Stop-using-property-listener-for-fgValidatePath.patch
@@ -0,0 +1,230 @@
+From 6a30e7086ea2f1a060dd77dab6e7e8a15b43e82d Mon Sep 17 00:00:00 2001
+From: "Rebecca N. Palmer" <rebecca_palmer at zoho.com>
+Date: Fri, 13 Mar 2015 17:54:44 +0000
+Subject: [PATCH] Stop using property listener for fgValidatePath
+
+This was insecure: while removelistener() won't remove it, there are
+other ways to remove a listener from Nasal
+---
+ src/Main/util.cxx | 155 ++++++++++++++++++++++++++++++++++++++-------
+ src/Main/util.hxx | 7 +-
+ src/Scripting/NasalSys.cxx | 3 +
+ 3 files changed, 142 insertions(+), 23 deletions(-)
+
+diff --git a/src/Main/util.cxx b/src/Main/util.cxx
+index 5eed377..28ee477 100644
+--- a/src/Main/util.cxx
++++ b/src/Main/util.cxx
+@@ -33,6 +33,7 @@
+ #include <simgear/math/SGLimits.hxx>
+ #include <simgear/math/SGMisc.hxx>
+
++#include <GUI/MessageBox.hxx>
+ #include "fg_io.hxx"
+ #include "fg_props.hxx"
+ #include "globals.hxx"
+@@ -71,32 +72,142 @@ fgGetLowPass (double current, double target, double timeratio)
+ return current;
+ }
+
+-// Write out path to validation node and read it back in. A Nasal
+-// listener is supposed to replace the path with a validated version
+-// or an empty string otherwise.
+-const char *fgValidatePath (const char *str, bool write)
++static string_list read_allowed_paths;
++static string_list write_allowed_paths;
++
++// Allowed paths here are absolute, and may contain _one_ *,
++// which matches any string
++// FG_SCENERY is deliberately not allowed, as it would make
++// /sim/terrasync/scenery-dir a security hole
++void fgInitAllowedPaths()
+ {
+- SGPropertyNode_ptr r, w;
+- r = fgGetNode("/sim/paths/validate/read", true);
+- r->setAttribute(SGPropertyNode::READ, true);
+- r->setAttribute(SGPropertyNode::WRITE, true);
+-
+- w = fgGetNode("/sim/paths/validate/write", true);
+- w->setAttribute(SGPropertyNode::READ, true);
+- w->setAttribute(SGPropertyNode::WRITE, true);
+-
+- SGPropertyNode *prop = write ? w : r;
+- prop->setStringValue(str);
+- const char *result = prop->getStringValue();
+- return result[0] ? result : 0;
++ read_allowed_paths.clear();
++ write_allowed_paths.clear();
++ read_allowed_paths.push_back(globals->get_fg_root() + "/*");
++ read_allowed_paths.push_back(globals->get_fg_home() + "/*");
++ string_list const aircraft_paths = globals->get_aircraft_paths();
++ for( string_list::const_iterator it = aircraft_paths.begin();
++ it != aircraft_paths.end();
++ ++it )
++ {
++ read_allowed_paths.push_back(*it + "/*");
++ }
++
++ for( string_list::const_iterator it = read_allowed_paths.begin();
++ it != read_allowed_paths.end();
++ ++it )
++ { // if we get the initialization order wrong, better to have an
++ // obvious error than a can-read-everything security hole...
++ if (!(it->compare("/*"))){
++ flightgear::fatalMessageBox("Nasal initialization error",
++ "Empty string in FG_ROOT, FG_HOME or FG_AIRCRAFT",
++ "or fgInitAllowedPaths() called too early");
++ exit(-1);
++ }
++ }
++ write_allowed_paths.push_back("/tmp/*.xml");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/*.sav");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/*.log");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/cache/*");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/Export/*");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/state/*.xml");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/aircraft-data/*.xml");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/Wildfire/*.xml");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/runtime-jetways/*.xml");
++ write_allowed_paths.push_back(globals->get_fg_home() + "/Input/Joysticks/*.xml");
++
++ if(!fgValidatePath(globals->get_fg_home() + "/../no.log",true).empty() ||
++ !fgValidatePath(globals->get_fg_home() + "/no.lot",true).empty() ||
++ fgValidatePath((globals->get_fg_home() + "/nolog").c_str(),true) ||
++ !fgValidatePath(globals->get_fg_home() + "no.log",true).empty() ||
++ !fgValidatePath("..\\" + globals->get_fg_home() + "/no.log",false).empty() ||
++ fgValidatePath("/tmp/no.xml",false) ||
++ fgValidatePath(globals->get_fg_home() + "/./ff/../Export\\yes..gg",true).empty() ||
++ !fgValidatePath((globals->get_fg_home() + "/aircraft-data/yes..xml").c_str(),true) ||
++ fgValidatePath(globals->get_fg_root() + "/./\\yes.bmp",false).empty()) {
++ flightgear::fatalMessageBox("Nasal initialization error",
++ "fgInitAllowedPaths() does not work",
++ "");
++ exit(-1);
++ }
+ }
+
+-//------------------------------------------------------------------------------
+-std::string fgValidatePath(const std::string& path, bool write)
++// Normalize a path
++// Unlike SGPath::realpath, does not require that the file already exists,
++// but does require that it be below the starting point
++static std::string fgNormalizePath (const std::string& path)
+ {
+- const char* validate_path = fgValidatePath(path.c_str(), write);
+- return std::string(validate_path ? validate_path : "");
+-}
++ string_list path_parts;
++ char c;
++ std::string normed_path = "", this_part = "";
++
++ for (int pos = 0; ; pos++) {
++ c = path[pos];
++ if (c == '\\') { c = '/'; }
++ if ((c == '/') || (c == 0)) {
++ if ((this_part == "/..") || (this_part == "..")) {
++ if (path_parts.empty()) { return ""; }
++ path_parts.pop_back();
++ } else if ((this_part != "/.") && (this_part != "/")) {
++ path_parts.push_back(this_part);
++ }
++ this_part = "";
++ }
++ if (c == 0) { break; }
++ this_part = this_part + c;
++ }
++ for( string_list::const_iterator it = path_parts.begin();
++ it != path_parts.end();
++ ++it )
++ {
++ normed_path.append(*it);
++ }
++ return normed_path;
++ }
++
+
++// Check whether Nasal is allowed to access a path
++std::string fgValidatePath (const std::string& path, bool write)
++{
++ const string_list& allowed_paths(write ? write_allowed_paths : read_allowed_paths);
++ int star_pos;
++
++ // Normalize the path (prevents ../../.. trickery)
++ std::string normed_path = fgNormalizePath(path);
++
++ // Check against each allowed pattern
++ for( string_list::const_iterator it = allowed_paths.begin();
++ it != allowed_paths.end();
++ ++it )
++ {
++ star_pos = it->find('*');
++ if (star_pos == std::string::npos) {
++ if (!(it->compare(normed_path))) {
++ return normed_path;
++ }
++ } else {
++ if ((it->size()-1 <= normed_path.size()) /* long enough to be a potential match */
++ && !(it->substr(0,star_pos)
++ .compare(normed_path.substr(0,star_pos))) /* before-star parts match */
++ && !(it->substr(star_pos+1,it->size()-star_pos-1)
++ .compare(normed_path.substr(star_pos+1+normed_path.size()-it->size(),
++ it->size()-star_pos-1))) /* after-star parts match */) {
++ return normed_path;
++ }
++ }
++ }
++ // no match found
++ return "";
++}
++// s.c_str() becomes invalid when s is destroyed, so need a static s
++std::string validate_path_temp;
++const char* fgValidatePath(const char* path, bool write)
++{
++ validate_path_temp = fgValidatePath(std::string(path), write);
++ if(validate_path_temp.empty()){
++ return 0;
++ }
++ return validate_path_temp.c_str();
++}
+ // end of util.cxx
+
+diff --git a/src/Main/util.hxx b/src/Main/util.hxx
+index 5172ca2..37f401d 100644
+--- a/src/Main/util.hxx
++++ b/src/Main/util.hxx
+@@ -36,7 +36,7 @@
+ double fgGetLowPass (double current, double target, double timeratio);
+
+ /**
+- * Validation listener interface for io.nas, used by fgcommands.
++ * File access control, used by Nasal and fgcommands.
+ * @param path Path to be validated
+ * @param write True for write operations and false for read operations.
+ * @return The validated path on success or 0 if access denied.
+@@ -44,4 +44,9 @@ double fgGetLowPass (double current, double target, double timeratio);
+ const char *fgValidatePath (const char *path, bool write);
+ std::string fgValidatePath(const std::string& path, bool write);
+
++/**
++ * Set allowed paths for fgValidatePath
++ */
++void fgInitAllowedPaths();
++
+ #endif // __UTIL_HXX
+diff --git a/src/Scripting/NasalSys.cxx b/src/Scripting/NasalSys.cxx
+index 0e9f656..421965e 100644
+--- a/src/Scripting/NasalSys.cxx
++++ b/src/Scripting/NasalSys.cxx
+@@ -835,6 +835,9 @@ void FGNasalSys::init()
+ .member("singleShot", &TimerObj::isSingleShot, &TimerObj::setSingleShot)
+ .member("isRunning", &TimerObj::isRunning);
+
++ // Set allowed paths for Nasal I/O
++ fgInitAllowedPaths();
++
+ // Now load the various source files in the Nasal directory
+ simgear::Dir nasalDir(SGPath(globals->get_fg_root(), "Nasal"));
+ loadScriptDirectory(nasalDir);
+--
+2.1.0
+
diff --git a/0008-Normalize-the-allowed-paths-as-well-fix-Windows-brea.patch b/0008-Normalize-the-allowed-paths-as-well-fix-Windows-brea.patch
new file mode 100644
index 0000000..d01cacc
--- /dev/null
+++ b/0008-Normalize-the-allowed-paths-as-well-fix-Windows-brea.patch
@@ -0,0 +1,143 @@
+From 2ee0cdb6d994b0effccbdb9f27f9d3ae6f26aeb1 Mon Sep 17 00:00:00 2001
+From: "Rebecca N. Palmer" <rebecca_palmer at zoho.com>
+Date: Fri, 13 Mar 2015 22:39:22 +0000
+Subject: [PATCH] Normalize the allowed paths as well (fix Windows breakage)
+
+(cherry picked from commit 196c6672667ae3043e739595ccd71dddb29e9a98)
+---
+ src/Main/util.cxx | 93 ++++++++++++++++++++++++++++---------------------------
+ 1 file changed, 47 insertions(+), 46 deletions(-)
+
+diff --git a/src/Main/util.cxx b/src/Main/util.cxx
+index 28ee477..c15a5c0 100644
+--- a/src/Main/util.cxx
++++ b/src/Main/util.cxx
+@@ -72,6 +72,39 @@ fgGetLowPass (double current, double target, double timeratio)
+ return current;
+ }
+
++// Normalize a path
++// Unlike SGPath::realpath, does not require that the file already exists,
++// but does require that it be below the starting point
++static std::string fgNormalizePath (const std::string& path)
++{
++ string_list path_parts;
++ char c;
++ std::string normed_path = "", this_part = "";
++
++ for (int pos = 0; ; pos++) {
++ c = path[pos];
++ if (c == '\\') { c = '/'; }
++ if ((c == '/') || (c == 0)) {
++ if ((this_part == "/..") || (this_part == "..")) {
++ if (path_parts.empty()) { return ""; }
++ path_parts.pop_back();
++ } else if ((this_part != "/.") && (this_part != "/")) {
++ path_parts.push_back(this_part);
++ }
++ this_part = "";
++ }
++ if (c == 0) { break; }
++ this_part = this_part + c;
++ }
++ for( string_list::const_iterator it = path_parts.begin();
++ it != path_parts.end();
++ ++it )
++ {
++ normed_path.append(*it);
++ }
++ return normed_path;
++ }
++
+ static string_list read_allowed_paths;
+ static string_list write_allowed_paths;
+
+@@ -83,14 +116,16 @@ void fgInitAllowedPaths()
+ {
+ read_allowed_paths.clear();
+ write_allowed_paths.clear();
+- read_allowed_paths.push_back(globals->get_fg_root() + "/*");
+- read_allowed_paths.push_back(globals->get_fg_home() + "/*");
++ std::string fg_root = fgNormalizePath(globals->get_fg_root());
++ std::string fg_home = fgNormalizePath(globals->get_fg_home());
++ read_allowed_paths.push_back(fg_root + "/*");
++ read_allowed_paths.push_back(fg_home + "/*");
+ string_list const aircraft_paths = globals->get_aircraft_paths();
+ for( string_list::const_iterator it = aircraft_paths.begin();
+ it != aircraft_paths.end();
+ ++it )
+ {
+- read_allowed_paths.push_back(*it + "/*");
++ read_allowed_paths.push_back(fgNormalizePath(*it) + "/*");
+ }
+
+ for( string_list::const_iterator it = read_allowed_paths.begin();
+@@ -106,15 +141,15 @@ void fgInitAllowedPaths()
+ }
+ }
+ write_allowed_paths.push_back("/tmp/*.xml");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/*.sav");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/*.log");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/cache/*");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/Export/*");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/state/*.xml");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/aircraft-data/*.xml");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/Wildfire/*.xml");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/runtime-jetways/*.xml");
+- write_allowed_paths.push_back(globals->get_fg_home() + "/Input/Joysticks/*.xml");
++ write_allowed_paths.push_back(fg_home + "/*.sav");
++ write_allowed_paths.push_back(fg_home + "/*.log");
++ write_allowed_paths.push_back(fg_home + "/cache/*");
++ write_allowed_paths.push_back(fg_home + "/Export/*");
++ write_allowed_paths.push_back(fg_home + "/state/*.xml");
++ write_allowed_paths.push_back(fg_home + "/aircraft-data/*.xml");
++ write_allowed_paths.push_back(fg_home + "/Wildfire/*.xml");
++ write_allowed_paths.push_back(fg_home + "/runtime-jetways/*.xml");
++ write_allowed_paths.push_back(fg_home + "/Input/Joysticks/*.xml");
+
+ if(!fgValidatePath(globals->get_fg_home() + "/../no.log",true).empty() ||
+ !fgValidatePath(globals->get_fg_home() + "/no.lot",true).empty() ||
+@@ -132,40 +167,6 @@ void fgInitAllowedPaths()
+ }
+ }
+
+-// Normalize a path
+-// Unlike SGPath::realpath, does not require that the file already exists,
+-// but does require that it be below the starting point
+-static std::string fgNormalizePath (const std::string& path)
+-{
+- string_list path_parts;
+- char c;
+- std::string normed_path = "", this_part = "";
+-
+- for (int pos = 0; ; pos++) {
+- c = path[pos];
+- if (c == '\\') { c = '/'; }
+- if ((c == '/') || (c == 0)) {
+- if ((this_part == "/..") || (this_part == "..")) {
+- if (path_parts.empty()) { return ""; }
+- path_parts.pop_back();
+- } else if ((this_part != "/.") && (this_part != "/")) {
+- path_parts.push_back(this_part);
+- }
+- this_part = "";
+- }
+- if (c == 0) { break; }
+- this_part = this_part + c;
+- }
+- for( string_list::const_iterator it = path_parts.begin();
+- it != path_parts.end();
+- ++it )
+- {
+- normed_path.append(*it);
+- }
+- return normed_path;
+- }
+-
+-
+ // Check whether Nasal is allowed to access a path
+ std::string fgValidatePath (const std::string& path, bool write)
+ {
+--
+2.1.0
+
diff --git a/FlightGear.spec b/FlightGear.spec
index 96db314..d8444fe 100644
--- a/FlightGear.spec
+++ b/FlightGear.spec
@@ -1,7 +1,7 @@
Name: FlightGear
Summary: The FlightGear Flight Simulator
Version: 3.4.0
-Release: 3%{?dist}
+Release: 4%{?dist}
License: GPLv2+
Source0: http://mirrors.ibiblio.org/flightgear/ftp/Source/flightgear-%{version}.tar.bz2
Source1: %{name}.desktop
@@ -18,6 +18,8 @@ Patch2: 0002-make-ShivaVG-and-FGAdminUI-static-libraries.patch
Patch3: 0003-Use-system-iaxclient-instead-of-bundled-one.patch
Patch5: 0005-explicitely-link-with-libX11.patch
Patch6: 0006-make-fglauncher-a-static-library.patch
+Patch7: 0007-Stop-using-property-listener-for-fgValidatePath.patch
+Patch8: 0008-Normalize-the-allowed-paths-as-well-fix-Windows-brea.patch
URL: http://www.flightgear.org/
BuildRequires: openal-soft-devel, SimGear-devel >= %{version}
@@ -42,6 +44,8 @@ expanded and improved upon by anyone interested in contributing
%patch3 -p1
%patch5 -p1
%patch6 -p1
+%patch7 -p1
+%patch8 -p1
rm -rf 3rdparty/iaxclient
# make rpmlint happy
@@ -171,6 +175,10 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
%{_datadir}/icons/hicolor/*/apps/*
%changelog
+* Sun Apr 19 2015 Fabrice Bellet <fabrice at bellet.info> - 3.4.0-4
+- Stop using property listener for fgValidatePath
+- Normalize the allowed paths as well (fix Windows breakage)
+
* Fri Apr 17 2015 Ralf Corsépius <corsepiu at fedoraproject.org> - 3.4.0-3
- Rebuild for Gcc-5.0.1 (FTBFS, RHBZ#1212707).
- Modernize spec.
--
cgit v0.10.2
http://pkgs.fedoraproject.org/cgit/FlightGear.git/commit/?h=master&id=5b152e7d5ef9382418344e91a12071219f57288a
More information about the scm-commits
mailing list