Add some resource tests and other small changes (#638)

* Cleanup custom icon resources and add resource tests

* Don't delete old custom icons

ProjectLighthouse.Servers.GameServer/Controllers/Resources/PhotosController.cs

@@ -2,6 +2,7 @@
 using Discord;
 using LBPUnion.ProjectLighthouse.Configuration;
 using LBPUnion.ProjectLighthouse.Extensions;
+using LBPUnion.ProjectLighthouse.Files;
 using LBPUnion.ProjectLighthouse.Helpers;
 using LBPUnion.ProjectLighthouse.Levels;
 using LBPUnion.ProjectLighthouse.Logging;
@@ -227,14 +228,7 @@ public class PhotosController : ControllerBase
         HashSet<string> photoResources = new(){photo.LargeHash, photo.SmallHash, photo.MediumHash, photo.PlanHash,};
         foreach (string hash in photoResources)
         {
-            if (System.IO.File.Exists(Path.Combine("png", $"{hash}.png")))
+            FileHelper.DeleteResource(hash);
-            {
-                System.IO.File.Delete(Path.Combine("png", $"{hash}.png"));
-            }
-            if (System.IO.File.Exists(Path.Combine("r", hash)))
-            {
-                System.IO.File.Delete(Path.Combine("r", hash));
-            }
         }
 
         this.database.Photos.Remove(photo);

ProjectLighthouse.Servers.Website/Controllers/Admin/AdminReportController.cs

@@ -1,5 +1,6 @@
 #nullable enable
 using LBPUnion.ProjectLighthouse.Administration.Reports;
+using LBPUnion.ProjectLighthouse.Files;
 using LBPUnion.ProjectLighthouse.PlayerData.Profiles;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.EntityFrameworkCore;
@@ -35,14 +36,7 @@ public class AdminReportController : ControllerBase
             hashes.Add(report.InitialStateHash);
         foreach (string hash in hashes)
         {
-            if (System.IO.File.Exists(Path.Combine("png", $"{hash}.png")))
+            FileHelper.DeleteResource(hash);
-            {
-                System.IO.File.Delete(Path.Combine("png", $"{hash}.png"));
-            }
-            if (System.IO.File.Exists(Path.Combine("r", hash)))
-            {
-                System.IO.File.Delete(Path.Combine("r", hash));
-            }
         }
         this.database.Reports.Remove(report);
 

ProjectLighthouse.Tests/Tests/ResourceTests.cs

@@ -0,0 +1,64 @@
+using System;
+using System.IO;
+using LBPUnion.ProjectLighthouse.Files;
+using Xunit;
+
+namespace LBPUnion.ProjectLighthouse.Tests;
+
+public class ResourceTests
+{
+    [Fact]
+    public void ShouldNotDeleteResourceFolder()
+    {
+        FileHelper.EnsureDirectoryCreated(FileHelper.ResourcePath);
+        Assert.True(Directory.Exists(FileHelper.ResourcePath));
+        FileHelper.DeleteResource(FileHelper.ResourcePath);
+        Assert.True(Directory.Exists(FileHelper.ResourcePath));
+    }
+
+    [Fact]
+    public void ShouldNotDeleteImagesFolder()
+    {
+        FileHelper.EnsureDirectoryCreated(FileHelper.ImagePath);
+        Assert.True(Directory.Exists(FileHelper.ImagePath));
+        FileHelper.DeleteResource(FileHelper.ImagePath);
+        Assert.True(Directory.Exists(FileHelper.ImagePath));
+    }
+
+    [Fact]
+    public void ShouldNotRecursivelyTraverseImage()
+    {
+        string path = Path.Combine(FileHelper.ImagePath, $"..{Path.DirectorySeparatorChar}appsettings.json");
+        FileHelper.DeleteResource(path);
+        Assert.True(File.Exists(Path.Combine(FileHelper.ImagePath, $"..{Path.DirectorySeparatorChar}appsettings.json")));
+    }
+
+    [Fact]
+    public void ShouldNotRecursivelyTraverseResource()
+    {
+        string path = Path.Combine(FileHelper.ResourcePath, $"..{Path.DirectorySeparatorChar}appsettings.json");
+        FileHelper.DeleteResource(path);
+        Assert.True(File.Exists(Path.Combine(FileHelper.ResourcePath, $"..{Path.DirectorySeparatorChar}appsettings.json")));
+    }
+
+    [Fact]
+    public async void ShouldDeleteResourceAndImage()
+    {
+        FileHelper.EnsureDirectoryCreated(FileHelper.ResourcePath);
+        FileHelper.EnsureDirectoryCreated(FileHelper.ImagePath);
+        string? hash = await FileHelper.ParseBase64Image("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8r8NQDwAFCQGsNA7jBAAAAABJRU5ErkJggg==");
+        LbpFile? file = LbpFile.FromHash("ed4e2857a2e315e4487ea976d1b398f57b863ff4");
+        Assert.True(file != null);
+        // Convert resource to png
+        FileHelper.LbpFileToPNG(file);
+
+        Assert.True(hash != null);
+        Assert.True(hash.Equals("ed4e2857a2e315e4487ea976d1b398f57b863ff4", StringComparison.InvariantCultureIgnoreCase));
+        Assert.True(File.Exists(Path.Combine(FileHelper.ResourcePath, hash)));
+        Assert.True(File.Exists(Path.Combine(FileHelper.ImagePath, $"{hash}.png")));
+
+        FileHelper.DeleteResource(hash);
+        Assert.False(File.Exists(Path.Combine(FileHelper.ResourcePath, hash)));
+        Assert.False(File.Exists(Path.Combine(FileHelper.ImagePath, $"{hash}.png")));
+    }
+}

ProjectLighthouse/Files/FileHelper.cs

@@ -7,7 +7,6 @@ using System.IO;
 using System.Linq;
 using System.Text;
 using System.Text.RegularExpressions;
-using System.Threading;
 using System.Threading.Tasks;
 using ICSharpCode.SharpZipLib.Zip.Compression;
 using LBPUnion.ProjectLighthouse.Configuration;
@@ -79,7 +78,7 @@ public static class FileHelper
             LbpFileType.Jpeg => true,
             LbpFileType.Png => true,
             #if DEBUG
-            _ => throw new ArgumentOutOfRangeException(nameof(file), $"Unhandled file type ({file.FileType}) in FileHelper.IsFileSafe()"),
+            _ => throw new ArgumentOutOfRangeException(nameof(file), $@"Unhandled file type ({file.FileType}) in FileHelper.IsFileSafe()"),
             #else
             _ => false,
             #endif
@@ -234,14 +233,26 @@ public static class FileHelper
     }
 
     public static bool ResourceExists(string hash) => File.Exists(GetResourcePath(hash));
+    public static bool ImageExists(string hash) => File.Exists(GetImagePath(hash));
 
     public static void DeleteResource(string hash)
     {
+        // Prevent directory traversal attacks
+        if (!Path.GetFullPath(GetResourcePath(hash)).StartsWith(FullResourcePath)) return;
+
         // sanity check so someone doesn't somehow delete the entire resource folder
-        if (ResourceExists(hash) && (File.GetAttributes(hash) & FileAttributes.Directory) != FileAttributes.Directory)
+        if (ResourceExists(hash) && (File.GetAttributes(GetResourcePath(hash)) & FileAttributes.Directory) != FileAttributes.Directory)
         {
             File.Delete(GetResourcePath(hash));
         }
+
+        string imageName = $"{hash}.png";
+        if (!Path.GetFullPath(GetImagePath(imageName)).StartsWith(FullImagePath)) return;
+
+        if (ImageExists(imageName) && (File.GetAttributes(GetImagePath(imageName)) & FileAttributes.Directory) != FileAttributes.Directory)
+        {
+            File.Delete(GetImagePath(imageName));
+        }
     }
 
     public static int ResourceSize(string hash)
@@ -294,39 +305,33 @@ public static class FileHelper
     public static void ConvertAllTexturesToPng()
     {
         EnsureDirectoryCreated(Path.Combine(Environment.CurrentDirectory, "png"));
-        if (Directory.Exists("r"))
+        if (!Directory.Exists("r")) return;
+
+        Logger.Info("Converting all textures to PNG. This may take a while if this is the first time running this operation...", LogArea.Startup);
+
+        ConcurrentQueue<string> fileQueue = new();
+
+        foreach (string filename in Directory.GetFiles("r")) fileQueue.Enqueue(filename);
+
+        List<Task> taskList = new();
+
+        for (int i = 0; i < Environment.ProcessorCount; i++)
         {
-            Logger.Info("Converting all textures to PNG. This may take a while if this is the first time running this operation...", LogArea.Startup);
+            taskList.Add(Task.Factory.StartNew(() =>
-
-            ConcurrentQueue<string> fileQueue = new();
-
-            foreach (string filename in Directory.GetFiles("r")) fileQueue.Enqueue(filename);
-
-            for(int i = 0; i < Environment.ProcessorCount; i++)
             {
-                Task.Factory.StartNew
+                while (fileQueue.TryDequeue(out string? filename))
-                (
+                {
-                    () =>
+                    LbpFile? file = LbpFile.FromHash(filename.Replace("r" + Path.DirectorySeparatorChar, ""));
+
+                    if (file?.FileType is LbpFileType.Jpeg or LbpFileType.Png or LbpFileType.Texture)
                     {
-                        while (fileQueue.TryDequeue(out string? filename))
+                        LbpFileToPNG(file);
-                        {
-                            LbpFile? file = LbpFile.FromHash(filename.Replace("r" + Path.DirectorySeparatorChar, ""));
-                            if (file == null) continue;
-
-                            if (file.FileType == LbpFileType.Jpeg || file.FileType == LbpFileType.Png || file.FileType == LbpFileType.Texture)
-                            {
-                                LbpFileToPNG(file);
-                            }
-                        }
                     }
-                );
+                }
-            }
+            }));
-
-            while (!fileQueue.IsEmpty)
-            {
-                Thread.Sleep(100);
-            }
         }
+
+        Task.WaitAll(taskList.ToArray());
     }
 
     #region Images
@@ -355,8 +360,7 @@ public static class FileHelper
         }
         catch(Exception e)
         {
-            Console.WriteLine($"Error while converting {hash}:");
+            Logger.Error($"Error while converting {type} {hash}: \n{e}", LogArea.Resources);
-            Console.WriteLine(e);
             return false;
         }
     }