From 944b2995f1bf7719cfcfb9bafe713523dbd8883f Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 18 Aug 2018 23:33:38 -0400 Subject: no longer allow non-relative paths for IContentPack.Read/WriteJsonFile (#468) --- docs/release-notes.md | 1 + src/SMAPI/Framework/ContentPack.cs | 8 ++++++++ src/SMAPI/IContentPack.cs | 2 ++ src/StardewModdingAPI.Toolkit/Utilities/PathUtilities.cs | 12 ++++++++++++ 4 files changed, 23 insertions(+) diff --git a/docs/release-notes.md b/docs/release-notes.md index fc6ea97f..09d444a3 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -5,6 +5,7 @@ * For modders: * Added `IContentPack.WriteJsonFile` method. + * Fixed `IContentPack.ReadJsonFile` allowing non-relative paths. ## 2.7 * For players: diff --git a/src/SMAPI/Framework/ContentPack.cs b/src/SMAPI/Framework/ContentPack.cs index ccb2b9a0..49285388 100644 --- a/src/SMAPI/Framework/ContentPack.cs +++ b/src/SMAPI/Framework/ContentPack.cs @@ -51,8 +51,12 @@ namespace StardewModdingAPI.Framework /// The model type. /// The file path relative to the contnet directory. /// Returns the deserialised model, or null if the file doesn't exist or is empty. + /// The is not relative or contains directory climbing (../). public TModel ReadJsonFile(string path) where TModel : class { + if (!PathUtilities.IsSafeRelativePath(path)) + throw new InvalidOperationException($"You must call {nameof(IContentPack)}.{nameof(this.ReadJsonFile)} with a relative path."); + path = Path.Combine(this.DirectoryPath, PathUtilities.NormalisePathSeparators(path)); return this.JsonHelper.ReadJsonFileIfExists(path, out TModel model) ? model @@ -63,8 +67,12 @@ namespace StardewModdingAPI.Framework /// The model type. This should be a plain class that has public properties for the data you want. The properties can be complex types. /// The file path relative to the mod folder. /// The arbitrary data to save. + /// The is not relative or contains directory climbing (../). public void WriteJsonFile(string path, TModel data) where TModel : class { + if (!PathUtilities.IsSafeRelativePath(path)) + throw new InvalidOperationException($"You must call {nameof(IContentPack)}.{nameof(this.WriteJsonFile)} with a relative path."); + path = Path.Combine(this.DirectoryPath, PathUtilities.NormalisePathSeparators(path)); this.JsonHelper.WriteJsonFile(path, data); } diff --git a/src/SMAPI/IContentPack.cs b/src/SMAPI/IContentPack.cs index fa793b13..9ba32394 100644 --- a/src/SMAPI/IContentPack.cs +++ b/src/SMAPI/IContentPack.cs @@ -25,12 +25,14 @@ namespace StardewModdingAPI /// The model type. This should be a plain class that has public properties for the data you want. The properties can be complex types. /// The file path relative to the content pack directory. /// Returns the deserialised model, or null if the file doesn't exist or is empty. + /// The is not relative or contains directory climbing (../). TModel ReadJsonFile(string path) where TModel : class; /// Save data to a JSON file in the content pack's folder. /// The model type. This should be a plain class that has public properties for the data you want. The properties can be complex types. /// The file path relative to the mod folder. /// The arbitrary data to save. + /// The is not relative or contains directory climbing (../). void WriteJsonFile(string path, TModel data) where TModel : class; /// Load content from the content pack folder (if not already cached), and return it. When loading a .png file, this must be called outside the game's draw loop. diff --git a/src/StardewModdingAPI.Toolkit/Utilities/PathUtilities.cs b/src/StardewModdingAPI.Toolkit/Utilities/PathUtilities.cs index b959f9b5..79748c25 100644 --- a/src/StardewModdingAPI.Toolkit/Utilities/PathUtilities.cs +++ b/src/StardewModdingAPI.Toolkit/Utilities/PathUtilities.cs @@ -63,6 +63,18 @@ namespace StardewModdingAPI.Toolkit.Utilities return relative; } + /// Get whether a path is relative and doesn't try to climb out of its containing folder (e.g. doesn't contain ../). + /// The path to check. + public static bool IsSafeRelativePath(string path) + { + if (string.IsNullOrWhiteSpace(path)) + return true; + + return + !Path.IsPathRooted(path) + && PathUtilities.GetSegments(path).All(segment => segment.Trim() != ".."); + } + /// Get whether a string is a valid 'slug', containing only basic characters that are safe in all contexts (e.g. filenames, URLs, etc). /// The string to check. public static bool IsSlug(string str) -- cgit