From 22779ee2c4c8d99485b95aebf4c8e1a2ca1c6bc5 Mon Sep 17 00:00:00 2001
From: Florent Rougon <f.rougon@frougon.net>
Date: Wed, 24 Aug 2022 01:01:05 +0200
Subject: [PATCH] ResourceManager::findPath(): validate; accept absolute,
 read-allowed paths

- Perform the SGPath::validate() test for read access before returning a
  path obtained with a non-null second argument ("aContext").

- If SGPath(aContext, aResource) is an absolute path for which read
  access is authorized by SGPath::validate(), return it. This restores
  the possibility of using the 'play-audio-sample' FGCommand with an
  absolute, read-allowed path (was lost in FG commit 8853fded2953959).

- Because a fair number of the existing uses of
  ResourceManager::findPath() are not quite correct IMO, we execute the
  final part where all providers are tried in turn even if
  SGPath(aContext, aResource) is an absolute path (otherwise, the sim
  wouldn't start because it couldn't load materials.xml---see [1]).

- The SG_LOG call will spot a few errors in calling code/data, such as
  access tried for
  '/Aircraft/Generic/flightrecorder/generic-piston-propeller-4.xml' and
  '/Textures/Sky/cl_cumulus2.png'; since the function does not return at
  this point, these incorrect absolute paths which should be relative
  will still be given a chance with the BasePathProvider that has its
  base path set to $FG_ROOT; thus, they will be found as before this
  commit despite the new "access refused" warning (but please fix
  them!).

This commit requires FlightGear commit e7594f46876fc6b0b.

[1] https://sourceforge.net/p/flightgear/mailman/message/37697516/
---
 simgear/misc/ResourceManager.cxx | 40 +++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/simgear/misc/ResourceManager.cxx b/simgear/misc/ResourceManager.cxx
index ec538f1b..dec847f5 100644
--- a/simgear/misc/ResourceManager.cxx
+++ b/simgear/misc/ResourceManager.cxx
@@ -127,20 +127,42 @@ void ResourceManager::removeProvider(ResourceProvider* aProvider)
 
 SGPath ResourceManager::findPath(const std::string& aResource, SGPath aContext)
 {
+    const SGPath completePath(aContext, aResource);
+
     if (!aContext.isNull()) {
-        SGPath r(aContext, aResource);
-        if (r.exists()) {
+        const SGPath r = completePath.validate(false); // read access
+        if (!r.isNull() && r.exists()) {
             return r;
         }
     }
-    
-    for (auto provider : _providers) {
-      SGPath path = provider->resolve(aResource, aContext);
-      if (!path.isNull()) {
-        return path;
-      }
+
+    // If the path is absolute and SGPath::validate() grants read access -> OK
+    if (completePath.isAbsolute()) {
+        const auto authorizedPath = completePath.validate(false);
+        if (authorizedPath.isNull()) {
+            const auto msg = "ResourceManager::findPath(): access refused "
+                "because of the SGPath::validate() security policy: '" +
+                completePath.utf8Str() + "' (maybe a path relative to $FG_ROOT "
+                "that incorrectly starts with a '/'?)";
+            SG_LOG(SG_GENERAL, SG_WARN, msg);
+        } else if (authorizedPath.exists()) {
+            return authorizedPath;
+        }
     }
-    
+
+    // Normally, we could skip this loop when completePath.isAbsolute() is
+    // true. However, we currently need it because a bunch of PropertyList
+    // files in $FG_ROOT/Materials, such as
+    // $FG_ROOT/Materials/regions/materials.xml, rely on a BasePathProvider to
+    // open included files with a path given relatively to $FG_ROOT
+    // (readProperties() uses this function to locate included files).
+    for (const auto& provider : _providers) {
+        const SGPath path = provider->resolve(aResource, aContext);
+        if (!path.isNull()) {
+            return path;
+        }
+    }
+
     return SGPath();
 }