diff --git a/Barotrauma/BarotraumaClient/ClientSource/Characters/CharacterInfo.cs b/Barotrauma/BarotraumaClient/ClientSource/Characters/CharacterInfo.cs index 1d2892d72..e88192195 100644 --- a/Barotrauma/BarotraumaClient/ClientSource/Characters/CharacterInfo.cs +++ b/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 diff --git a/Barotrauma/BarotraumaClient/ClientSource/Networking/FileTransfer/FileReceiver.cs b/Barotrauma/BarotraumaClient/ClientSource/Networking/FileTransfer/FileReceiver.cs index 5de8e14c5..4cd0d5e84 100644 --- a/Barotrauma/BarotraumaClient/ClientSource/Networking/FileTransfer/FileReceiver.cs +++ b/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; } diff --git a/Barotrauma/BarotraumaClient/ClientSource/Screens/EditorImage.cs b/Barotrauma/BarotraumaClient/ClientSource/Screens/EditorImage.cs index 4563b2ff6..dab80c527 100644 --- a/Barotrauma/BarotraumaClient/ClientSource/Screens/EditorImage.cs +++ b/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; diff --git a/Barotrauma/BarotraumaShared/SharedSource/ContentPackage.cs b/Barotrauma/BarotraumaShared/SharedSource/ContentPackage.cs index dbfb4cd19..ba609f244 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/ContentPackage.cs +++ b/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) { diff --git a/Barotrauma/BarotraumaShared/SharedSource/ForbiddenWordFilter.cs b/Barotrauma/BarotraumaShared/SharedSource/ForbiddenWordFilter.cs index 593283d23..bcdbde9f6 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/ForbiddenWordFilter.cs +++ b/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); } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Map/SubmarineInfo.cs b/Barotrauma/BarotraumaShared/SharedSource/Map/SubmarineInfo.cs index 8e718636a..1a1876939 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Map/SubmarineInfo.cs +++ b/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; diff --git a/Barotrauma/BarotraumaShared/SharedSource/Serialization/XMLExtensions.cs b/Barotrauma/BarotraumaShared/SharedSource/Serialization/XMLExtensions.cs index fb24f1a63..0e6da820c 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Serialization/XMLExtensions.cs +++ b/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 { diff --git a/Barotrauma/BarotraumaShared/SharedSource/Utils/SafeIO.cs b/Barotrauma/BarotraumaShared/SharedSource/Utils/SafeIO.cs index 201c9d7e9..563760152 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Utils/SafeIO.cs +++ b/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 + }; /// /// When set to true, the game is allowed to modify the vanilla content in debug builds. Has no effect in non-debug builds. /// 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 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(); diff --git a/Barotrauma/BarotraumaShared/SharedSource/Utils/SaveUtil.cs b/Barotrauma/BarotraumaShared/SharedSource/Utils/SaveUtil.cs index 105395eb2..2227bfdf5 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Utils/SaveUtil.cs +++ b/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 EnumerateContainedFiles(string sCompressedFile) + { + int maxRetries = 4; + HashSet paths = new HashSet(); + 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. diff --git a/Barotrauma/BarotraumaShared/changelog.txt b/Barotrauma/BarotraumaShared/changelog.txt index 02351378a..a3942d1a0 100644 --- a/Barotrauma/BarotraumaShared/changelog.txt +++ b/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 ---------------------------------------------------------------------------------------------------------