1
0
Fork 0

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
This commit is contained in:
Rebecca N. Palmer 2015-03-13 17:54:44 +00:00
parent a6937e25ad
commit 6a30e7086e
3 changed files with 141 additions and 22 deletions

View file

@ -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);
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 + "/*");
}
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;
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

View file

@ -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

View file

@ -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);