diff --git a/src/Add-ons/Addon.cxx b/src/Add-ons/Addon.cxx index 4342f1ae4..1458285e5 100644 --- a/src/Add-ons/Addon.cxx +++ b/src/Add-ons/Addon.cxx @@ -35,7 +35,6 @@ #include
#include
-#include
#include #include "addon_fwd.hxx" @@ -200,12 +199,12 @@ SGPath Addon::createStorageDir() const throw errors::unable_to_create_addon_storage_dir(msg); } } else { - SGPath authorizedPath = fgValidatePath(_storagePath, true /* write */); - + const SGPath authorizedPath = SGPath(_storagePath).validate(/* write */ + true); if (authorizedPath.isNull()) { string msg = "Unable to create add-on storage directory because of the FlightGear " - "security policy (refused by fgValidatePath()): '" + + "security policy (refused by SGPath::validate()): '" + _storagePath.utf8Str() + "'"; SG_LOG(SG_GENERAL, SG_POPUP, msg); throw errors::unable_to_create_addon_storage_dir(msg); @@ -218,8 +217,8 @@ SGPath Addon::createStorageDir() const // _storagePath instead of authorizedPath for consistency with the // getStoragePath() method (_storagePath and authorizedPath could be // different in case the former contains symlink components). Further - // sensitive operations beneath _storagePath must use fgValidatePath() again - // every time, of course (otherwise attackers could use symlinks in + // sensitive operations beneath _storagePath must use SGPath::validate() + // again every time, of course (otherwise, attackers could use symlinks in // _storagePath to bypass the security policy). return _storagePath; } diff --git a/src/Add-ons/AddonManager.cxx b/src/Add-ons/AddonManager.cxx index b5aea6c02..141094508 100644 --- a/src/Add-ons/AddonManager.cxx +++ b/src/Add-ons/AddonManager.cxx @@ -178,8 +178,8 @@ string AddonManager::registerAddon(const SGPath& addonPath) { // Use realpath() as in FGGlobals::append_aircraft_path(), otherwise - // fgValidatePath() will deny access to resources under the add-on path if - // one of its components is a symlink. + // SGPath::validate() will deny access to resources under the add-on path + // if one of its components is a symlink. const SGPath addonRealPath = addonPath.realpath(); const string addonId = registerAddonMetadata(addonRealPath); loadConfigFileIfExists(addonRealPath / "addon-config.xml"); diff --git a/src/Add-ons/AddonResourceProvider.cxx b/src/Add-ons/AddonResourceProvider.cxx index d7d440a03..7965c1363 100644 --- a/src/Add-ons/AddonResourceProvider.cxx +++ b/src/Add-ons/AddonResourceProvider.cxx @@ -23,8 +23,6 @@ #include #include -#include
- #include "AddonManager.hxx" #include "AddonResourceProvider.hxx" @@ -72,7 +70,7 @@ ResourceProvider::resolve(const string& resource, SGPath& context) const return SGPath(); } - return fgValidatePath(candidate, /* write */ false); + return SGPath(candidate).validate(/* write */ false); } } // of namespace addons diff --git a/src/Autopilot/route_mgr.cxx b/src/Autopilot/route_mgr.cxx index 1b057758c..888d3fc59 100644 --- a/src/Autopilot/route_mgr.cxx +++ b/src/Autopilot/route_mgr.cxx @@ -49,7 +49,6 @@ #include "Airports/runways.hxx" #include #include -#include
// fgValidatePath() #include #define RM "/autopilot/route-manager/" @@ -69,7 +68,7 @@ static bool commandSaveFlightPlan(const SGPropertyNode* arg, SGPropertyNode *) { FGRouteMgr* self = (FGRouteMgr*) globals->get_subsystem("route-manager"); SGPath path = SGPath::fromUtf8(arg->getStringValue("path")); - SGPath authorizedPath = fgValidatePath(path, true /* write */); + const SGPath authorizedPath = SGPath(path).validate(true /* write */); if (!authorizedPath.isNull()) { return self->saveRoute(authorizedPath); @@ -768,7 +767,7 @@ void FGRouteMgr::InputListener::valueChanged(SGPropertyNode *prop) mgr->loadRoute(path); } else if (input == "@SAVE") { SGPath path = SGPath::fromUtf8(mgr->_pathNode->getStringValue()); - SGPath authorizedPath = fgValidatePath(path, true /* write */); + const SGPath authorizedPath = SGPath(path).validate(true /* write */); if (!authorizedPath.isNull()) { mgr->saveRoute(authorizedPath); diff --git a/src/Canvas/FGCanvasSystemAdapter.cxx b/src/Canvas/FGCanvasSystemAdapter.cxx index a0b9b6651..67f9afef8 100644 --- a/src/Canvas/FGCanvasSystemAdapter.cxx +++ b/src/Canvas/FGCanvasSystemAdapter.cxx @@ -19,7 +19,6 @@ #include "FGCanvasSystemAdapter.hxx" #include
-#include
#include #include @@ -88,7 +87,7 @@ namespace canvas if( p.isAbsolute() ) { - SGPath valid_path = fgValidatePath(p, false); + const SGPath valid_path = SGPath(p).validate(false); if( !valid_path.isNull() ) return osgDB::readRefImageFile(valid_path.utf8Str(), localReaderWriterOptions); diff --git a/src/Main/fg_commands.cxx b/src/Main/fg_commands.cxx index 30b9ed30c..e763c368a 100644 --- a/src/Main/fg_commands.cxx +++ b/src/Main/fg_commands.cxx @@ -44,7 +44,6 @@ #include "fg_props.hxx" #include "globals.hxx" #include "logger.hxx" -#include "util.hxx" #include "main.hxx" #include "positioninit.hxx" @@ -258,7 +257,7 @@ do_load (const SGPropertyNode * arg, SGPropertyNode * root) if (file.extension() != "sav") file.concat(".sav"); - SGPath validated_path = fgValidatePath(file, false); + const SGPath validated_path = SGPath(file).validate(false); if (validated_path.isNull()) { SG_LOG(SG_IO, SG_ALERT, "load: reading '" << file << "' denied " "(unauthorized access)"); @@ -291,7 +290,7 @@ do_save (const SGPropertyNode * arg, SGPropertyNode * root) if (file.extension() != "sav") file.concat(".sav"); - SGPath validated_path = fgValidatePath(file, true); + const SGPath validated_path = SGPath(file).validate(true); if (validated_path.isNull()) { SG_LOG(SG_IO, SG_ALERT, "save: writing '" << file << "' denied " "(unauthorized access)"); @@ -884,7 +883,7 @@ do_load_xml_to_proptree(const SGPropertyNode * arg, SGPropertyNode * root) return false; } - SGPath validated_path = fgValidatePath(file, false); + const SGPath validated_path = SGPath(file).validate(false); if (validated_path.isNull()) { SG_LOG(SG_IO, quiet ? SG_DEV_WARN : SG_ALERT, "loadxml: reading '" << file << "' denied " "(unauthorized directory - authorization no longer follows symlinks; to authorize reading additional directories, pass them to --allow-nasal-read)"); @@ -971,7 +970,7 @@ do_save_xml_from_proptree(const SGPropertyNode * arg, SGPropertyNode * root) if (file.extension() != "xml") file.concat(".xml"); - SGPath validated_path = fgValidatePath(file, true); + const SGPath validated_path = SGPath(file).validate(true); if (validated_path.isNull()) { SG_LOG(SG_IO, SG_ALERT, "savexml: writing to '" << file << "' denied " "(unauthorized directory - authorization no longer follows symlinks)"); diff --git a/src/Main/fg_init.cxx b/src/Main/fg_init.cxx index 63b3cf74f..0b57a2676 100755 --- a/src/Main/fg_init.cxx +++ b/src/Main/fg_init.cxx @@ -696,7 +696,7 @@ int fgInitConfig ( int argc, char **argv, bool reinit ) createBaseStorageDirForAddons(exportDir.path()); // Set /sim/fg-home. Use FG_HOME if necessary. - // deliberately not a tied property, for fgValidatePath security + // deliberately not a tied property, for SGPath::validate() security // write-protect to avoid accidents SGPropertyNode *home = fgGetNode("/sim", true); home->removeChild("fg-home", 0); @@ -766,7 +766,7 @@ int fgInitConfig ( int argc, char **argv, bool reinit ) static void initAircraftDirsNasalSecurity() { - // deliberately not a tied property, for fgValidatePath security + // deliberately not a tied property, for SGPath::validate() security // write-protect to avoid accidents SGPropertyNode* sim = fgGetNode("/sim", true); sim->removeChildren("fg-aircraft"); diff --git a/src/Main/globals.cxx b/src/Main/globals.cxx index 667904c2f..d95acd6ef 100644 --- a/src/Main/globals.cxx +++ b/src/Main/globals.cxx @@ -279,7 +279,7 @@ void FGGlobals::set_fg_root (const SGPath &root) { << fg_root << "'\n***\n***"); } - // deliberately not a tied property, for fgValidatePath security + // deliberately not a tied property, for SGPath::validate() security // write-protect to avoid accidents SGPropertyNode *n = fgGetNode("/sim", true); n->removeChild("fg-root", 0); @@ -467,7 +467,7 @@ void FGGlobals::set_terrasync_dir(const SGPath &path) } SGPath abspath(path.realpath()); terrasync_dir = abspath; - // deliberately not a tied property, for fgValidatePath security + // deliberately not a tied property, for SGPath::validate() security // write-protect to avoid accidents SGPropertyNode *n = fgGetNode("/sim/terrasync/scenery-dir", true); n->setAttribute(SGPropertyNode::WRITE, true); diff --git a/src/Main/logger.cxx b/src/Main/logger.cxx index f1d9d15fc..ddb0efbbe 100644 --- a/src/Main/logger.cxx +++ b/src/Main/logger.cxx @@ -19,7 +19,6 @@ #include "fg_props.hxx" #include "globals.hxx" -#include "util.hxx" using std::string; using std::endl; @@ -53,7 +52,7 @@ FGLogger::init () // 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), + const SGPath authorizedPath = SGPath::fromUtf8(filename).validate( /* write */ true); if (authorizedPath.isNull()) { @@ -79,7 +78,7 @@ 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]; - // Security: use the return value of fgValidatePath() + // Security: use the return value of SGPath::validate() log.output.reset(new sg_ofstream(authorizedPath, std::ios_base::out)); if ( !(*log.output) ) { SG_LOG(SG_GENERAL, SG_ALERT, "Cannot write log to " << filename); diff --git a/src/Main/main.cxx b/src/Main/main.cxx index b94f1133b..2cbf2f4e5 100755 --- a/src/Main/main.cxx +++ b/src/Main/main.cxx @@ -667,8 +667,6 @@ int fgMainInit( int argc, char **argv ) string_list *col = new string_list; globals->set_channel_options_list( col ); - fgValidatePath(globals->get_fg_home(), false); // initialize static variables - if (showLauncher) { // to minimise strange interactions when launcher and config files // set overlaping options, we disable the default files. Users can diff --git a/src/Main/options.cxx b/src/Main/options.cxx index 8e5de40cb..f63d2a874 100644 --- a/src/Main/options.cxx +++ b/src/Main/options.cxx @@ -74,7 +74,6 @@ #include "fg_os.hxx" #include "fg_props.hxx" #include "options.hxx" -#include "util.hxx" #include "main.hxx" #include "locale.hxx" #include @@ -2458,7 +2457,7 @@ OptionResult Options::initAircraft() globals->append_read_allowed_paths(realAircraftPath); // Set this now, so it's available in FindAndCacheAircraft. Use realpath() - // as in FGGlobals::append_aircraft_path(), otherwise fgValidatePath() + // as in FGGlobals::append_aircraft_path(), otherwise SGPath::validate() // will deny access to resources under this path if one of its components // is a symlink (which is not a problem, since it was given as is by the // user---this is very different from a symlink *under* the aircraft dir diff --git a/src/Main/util.cxx b/src/Main/util.cxx index f20bae1f8..38fec73fc 100644 --- a/src/Main/util.cxx +++ b/src/Main/util.cxx @@ -72,9 +72,6 @@ fgGetLowPass (double current, double target, double timeratio) return current; } -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 @@ -82,54 +79,54 @@ static string_list write_allowed_paths; void fgInitAllowedPaths() { if(SGPath("ygjmyfvhhnvdoesnotexist").realpath().utf8Str() == "ygjmyfvhhnvdoesnotexist"){ - // Forbid using this version of fgValidatePath() with older - // (not normalizing non-existent files) versions of realpath(), - // as that would be a security hole + // Abort in case this is used with older versions of realpath() + // that don't normalize non-existent files, as that would be a security + // hole. flightgear::fatalMessageBoxThenExit( "Nasal initialization error", "Version mismatch - please update simgear"); } - read_allowed_paths.clear(); - write_allowed_paths.clear(); + SGPath::clearListOfAllowedPaths(false); // clear list of read-allowed paths + SGPath::clearListOfAllowedPaths(true); // clear list of write-allowed paths PathList read_paths = globals->get_extra_read_allowed_paths(); read_paths.push_back(globals->get_fg_root()); read_paths.push_back(globals->get_fg_home()); - for( PathList::const_iterator it = read_paths.begin(); it != read_paths.end(); ++it ) - { - // if we get the initialization order wrong, better to have an + for (const auto& path: read_paths) { + // If we get the initialization order wrong, better to have an // obvious error than a can-read-everything security hole... - if (it->isNull()) { - flightgear::fatalMessageBoxThenExit( + if (path.isNull()) { + flightgear::fatalMessageBoxThenExit( "Nasal initialization error", "Empty string in FG_ROOT, FG_HOME, FG_AIRCRAFT, FG_SCENERY or " "--allow-nasal-read, or fgInitAllowedPaths() called too early"); - } - read_allowed_paths.push_back(it->realpath().utf8Str() + "/*"); - read_allowed_paths.push_back(it->realpath().utf8Str()); - } - - std::string fg_home = globals->get_fg_home().realpath().utf8Str(); - 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"); - + } + SGPath::addAllowedPathPattern(path.realpath().utf8Str() + "/*", + false /* write */); + SGPath::addAllowedPathPattern(path.realpath().utf8Str(), false); + } + + const std::string fg_home = globals->get_fg_home().realpath().utf8Str(); + SGPath::addAllowedPathPattern(fg_home + "/*.sav", true /* write */); + SGPath::addAllowedPathPattern(fg_home + "/*.log", true); + SGPath::addAllowedPathPattern(fg_home + "/cache/*", true); + SGPath::addAllowedPathPattern(fg_home + "/Export/*", true); + SGPath::addAllowedPathPattern(fg_home + "/state/*.xml", true); + SGPath::addAllowedPathPattern(fg_home + "/aircraft-data/*.xml", true); + SGPath::addAllowedPathPattern(fg_home + "/Wildfire/*.xml", true); + SGPath::addAllowedPathPattern(fg_home + "/runtime-jetways/*.xml", true); + SGPath::addAllowedPathPattern(fg_home + "/Input/Joysticks/*.xml", true); + // Check that it works - std::string homePath = globals->get_fg_home().utf8Str(); - if(!fgValidatePath(homePath + "/../no.log",true).isNull() || - !fgValidatePath(homePath + "/no.logt",true).isNull() || - !fgValidatePath(homePath + "/nolog",true).isNull() || - !fgValidatePath(homePath + "no.log",true).isNull() || - !fgValidatePath(homePath + "\\..\\no.log",false).isNull() || - fgValidatePath(homePath + "/aircraft-data/yes..xml",true).isNull() || - fgValidatePath(homePath + "/.\\yes.bmp",false).isNull()) { + const std::string homePath = globals->get_fg_home().utf8Str(); + if (! SGPath(homePath + "/../no.log").validate(true).isNull() || + ! SGPath(homePath + "/no.logt").validate(true).isNull() || + ! SGPath(homePath + "/nolog").validate(true).isNull() || + ! SGPath(homePath + "no.log").validate(true).isNull() || + ! SGPath(homePath + "\\..\\no.log").validate(false).isNull() || + SGPath(homePath + "/aircraft-data/yes..xml").validate(true).isNull() || + SGPath(homePath + "/.\\yes.bmp").validate(false).isNull()) { flightgear::fatalMessageBoxThenExit( "Nasal initialization error", "The FG_HOME directory must not be inside any of the FG_ROOT, " @@ -139,46 +136,6 @@ void fgInitAllowedPaths() } } -/** - * Check whether Nasal is allowed to access a path - * Warning: because this always (not just on Windows) treats both \ and / - * as path separators, and accepts relative paths (check-to-use race if - * the current directory changes), - * always use the returned path not the original one - */ -SGPath fgValidatePath (const SGPath& path, bool write) -{ - // Normalize the path (prevents ../../.. or symlink trickery) - std::string normed_path = path.realpath().utf8Str(); - - const string_list& allowed_paths(write ? write_allowed_paths : read_allowed_paths); - size_t star_pos; - - // 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 SGPath::fromUtf8(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 SGPath::fromUtf8(normed_path); - } - } - } - // no match found - return SGPath(); -} - std::string generateAuthorsText(SGPropertyNode* authors) { std::string result; diff --git a/src/Main/util.hxx b/src/Main/util.hxx index c6816ef7d..0265a2632 100644 --- a/src/Main/util.hxx +++ b/src/Main/util.hxx @@ -37,20 +37,7 @@ double fgGetLowPass (double current, double target, double timeratio); /** - * 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 empty if access denied. - * - * Warning: because this always (not just on Windows) treats both \ and / - * as path separators, and accepts relative paths (check-to-use race if - * the current directory changes), - * always use the returned path not the original one - */ -SGPath fgValidatePath(const SGPath& path, bool write); - -/** - * Set allowed paths for fgValidatePath + * Set the read and write lists of allowed paths patterns for SGPath::validate() */ void fgInitAllowedPaths(); diff --git a/src/Scripting/NasalFlightPlan.cxx b/src/Scripting/NasalFlightPlan.cxx index 12b1ba0e6..d8935bac7 100644 --- a/src/Scripting/NasalFlightPlan.cxx +++ b/src/Scripting/NasalFlightPlan.cxx @@ -35,7 +35,6 @@ #include #include
#include
-#include
#include #include #include @@ -1930,8 +1929,8 @@ static naRef f_flightplan_save(naContext c, naRef me, int argc, naRef* args) naRuntimeError(c, "flightplan.save, no file path argument"); } - SGPath raw_path(naStr_data(args[0])); - SGPath validated_path = fgValidatePath(raw_path, true); + const SGPath raw_path(naStr_data(args[0])); + const SGPath validated_path = SGPath(raw_path).validate(true); if (validated_path.isNull()) { naRuntimeError(c, "flightplan.save, writing to path is not permitted"); } diff --git a/src/Scripting/NasalHTTP.cxx b/src/Scripting/NasalHTTP.cxx index 3c88bb8a9..4a754dc9f 100644 --- a/src/Scripting/NasalHTTP.cxx +++ b/src/Scripting/NasalHTTP.cxx @@ -22,7 +22,6 @@ #include "NasalHTTP.hxx" #include
-#include
#include #include @@ -55,7 +54,7 @@ static naRef f_http_save(const nasal::CallContext& ctx) // Check for write access to target file const std::string filename = ctx.requireArg(1); - const SGPath validated_path = fgValidatePath(filename, true); + const SGPath validated_path = SGPath(filename).validate(true); if( validated_path.isNull() ) ctx.runtimeError("Access denied: can not write to %s", filename.c_str()); diff --git a/src/Scripting/NasalSGPath.cxx b/src/Scripting/NasalSGPath.cxx index c7f8adad5..92abbbe87 100644 --- a/src/Scripting/NasalSGPath.cxx +++ b/src/Scripting/NasalSGPath.cxx @@ -22,7 +22,6 @@ #include "NasalSGPath.hxx" #include
-#include
#include #include @@ -42,8 +41,8 @@ SGPath::Permissions checkIORules(const SGPath& path) "realpath() to make a path absolute)"); } - perm.read = path.isAbsolute() && !fgValidatePath(path, false).isNull(); - perm.write = path.isAbsolute() && !fgValidatePath(path, true ).isNull(); + perm.read = path.isAbsolute() && !SGPath(path).validate(false).isNull(); + perm.write = path.isAbsolute() && !SGPath(path).validate(true).isNull(); return perm; } diff --git a/src/Scripting/NasalSys.cxx b/src/Scripting/NasalSys.cxx index 05ed1b1b7..1bc985d06 100644 --- a/src/Scripting/NasalSys.cxx +++ b/src/Scripting/NasalSys.cxx @@ -70,7 +70,6 @@ #include "NasalUnitTesting.hxx" #include
-#include
#include
#include
@@ -738,7 +737,7 @@ static naRef f_directory(naContext c, naRef me, int argc, naRef* args) if(argc != 1 || !naIsString(args[0])) naRuntimeError(c, "bad arguments to directory()"); - SGPath dirname = fgValidatePath(SGPath::fromUtf8(naStr_data(args[0])), false); + SGPath dirname = SGPath::fromUtf8(naStr_data(args[0])).validate(false); if(dirname.isNull()) { SG_LOG(SG_NASAL, SG_ALERT, "directory(): listing '" << naStr_data(args[0]) << "' denied (unauthorized directory - authorization" @@ -853,7 +852,7 @@ static naRef f_open(naContext c, naRef me, int argc, naRef* args) naRef mode = argc > 1 ? naStringValue(c, args[1]) : naNil(); if(!naStr_data(file)) naRuntimeError(c, "bad argument to open()"); const char* modestr = naStr_data(mode) ? naStr_data(mode) : "rb"; - SGPath filename = fgValidatePath(SGPath::fromUtf8(naStr_data(file)), + const SGPath filename = SGPath::fromUtf8(naStr_data(file)).validate( strcmp(modestr, "rb") && strcmp(modestr, "r")); if(filename.isNull()) { SG_LOG(SG_NASAL, SG_ALERT, "open(): reading/writing '" << @@ -897,7 +896,7 @@ static naRef f_custom_stat(naContext ctx, naRef me, int argc, naRef* args) return naNil(); } - const SGPath filename = fgValidatePath(path, false ); + const SGPath filename = SGPath(path).validate(false); if (filename.isNull()) { SG_LOG(SG_NASAL, SG_ALERT, "stat(): reading '" << naStr_data(pathArg) << "' denied (unauthorized directory - authorization" @@ -948,7 +947,7 @@ static naRef f_parsexml(naContext c, naRef me, int argc, naRef* args) if(!(naIsNil(args[i]) || naIsFunc(args[i]))) naRuntimeError(c, "parsexml(): callback argument not a function"); - SGPath file = fgValidatePath(SGPath::fromUtf8(naStr_data(args[0])), false); + const SGPath file = SGPath::fromUtf8(naStr_data(args[0])).validate(false); if(file.isNull()) { SG_LOG(SG_NASAL, SG_ALERT, "parsexml(): reading '" << naStr_data(args[0]) << "' denied (unauthorized directory - authorization"