Merge pull request #16 from Regalis11/master

v0.14.9.1

Barotrauma/BarotraumaClient/ClientSource/Characters/CharacterInfo.cs

@@ -1,12 +1,12 @@
 using Barotrauma.Extensions;
 using Barotrauma.Networking;
-using System;
+using System;
 using System.Linq;
 using System.Collections.Generic;
 using Microsoft.Xna.Framework;
 using Microsoft.Xna.Framework.Graphics;
 using System.Xml.Linq;
-using System.IO;
+using Barotrauma.IO;
 using Barotrauma.Items.Components;
 
 namespace Barotrauma

Barotrauma/BarotraumaClient/ClientSource/Networking/FileTransfer/FileReceiver.cs

@@ -494,7 +494,26 @@ namespace Barotrauma.Networking
                     stream?.Close();
                     break;
                 case FileTransferType.CampaignSave:
-                    //TODO: verify that the received file is a valid save file
+                    try
+                    {
+                        var files = SaveUtil.EnumerateContainedFiles(fileTransfer.FilePath);
+                        foreach (var file in files)
+                        {
+                            string extension = Path.GetExtension(file);
+                            if ((!extension.Equals(".sub", StringComparison.OrdinalIgnoreCase)
+                                && !file.Equals("gamesession.xml"))
+                                || file.CleanUpPathCrossPlatform(correctFilenameCase: false).Contains('/'))
+                            {
+                                ErrorMessage = $"Found unexpected file in \"{fileTransfer.FileName}\"! ({file})";
+                                return false;
+                            }
+                        }
+                    }
+                    catch (Exception e)
+                    {
+                        ErrorMessage = $"Loading received campaign save \"{fileTransfer.FileName}\" failed! {{{e.Message}}}";
+                        return false;
+                    }
                     break;
             }
 

Barotrauma/BarotraumaClient/ClientSource/Screens/EditorImage.cs

@@ -1,7 +1,7 @@
 #nullable enable
 using System;
 using System.Collections.Generic;
-using System.IO;
+using Barotrauma.IO;
 using System.Linq;
 using System.Xml.Linq;
 using Microsoft.Xna.Framework;

Barotrauma/BarotraumaShared/SharedSource/ContentPackage.cs

@@ -416,7 +416,9 @@ namespace Barotrauma
                     default:
                         try
                         {
-                            XDocument.Load(file.Path);
+                            using FileStream stream = File.Open(file.Path, System.IO.FileMode.Open, System.IO.FileAccess.Read);
+                            using var reader = XMLExtensions.CreateReader(stream);
+                            XDocument.Load(reader);
                         }
                         catch (Exception e)
                         {

Barotrauma/BarotraumaShared/SharedSource/ForbiddenWordFilter.cs

@@ -1,5 +1,5 @@
 using System.Collections.Generic;
-using System.IO;
+using Barotrauma.IO;
 using System.Linq;
 
 namespace Barotrauma
@@ -16,7 +16,7 @@ namespace Barotrauma
             {
                 forbiddenWords = File.ReadAllLines(fileListPath).Select(s => s.ToLowerInvariant()).ToHashSet();
             }
-            catch (IOException e)
+            catch (System.IO.IOException e)
             {
                 DebugConsole.ThrowError($"Failed to load the list of forbidden words from {fileListPath}.", e);
             }

Barotrauma/BarotraumaShared/SharedSource/Map/SubmarineInfo.cs

@@ -743,7 +743,10 @@ namespace Barotrauma
                 try
                 {
                     stream.Position = 0;
-                    doc = XDocument.Load(stream); //ToolBox.TryLoadXml(file);
+                    using (var reader = XMLExtensions.CreateReader(stream))
+                    {
+                        doc = XDocument.Load(reader);
+                    }
                     stream.Close();
                     stream.Dispose();
                 }
@@ -760,9 +763,10 @@ namespace Barotrauma
                 try
                 {
                     ToolBox.IsProperFilenameCase(file);
-                    doc = XDocument.Load(file, LoadOptions.SetBaseUri);
+                    using var stream = File.Open(file, System.IO.FileMode.Open, System.IO.FileAccess.Read);
+                    using var reader = XMLExtensions.CreateReader(stream);
+                    doc = XDocument.Load(reader);
                 }
-
                 catch (Exception e)
                 {
                     exception = e;

Barotrauma/BarotraumaShared/SharedSource/Serialization/XMLExtensions.cs

@@ -1,5 +1,4 @@
 using System;
-using Barotrauma.IO;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
@@ -7,6 +6,8 @@ using System.Text;
 using System.Xml;
 using System.Xml.Linq;
 using Microsoft.Xna.Framework;
+using File = Barotrauma.IO.File;
+using FileStream = Barotrauma.IO.FileStream;
 
 namespace Barotrauma
 {
@@ -14,13 +15,45 @@ namespace Barotrauma
     {
         public static string ParseContentPathFromUri(this XObject element) => ToolBox.ConvertAbsoluteToRelativePath(element.BaseUri);
 
+        public static readonly XmlReaderSettings ReaderSettings = new XmlReaderSettings
+        {
+            DtdProcessing = DtdProcessing.Prohibit,
+            XmlResolver = null
+        };
+        
+        public static XmlReader CreateReader(System.IO.Stream stream)
+            => XmlReader.Create(stream, ReaderSettings);
+        
+        public static XDocument TryLoadXml(System.IO.Stream stream)
+        {
+            XDocument doc;
+            try
+            {
+                using XmlReader reader = CreateReader(stream);
+                doc = XDocument.Load(reader);
+            }
+            catch (Exception e)
+            {
+                DebugConsole.ThrowError($"Couldn't load xml document from stream!", e);
+                return null;
+            }
+            if (doc?.Root == null)
+            {
+                DebugConsole.ThrowError("XML could not be loaded from stream: Document or the root element is invalid!");
+                return null;
+            }
+            return doc;
+        }
+        
         public static XDocument TryLoadXml(string filePath)
         {
             XDocument doc;
             try
             {
                 ToolBox.IsProperFilenameCase(filePath);
-                doc = XDocument.Load(filePath, LoadOptions.SetBaseUri);
+                using FileStream stream = File.Open(filePath, System.IO.FileMode.Open, System.IO.FileAccess.Read);
+                using XmlReader reader = CreateReader(stream);
+                doc = XDocument.Load(reader);
             }
             catch (Exception e)
             {
@@ -45,7 +78,9 @@ namespace Barotrauma
             {
                 try
                 {
-                    doc = XDocument.Load(filePath, LoadOptions.SetBaseUri);
+                    using FileStream stream = File.Open(filePath, System.IO.FileMode.Open, System.IO.FileAccess.Read);
+                    using XmlReader reader = CreateReader(stream);
+                    doc = XDocument.Load(reader);
                 }
                 catch
                 {

Barotrauma/BarotraumaShared/SharedSource/Utils/SafeIO.cs

@@ -1,21 +1,41 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 
 namespace Barotrauma.IO
 {
     static class Validation
     {
-        static readonly string[] unwritableDirs = new string[] { "Content", "Data/ContentPackages" };
+        private static readonly string[] unwritableDirs = new string[] { "Content", "Data/ContentPackages" };
+        private static readonly string[] unwritableExtensions = new string[]
+        {
+            ".pdb", ".com", ".scr", ".dylib", ".so", ".a", ".app", //executables and libraries (.exe and .dll handled separately in CanWrite)
+            ".bat", ".sh", //shell scripts
+            ".json" //deps.json
+        };
 
         /// <summary>
         /// When set to true, the game is allowed to modify the vanilla content in debug builds. Has no effect in non-debug builds.
         /// </summary>
         public static bool SkipValidationInDebugBuilds;
 
-        public static bool CanWrite(string path)
+        public static bool CanWrite(string path, bool isDirectory)
         {
             path = System.IO.Path.GetFullPath(path).CleanUpPath();
 
+            string extension = System.IO.Path.GetExtension(path).Replace(" ", "");
+            if (unwritableExtensions.Any(e => e.Equals(extension, StringComparison.OrdinalIgnoreCase)))
+            {
+                return false;
+            }
+
+            if (!path.StartsWith(System.IO.Path.GetFullPath("Mods/").CleanUpPath(), StringComparison.OrdinalIgnoreCase)
+                && (extension.Equals(".dll", StringComparison.OrdinalIgnoreCase)
+                    || extension.Equals(".exe", StringComparison.OrdinalIgnoreCase)))
+            {
+                return false;
+            }
+            
             foreach (string unwritableDir in unwritableDirs)
             {
                 string dir = System.IO.Path.GetFullPath(unwritableDir).CleanUpPath();
@@ -38,9 +58,9 @@ namespace Barotrauma.IO
     {
         public static void SaveSafe(this System.Xml.Linq.XDocument doc, string path)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, false))
             {
-                DebugConsole.ThrowError($"Cannot save XML document to \"{path}\": modifying the files in the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot save XML document to \"{path}\": modifying the files in this folder/with this extension is not allowed.");
                 return;
             }
             doc.Save(path);
@@ -48,9 +68,9 @@ namespace Barotrauma.IO
 
         public static void SaveSafe(this System.Xml.Linq.XElement element, string path)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, false))
             {
-                DebugConsole.ThrowError($"Cannot save XML element to \"{path}\": modifying the files in the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot save XML element to \"{path}\": modifying the files in this folder/with this extension is not allowed.");
                 return;
             }
             element.Save(path);
@@ -73,9 +93,9 @@ namespace Barotrauma.IO
 
         public XmlWriter(string path, System.Xml.XmlWriterSettings settings)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, false))
             {
-                DebugConsole.ThrowError($"Cannot write XML document to \"{path}\": modifying the files in the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot write XML document to \"{path}\": modifying the files in this folder/with this extension is not allowed.");
                 Writer = null;
                 return;
             }
@@ -223,9 +243,9 @@ namespace Barotrauma.IO
 
         public static System.IO.DirectoryInfo CreateDirectory(string path)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, true))
             {
-                DebugConsole.ThrowError($"Cannot create directory \"{path}\": modifying the contents of the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot create directory \"{path}\": modifying the contents of this folder/using this extension is not allowed.");
                 return null;
             }
             return System.IO.Directory.CreateDirectory(path);
@@ -233,9 +253,9 @@ namespace Barotrauma.IO
 
         public static void Delete(string path, bool recursive=true)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, true))
             {
-                DebugConsole.ThrowError($"Cannot delete directory \"{path}\": modifying the contents of the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot delete directory \"{path}\": modifying the contents of this folder/using this extension is not allowed.");
                 return;
             }
             //TODO: validate recursion?
@@ -252,9 +272,9 @@ namespace Barotrauma.IO
 
         public static void Copy(string src, string dest, bool overwrite=false)
         {
-            if (!Validation.CanWrite(dest))
+            if (!Validation.CanWrite(dest, false))
             {
-                DebugConsole.ThrowError($"Cannot copy \"{src}\" to \"{dest}\": modifying the contents of the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot copy \"{src}\" to \"{dest}\": modifying the contents of this folder/using this extension is not allowed.");
                 return;
             }
             System.IO.File.Copy(src, dest, overwrite);
@@ -262,12 +282,12 @@ namespace Barotrauma.IO
 
         public static void Move(string src, string dest)
         {
-            if (!Validation.CanWrite(src))
+            if (!Validation.CanWrite(src, false))
             {
                 DebugConsole.ThrowError($"Cannot move \"{src}\" to \"{dest}\": modifying the contents of the source folder is not allowed.");
                 return;
             }
-            if (!Validation.CanWrite(dest))
+            if (!Validation.CanWrite(dest, false))
             {
                 DebugConsole.ThrowError($"Cannot move \"{src}\" to \"{dest}\": modifying the contents of the destination folder is not allowed");
                 return;
@@ -277,9 +297,9 @@ namespace Barotrauma.IO
 
         public static void Delete(string path)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, false))
             {
-                DebugConsole.ThrowError($"Cannot delete file \"{path}\": modifying the contents of the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot delete file \"{path}\": modifying the contents of this folder/using this extension is not allowed.");
                 return;
             }
             System.IO.File.Delete(path);
@@ -299,15 +319,15 @@ namespace Barotrauma.IO
                 case System.IO.FileMode.OpenOrCreate:
                 case System.IO.FileMode.Append:
                 case System.IO.FileMode.Truncate:
-                    if (!Validation.CanWrite(path))
+                    if (!Validation.CanWrite(path, false))
                     {
-                        DebugConsole.ThrowError($"Cannot open \"{path}\" in {mode} mode: modifying the contents of the folder is not allowed.");
+                        DebugConsole.ThrowError($"Cannot open \"{path}\" in {mode} mode: modifying the contents of this folder/using this extension is not allowed.");
                         return null;
                     }
                     break;
             }
             return new FileStream(path, System.IO.File.Open(path, mode,
-                !Validation.CanWrite(path) ?
+                !Validation.CanWrite(path, false) ?
                 System.IO.FileAccess.Read :
                 access));
         }
@@ -329,9 +349,9 @@ namespace Barotrauma.IO
 
         public static void WriteAllBytes(string path, byte[] contents)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, false))
             {
-                DebugConsole.ThrowError($"Cannot write all bytes to \"{path}\": modifying the files in the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot write all bytes to \"{path}\": modifying the files in this folder/with this extension is not allowed.");
                 return;
             }
             System.IO.File.WriteAllBytes(path, contents);
@@ -339,9 +359,9 @@ namespace Barotrauma.IO
 
         public static void WriteAllText(string path, string contents, System.Text.Encoding? encoding = null)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, false))
             {
-                DebugConsole.ThrowError($"Cannot write all text to \"{path}\": modifying the files in the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot write all text to \"{path}\": modifying the files in this folder/with this extension is not allowed.");
                 return;
             }
             System.IO.File.WriteAllText(path, contents, encoding ?? System.Text.Encoding.UTF8);
@@ -349,9 +369,9 @@ namespace Barotrauma.IO
 
         public static void WriteAllLines(string path, IEnumerable<string> contents, System.Text.Encoding? encoding = null)
         {
-            if (!Validation.CanWrite(path))
+            if (!Validation.CanWrite(path, false))
             {
-                DebugConsole.ThrowError($"Cannot write all lines to \"{path}\": modifying the files in the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot write all lines to \"{path}\": modifying the files in this folder/with this extension is not allowed.");
                 return;
             }
             System.IO.File.WriteAllLines(path, contents, encoding ?? System.Text.Encoding.UTF8);
@@ -391,7 +411,7 @@ namespace Barotrauma.IO
         {
             get
             {
-                if (!Validation.CanWrite(fileName)) { return false; }
+                if (!Validation.CanWrite(fileName, false)) { return false; }
                 return innerStream.CanWrite;
             }
         }
@@ -417,13 +437,13 @@ namespace Barotrauma.IO
 
         public override void Write(byte[] buffer, int offset, int count)
         {
-            if (Validation.CanWrite(fileName))
+            if (Validation.CanWrite(fileName, false))
             {
                 innerStream.Write(buffer, offset, count);
             }
             else
             {
-                DebugConsole.ThrowError($"Cannot write to file \"{fileName}\": modifying the files in the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot write to file \"{fileName}\": modifying the files in this folder/with this extension is not allowed.");
             }
         }
 
@@ -488,9 +508,9 @@ namespace Barotrauma.IO
 
         public void Delete()
         {
-            if (!Validation.CanWrite(innerInfo.FullName))
+            if (!Validation.CanWrite(innerInfo.FullName, false))
             {
-                DebugConsole.ThrowError($"Cannot delete directory \"{Name}\": modifying the contents of the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot delete directory \"{Name}\": modifying the contents of this folder/using this extension is not allowed.");
                 return;
             }
             innerInfo.Delete();
@@ -524,9 +544,9 @@ namespace Barotrauma.IO
             }
             set
             {
-                if (!Validation.CanWrite(innerInfo.FullName))
+                if (!Validation.CanWrite(innerInfo.FullName, false))
                 {
-                    DebugConsole.ThrowError($"Cannot set read-only to {value} for \"{Name}\": modifying the files in the folder is not allowed.");
+                    DebugConsole.ThrowError($"Cannot set read-only to {value} for \"{Name}\": modifying the files in this folder/with this extension is not allowed.");
                     return;
                 }
                 innerInfo.IsReadOnly = value;
@@ -535,7 +555,7 @@ namespace Barotrauma.IO
 
         public void CopyTo(string dest, bool overwriteExisting = false)
         {
-            if (!Validation.CanWrite(dest))
+            if (!Validation.CanWrite(dest, false))
             {
                 DebugConsole.ThrowError($"Cannot copy \"{Name}\" to \"{dest}\": modifying the contents of the destination folder is not allowed.");
                 return;
@@ -545,9 +565,9 @@ namespace Barotrauma.IO
 
         public void Delete()
         {
-            if (!Validation.CanWrite(innerInfo.FullName))
+            if (!Validation.CanWrite(innerInfo.FullName, false))
             {
-                DebugConsole.ThrowError($"Cannot delete file \"{Name}\": modifying the files in the folder is not allowed.");
+                DebugConsole.ThrowError($"Cannot delete file \"{Name}\": modifying the files in this folder/with this extension is not allowed.");
                 return;
             }
             innerInfo.Delete();

Barotrauma/BarotraumaShared/SharedSource/Utils/SaveUtil.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using Barotrauma.IO;
 using System.IO.Compression;
@@ -362,8 +363,10 @@ namespace Barotrauma
             }
         }
 
-        public static bool DecompressFile(string sDir, GZipStream zipStream, ProgressDelegate progress)
+        private static bool DecompressFile(bool writeFile, string sDir, GZipStream zipStream, ProgressDelegate progress, out string fileName)
         {
+            fileName = null;
+            
             //Decompress file name
             byte[] bytes = new byte[sizeof(int)];
             int Readed = zipStream.Read(bytes, 0, sizeof(int));
@@ -385,6 +388,8 @@ namespace Barotrauma
                 sb.Append(c);
             }
             string sFileName = sb.ToString();
+            
+            fileName = sFileName;
             progress?.Invoke(sFileName);
 
             //Decompress file content
@@ -397,6 +402,17 @@ namespace Barotrauma
 
             string sFilePath = Path.Combine(sDir, sFileName);
             string sFinalDir = Path.GetDirectoryName(sFilePath);
+
+            string sDirFull = (string.IsNullOrEmpty(sDir) ? Directory.GetCurrentDirectory() : Path.GetFullPath(sDir)).CleanUpPathCrossPlatform(correctFilenameCase: false);
+            string sFinalDirFull = (string.IsNullOrEmpty(sFinalDir) ? Directory.GetCurrentDirectory() : Path.GetFullPath(sFinalDir)).CleanUpPathCrossPlatform(correctFilenameCase: false);
+            
+            if (!sFinalDirFull.StartsWith(sDirFull, StringComparison.OrdinalIgnoreCase))
+            {
+                throw new InvalidOperationException(
+                    $"Error extracting \"{sFileName}\": cannot be extracted to parent directory");
+            }
+
+            if (!writeFile) { return true; }
             if (!Directory.Exists(sFinalDir))
                 Directory.CreateDirectory(sFinalDir);
 
@@ -431,7 +447,7 @@ namespace Barotrauma
                 {
                     using (FileStream inFile = File.Open(sCompressedFile, System.IO.FileMode.Open, System.IO.FileAccess.Read))
                     using (GZipStream zipStream = new GZipStream(inFile, CompressionMode.Decompress, true))
-                        while (DecompressFile(sDir, zipStream, progress)) { };
+                        while (DecompressFile(true, sDir, zipStream, progress, out _)) { };
 
                     break;
                 }
@@ -444,6 +460,35 @@ namespace Barotrauma
             }
         }
 
+        public static IEnumerable<string> EnumerateContainedFiles(string sCompressedFile)
+        {
+            int maxRetries = 4;
+            HashSet<string> paths = new HashSet<string>();
+            for (int i = 0; i <= maxRetries; i++)
+            {
+                try
+                {
+                    using FileStream inFile = File.Open(sCompressedFile, System.IO.FileMode.Open, System.IO.FileAccess.Read);
+                    using GZipStream zipStream = new GZipStream(inFile, CompressionMode.Decompress, true);
+                    while (DecompressFile(false, "", zipStream, null, out string fileName))
+                    {
+                        paths.Add(fileName);
+                    }
+                }
+                catch (System.IO.IOException e)
+                {
+                    if (i >= maxRetries || !File.Exists(sCompressedFile)) { throw; }
+
+                    DebugConsole.NewMessage(
+                        $"Failed to decompress file \"{sCompressedFile}\" for enumeration {{{e.Message}}}, retrying in 250 ms...",
+                        Color.Red);
+                    Thread.Sleep(250);
+                }
+            }
+
+            return paths;
+        }
+
         public static void CopyFolder(string sourceDirName, string destDirName, bool copySubDirs, bool overwriteExisting = false)
         {
             // Get the subdirectories for the specified directory.

Barotrauma/BarotraumaShared/changelog.txt

@@ -1,3 +1,9 @@
+---------------------------------------------------------------------------------------------------------
+v0.14.9.1
+---------------------------------------------------------------------------------------------------------
+
+- Fixed an exploit that allowed modified servers to send malicious campaign save files to clients.
+
 ---------------------------------------------------------------------------------------------------------
 v0.14.9.0
 ---------------------------------------------------------------------------------------------------------