U4-7042 - bugfix the physical filesystem

src/Umbraco.Core/IO/IOHelper.cs

@@ -152,12 +152,7 @@ namespace Umbraco.Core.IO
         /// <returns>A value indicating whether the filepath is valid.</returns>
         internal static bool VerifyEditPath(string filePath, string validDir)
         {
-            if (filePath.StartsWith(MapPath(SystemDirectories.Root)) == false)
-                filePath = MapPath(filePath);
-            if (validDir.StartsWith(MapPath(SystemDirectories.Root)) == false)
-                validDir = MapPath(validDir);
-
-            return filePath.StartsWith(validDir);
+            return VerifyEditPath(filePath, new[] { validDir });
         }
 
         /// <summary>
@@ -182,12 +177,17 @@ namespace Umbraco.Core.IO
         /// <returns>A value indicating whether the filepath is valid.</returns>
         internal static bool VerifyEditPath(string filePath, IEnumerable<string> validDirs)
         {
+            var mappedRoot = MapPath(SystemDirectories.Root);
+            if (filePath.StartsWith(mappedRoot) == false)
+                filePath = MapPath(filePath);
+
+            // don't trust what we get, it may contain relative segments
+            filePath = Path.GetFullPath(filePath);
+
             foreach (var dir in validDirs)
             {
                 var validDir = dir;
-                if (filePath.StartsWith(MapPath(SystemDirectories.Root)) == false)
-                    filePath = MapPath(filePath);
-                if (validDir.StartsWith(MapPath(SystemDirectories.Root)) == false)
+                if (validDir.StartsWith(mappedRoot) == false)
                     validDir = MapPath(validDir);
 
                 if (filePath.StartsWith(validDir))
@@ -219,11 +219,8 @@ namespace Umbraco.Core.IO
         /// <returns>A value indicating whether the filepath is valid.</returns>
         internal static bool VerifyFileExtension(string filePath, List<string> validFileExtensions)
         {
-            if (filePath.StartsWith(MapPath(SystemDirectories.Root)) == false)
-                filePath = MapPath(filePath);
-            var f = new FileInfo(filePath);
-            
-            return validFileExtensions.Contains(f.Extension.Substring(1));
+            var ext = Path.GetExtension(filePath);
+            return ext != null && validFileExtensions.Contains(ext.TrimStart('.'));
         }
 
         /// <summary>

src/Umbraco.Core/IO/PhysicalFileSystem.cs

@@ -177,9 +177,23 @@ namespace Umbraco.Core.IO
                 path = GetRelativePath(path);
             }
 
-            return !path.StartsWith(RootPath) 
-                ? Path.Combine(RootPath, path)
-                : path;
+            // if already a full path, return
+            if (path.StartsWith(RootPath))
+                return path;
+
+            // else combine and sanitize, ie GetFullPath will take care of any relative
+            // segments in path, eg '../../foo.tmp' - it may throw a SecurityException
+            // if the combined path reaches illegal parts of the filesystem
+            var fpath = Path.Combine(RootPath, path);
+            fpath = Path.GetFullPath(fpath);
+
+            // at that point, path is within legal parts of the filesystem, ie we have
+            // permissions to reach that path, but it may nevertheless be outside of
+            // our root path, due to relative segments, so better check
+            if (fpath.StartsWith(RootPath))
+                return fpath;
+
+            throw new FileSecurityException("File '" + path + "' is outside this filesystem's root.");
         }
 
         public string GetUrl(string path)

src/Umbraco.Core/Persistence/Repositories/ScriptRepository.cs

@@ -105,12 +105,22 @@ namespace Umbraco.Core.Persistence.Repositories
                 dirs += "," + SystemDirectories.MvcViews;*/
 
             //Validate file
-            var validFile = IOHelper.VerifyEditPath(script.VirtualPath, dirs.Split(','));
+            string fullPath;
+            try
+            {
+                // may throw for security reasons
+                fullPath = FileSystem.GetFullPath(script.Path);
+            }
+            catch
+            {
+                return false;
+            }
+            var isValidPath = IOHelper.VerifyEditPath(fullPath, dirs.Split(','));
 
             //Validate extension
-            var validExtension = IOHelper.VerifyFileExtension(script.VirtualPath, exts);
+            var isValidExtension = IOHelper.VerifyFileExtension(script.Path, exts);
 
-            return validFile && validExtension;
+            return isValidPath && isValidExtension;
         }
 
         #endregion

src/Umbraco.Core/Persistence/Repositories/StylesheetRepository.cs

@@ -99,19 +99,29 @@ namespace Umbraco.Core.Persistence.Repositories
             return FileSystem.GetFiles(rootPath ?? string.Empty, "*.css").Select(Get);
         }
 
+        private static readonly List<string> ValidExtensions = new List<string> { "css" };
+
         public bool ValidateStylesheet(Stylesheet stylesheet)
         {
             var dirs = SystemDirectories.Css;
 
             //Validate file
-            var validFile = IOHelper.VerifyEditPath(stylesheet.VirtualPath, dirs.Split(','));
+            string fullPath;
+            try
+            {
+                // may throw for security reasons
+                fullPath = FileSystem.GetFullPath(stylesheet.Path);
+            }
+            catch
+            {
+                return false;
+            }
+            var isValidPath = IOHelper.VerifyEditPath(fullPath, dirs.Split(','));
 
             //Validate extension
-            var validExtension = IOHelper.VerifyFileExtension(stylesheet.VirtualPath, new List<string> { "css" });
+            var isValidExtension = IOHelper.VerifyFileExtension(stylesheet.Path, ValidExtensions);
 
-            var fileValid = validFile && validExtension;
-
-            return fileValid;
+            return isValidPath && isValidExtension;
         }
 
         #endregion

src/Umbraco.Tests/IO/PhysicalFileSystemTests.cs

@@ -27,6 +27,8 @@ namespace Umbraco.Tests.IO
         public void TearDown()
         {
             var path = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "FileSysTests");
+            if (Directory.Exists(path) == false) return;
+
             var files = Directory.GetFiles(path);
             foreach (var f in files)
             {
@@ -39,5 +41,31 @@ namespace Umbraco.Tests.IO
         {
             return "/Media/" + path;
         }
+
+        [Test]
+        public void GetFullPathTest()
+        {
+            // outside of tests, one initializes the PhysicalFileSystem with eg ~/Dir
+            // and then, rootPath = /path/to/Dir and rootUrl = /Dir/
+            // here we initialize the PhysicalFileSystem with
+            // rootPath = /path/to/FileSysTests
+            // rootUrl = /Media/
+
+            var basePath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "FileSysTests");
+
+            // ensure that GetFullPath
+            // - does return the proper full path
+            // - does properly normalize separators
+            // - does throw on invalid paths
+
+            var path = _fileSystem.GetFullPath("foo.tmp");
+            Assert.AreEqual(Path.Combine(basePath, @"foo.tmp"), path);
+
+            path = _fileSystem.GetFullPath("foo/bar.tmp");
+            Assert.AreEqual(Path.Combine(basePath, @"foo\bar.tmp"), path);
+
+            // that path is invalid as it would be outside the root directory
+            Assert.Throws<FileSecurityException>(() => _fileSystem.GetFullPath("../../foo.tmp"));
+        }
     }
 }