From 58664a2c7e2e131b48458ab8083fef0b996130df Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 24 Oct 2023 17:14:52 -0500 Subject: [PATCH] Add checks for resource strings (#930) * Add checks for resource strings * Don't use resource regex against png files --- .../Controllers/Resources/PhotosController.cs | 8 +++++ .../Resources/ResourcesController.cs | 1 - .../Controllers/UserController.cs | 7 ++-- .../Controllers/Admin/AdminUserController.cs | 3 +- ProjectLighthouse.Tests/Unit/ResourceTests.cs | 33 +++++++++++++++++++ ProjectLighthouse/Files/FileHelper.cs | 8 ++++- ProjectLighthouse/Types/Resources/LbpFile.cs | 3 +- 7 files changed, 53 insertions(+), 10 deletions(-) diff --git a/ProjectLighthouse.Servers.GameServer/Controllers/Resources/PhotosController.cs b/ProjectLighthouse.Servers.GameServer/Controllers/Resources/PhotosController.cs index a9c640d89..e79626439 100644 --- a/ProjectLighthouse.Servers.GameServer/Controllers/Resources/PhotosController.cs +++ b/ProjectLighthouse.Servers.GameServer/Controllers/Resources/PhotosController.cs @@ -3,6 +3,7 @@ using LBPUnion.ProjectLighthouse.Configuration; using LBPUnion.ProjectLighthouse.Database; using LBPUnion.ProjectLighthouse.Extensions; +using LBPUnion.ProjectLighthouse.Files; using LBPUnion.ProjectLighthouse.Helpers; using LBPUnion.ProjectLighthouse.Logging; using LBPUnion.ProjectLighthouse.Types.Entities.Level; @@ -42,6 +43,13 @@ public async Task UploadPhoto() GamePhoto? photo = await this.DeserializeBody(); if (photo == null) return this.BadRequest(); + string[] photoHashes = + { + photo.LargeHash, photo.MediumHash, photo.SmallHash, photo.PlanHash, + }; + + if (photoHashes.Any(hash => !FileHelper.ResourceExists(hash))) return this.BadRequest(); + foreach (PhotoEntity p in this.database.Photos.Where(p => p.CreatorId == token.UserId)) { if (p.LargeHash == photo.LargeHash) return this.Ok(); // photo already uploaded diff --git a/ProjectLighthouse.Servers.GameServer/Controllers/Resources/ResourcesController.cs b/ProjectLighthouse.Servers.GameServer/Controllers/Resources/ResourcesController.cs index 7973c9a85..3c1c2f822 100644 --- a/ProjectLighthouse.Servers.GameServer/Controllers/Resources/ResourcesController.cs +++ b/ProjectLighthouse.Servers.GameServer/Controllers/Resources/ResourcesController.cs @@ -49,7 +49,6 @@ public IActionResult GetResource(string hash) return this.NotFound(); } - // TODO: check if this is a valid hash [HttpPost("upload/{hash}/unattributed")] [HttpPost("upload/{hash}")] public async Task UploadResource(string hash) diff --git a/ProjectLighthouse.Servers.GameServer/Controllers/UserController.cs b/ProjectLighthouse.Servers.GameServer/Controllers/UserController.cs index 98e8ea665..c8d23089c 100644 --- a/ProjectLighthouse.Servers.GameServer/Controllers/UserController.cs +++ b/ProjectLighthouse.Servers.GameServer/Controllers/UserController.cs @@ -83,12 +83,9 @@ public async Task UpdateUser() // ReSharper disable once LoopCanBeConvertedToQuery foreach (string? resource in new[]{update.IconHash, update.YayHash, update.MehHash, update.BooHash, update.PlanetHash,}) { - if (resource == "0") continue; + if (string.IsNullOrWhiteSpace(resource)) continue; - if (resource != null && !resource.StartsWith('g') && !FileHelper.ResourceExists(resource)) - { - return this.BadRequest(); - } + if (!FileHelper.ResourceExists(resource)) return this.BadRequest(); } if (update.IconHash != null) user.IconHash = update.IconHash; diff --git a/ProjectLighthouse.Servers.Website/Controllers/Admin/AdminUserController.cs b/ProjectLighthouse.Servers.Website/Controllers/Admin/AdminUserController.cs index 3a05d089d..0fa327d8a 100644 --- a/ProjectLighthouse.Servers.Website/Controllers/Admin/AdminUserController.cs +++ b/ProjectLighthouse.Servers.Website/Controllers/Admin/AdminUserController.cs @@ -8,7 +8,6 @@ using LBPUnion.ProjectLighthouse.Types.Users; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; -using IOFile = System.IO.File; namespace LBPUnion.ProjectLighthouse.Servers.Website.Controllers.Admin; @@ -68,7 +67,7 @@ public async Task WipePlanets([FromRoute] int id) { // And finally, attempt to remove the resource from the filesystem. We don't want that taking up space. try { - IOFile.Delete(FileHelper.GetResourcePath(hash)); + FileHelper.DeleteResource(hash); Logger.Success($"Deleted planet resource {hash}", LogArea.Admin); } diff --git a/ProjectLighthouse.Tests/Unit/ResourceTests.cs b/ProjectLighthouse.Tests/Unit/ResourceTests.cs index 3fe5a7588..314a2475f 100644 --- a/ProjectLighthouse.Tests/Unit/ResourceTests.cs +++ b/ProjectLighthouse.Tests/Unit/ResourceTests.cs @@ -10,6 +10,39 @@ namespace LBPUnion.ProjectLighthouse.Tests.Unit; [Trait("Category", "Unit")] public class ResourceTests { + + [Fact] + public void IsResourceValid_ReturnsTrue_ForValidResource() + { + string[] resources = { + "g123456", "g123", "98f54143ab4e86b28c3afee0f50f2f51cfb2ed38", "0ebe53fc820a544798000188d39bfda94f53fe37" + }; + Assert.Multiple(() => + { + foreach (string resource in resources) + { + Assert.True(FileHelper.IsResourceValid(resource)); + } + }); + + } + + [Fact] + public void IsResourceValid_ReturnsFalse_ForInvalidResource() + { + string[] resources = + { + "G0234", "g123456789012334567", "b28c3afee0f50f2f51cfb2ed38", "../Test", + }; + Assert.Multiple(() => + { + foreach (string resource in resources) + { + Assert.False(FileHelper.IsResourceValid(resource)); + } + }); + } + [Fact] public void ShouldNotDeleteResourceFolder() { diff --git a/ProjectLighthouse/Files/FileHelper.cs b/ProjectLighthouse/Files/FileHelper.cs index e1e49eeae..880cb9604 100644 --- a/ProjectLighthouse/Files/FileHelper.cs +++ b/ProjectLighthouse/Files/FileHelper.cs @@ -2,6 +2,7 @@ using System; using System.IO; using System.Linq; +using System.Text.RegularExpressions; using LBPUnion.ProjectLighthouse.Configuration; using LBPUnion.ProjectLighthouse.Types.Resources; @@ -9,6 +10,9 @@ namespace LBPUnion.ProjectLighthouse.Files; public static partial class FileHelper { + [GeneratedRegex("^(g[0-9]{3,16}|[a-z0-9]{40})$")] + private static partial Regex ResourceRegex(); + public static readonly string ResourcePath = Path.Combine(Environment.CurrentDirectory, "r"); public static readonly string FullResourcePath = Path.GetFullPath(ResourcePath); @@ -21,6 +25,8 @@ public static partial class FileHelper public static string GetImagePath(string hash) => Path.Combine(ImagePath, hash); + public static bool IsResourceValid(string hash) => ResourceRegex().IsMatch(hash); + public static bool IsFileSafe(LbpFile file) { if (!ServerConfiguration.Instance.CheckForUnsafeFiles) return true; @@ -52,7 +58,7 @@ public static bool IsFileSafe(LbpFile file) }; } - public static bool ResourceExists(string hash) => File.Exists(GetResourcePath(hash)); + public static bool ResourceExists(string hash) => ResourceRegex().IsMatch(hash) && File.Exists(GetResourcePath(hash)); public static bool ImageExists(string hash) => File.Exists(GetImagePath(hash)); public static void DeleteResource(string hash) diff --git a/ProjectLighthouse/Types/Resources/LbpFile.cs b/ProjectLighthouse/Types/Resources/LbpFile.cs index cc1a12730..ecf9d688e 100644 --- a/ProjectLighthouse/Types/Resources/LbpFile.cs +++ b/ProjectLighthouse/Types/Resources/LbpFile.cs @@ -30,7 +30,8 @@ public LbpFile(byte[] data) public static LbpFile? FromHash(string? hash) { - if (hash == null) return null; + if (hash == null || !FileHelper.IsResourceValid(hash)) return null; + string path = FileHelper.GetResourcePath(hash); if (!File.Exists(path)) return null;