Merge commit from fork

* Prevent path traveral vulnerability with upload of temporary files.

* Used BadRequest instead of NotFound for invalid file name response.

Author: AndyButland

src/Umbraco.Cms.Api.Management/Controllers/TemporaryFile/TemporaryFileControllerBase.cs

@@ -1,4 +1,4 @@
-using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using Umbraco.Cms.Api.Common.Builders;
 using Umbraco.Cms.Api.Management.Routing;
@@ -17,6 +17,9 @@ protected IActionResult TemporaryFileStatusResult(TemporaryFileOperationStatus o
                 .WithTitle("File extension not allowed")
                 .WithDetail("The file extension is not allowed.")
                 .Build()),
+            TemporaryFileOperationStatus.InvalidFileName => BadRequest(problemDetailsBuilder
+                .WithTitle("The provided file name is not valid")
+                .Build()),
             TemporaryFileOperationStatus.KeyAlreadyUsed => BadRequest(problemDetailsBuilder
                 .WithTitle("Key already used")
                 .WithDetail("The specified key is already used.")

src/Umbraco.Core/Services/OperationStatus/TemporaryFileUploadStatus.cs

@@ -6,5 +6,6 @@ public enum TemporaryFileOperationStatus
     FileExtensionNotAllowed = 1,
     KeyAlreadyUsed = 2,
     NotFound = 3,
-    UploadBlocked
+    UploadBlocked = 4,
+    InvalidFileName = 5,
 }

src/Umbraco.Core/Services/TemporaryFileService.cs

@@ -45,21 +45,19 @@ public TemporaryFileService(
             return Attempt.FailWithStatus<TemporaryFileModel?, TemporaryFileOperationStatus>(TemporaryFileOperationStatus.KeyAlreadyUsed, null);
         }
 
-
         await using Stream dataStream = createModel.OpenReadStream();
         dataStream.Seek(0, SeekOrigin.Begin);
         if (_fileStreamSecurityValidator.IsConsideredSafe(dataStream) is false)
         {
             return Attempt.FailWithStatus<TemporaryFileModel?, TemporaryFileOperationStatus>(TemporaryFileOperationStatus.UploadBlocked, null);
         }
 
-
         temporaryFileModel = new TemporaryFileModel
         {
             Key = createModel.Key,
             FileName = createModel.FileName,
             OpenReadStream = createModel.OpenReadStream,
-            AvailableUntil = DateTime.Now.Add(_runtimeSettings.TemporaryFileLifeTime)
+            AvailableUntil = DateTime.Now.Add(_runtimeSettings.TemporaryFileLifeTime),
         };
 
         await _temporaryFileRepository.SaveAsync(temporaryFileModel);
@@ -68,17 +66,29 @@ public TemporaryFileService(
     }
 
     private TemporaryFileOperationStatus Validate(TemporaryFileModelBase temporaryFileModel)
-        => IsAllowedFileExtension(temporaryFileModel) == false
-            ? TemporaryFileOperationStatus.FileExtensionNotAllowed
-            : TemporaryFileOperationStatus.Success;
-
-    private bool IsAllowedFileExtension(TemporaryFileModelBase temporaryFileModel)
     {
-        var extension = Path.GetExtension(temporaryFileModel.FileName)[1..];
+        if (IsAllowedFileExtension(temporaryFileModel.FileName) == false)
+        {
+            return TemporaryFileOperationStatus.FileExtensionNotAllowed;
+        }
+
+        if (IsValidFileName(temporaryFileModel.FileName) == false)
+        {
+            return TemporaryFileOperationStatus.InvalidFileName;
+        }
+
+        return TemporaryFileOperationStatus.Success;
+    }
 
+    private bool IsAllowedFileExtension(string fileName)
+    {
+        var extension = Path.GetExtension(fileName)[1..];
         return _contentSettings.IsFileAllowedForUpload(extension);
     }
 
+    private static bool IsValidFileName(string fileName) =>
+        !string.IsNullOrEmpty(fileName) && fileName.IndexOfAny(Path.GetInvalidFileNameChars()) < 0;
+
     public async Task<Attempt<TemporaryFileModel?, TemporaryFileOperationStatus>> DeleteAsync(Guid key)
     {
         TemporaryFileModel? model = await _temporaryFileRepository.GetAsync(key);

tests/Umbraco.Tests.Integration/Umbraco.Core/Services/TemporaryFileServiceTests.cs

@@ -0,0 +1,91 @@
+using Microsoft.Extensions.DependencyInjection;
+using NUnit.Framework;
+using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.Models.TemporaryFile;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Core.Services.OperationStatus;
+using Umbraco.Cms.Tests.Common.Testing;
+using Umbraco.Cms.Tests.Integration.Attributes;
+using Umbraco.Cms.Tests.Integration.Testing;
+
+namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Services;
+
+[TestFixture]
+[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerFixture)]
+public class TemporaryFileServiceTests : UmbracoIntegrationTest
+{
+    private ITemporaryFileService TemporaryFileService => GetRequiredService<ITemporaryFileService>();
+
+    public static void ConfigureAllowedUploadedFileExtensions(IUmbracoBuilder builder)
+    {
+        builder.Services.Configure<ContentSettings>(config =>
+            config.AllowedUploadedFileExtensions = ["txt"]);
+    }
+
+    [Test]
+    [ConfigureBuilder(ActionName = nameof(ConfigureAllowedUploadedFileExtensions))]
+    public async Task Can_Create_Get_And_Delete_Temporary_File()
+    {
+        var key = Guid.NewGuid();
+        const string FileName = "test.txt";
+        const string FileContents = "test";
+        var model = new CreateTemporaryFileModel
+        {
+            FileName = FileName,
+            Key = key,
+            OpenReadStream = () =>
+            {
+                var stream = new MemoryStream();
+                var writer = new StreamWriter(stream);
+                writer.Write(FileContents);
+                writer.Flush();
+                stream.Position = 0;
+                return stream;
+            }
+        };
+        var createAttempt = await TemporaryFileService.CreateAsync(model);
+        Assert.IsTrue(createAttempt.Success);
+
+        TemporaryFileModel? fileModel = await TemporaryFileService.GetAsync(key);
+        Assert.IsNotNull(fileModel);
+        Assert.AreEqual(key, fileModel.Key);
+        Assert.AreEqual(FileName, fileModel.FileName);
+
+        using (var reader = new StreamReader(fileModel.OpenReadStream()))
+        {
+            string fileContents = reader.ReadToEnd();
+            Assert.AreEqual(FileContents, fileContents);
+        }
+
+        var deleteAttempt = await TemporaryFileService.DeleteAsync(key);
+        Assert.IsTrue(createAttempt.Success);
+
+        fileModel = await TemporaryFileService.GetAsync(key);
+        Assert.IsNull(fileModel);
+    }
+
+    [Test]
+    [ConfigureBuilder(ActionName = nameof(ConfigureAllowedUploadedFileExtensions))]
+    public async Task Cannot_Create_File_Outside_Of_Temporary_Files_Root()
+    {
+        var key = Guid.NewGuid();
+        const string FileName = "../test.txt";
+        var model = new CreateTemporaryFileModel
+        {
+            FileName = FileName,
+            Key = key,
+            OpenReadStream = () =>
+            {
+                var stream = new MemoryStream();
+                var writer = new StreamWriter(stream);
+                writer.Write(string.Empty);
+                writer.Flush();
+                stream.Position = 0;
+                return stream;
+            }
+        };
+        var createAttempt = await TemporaryFileService.CreateAsync(model);
+        Assert.IsFalse(createAttempt.Success);
+        Assert.AreEqual(TemporaryFileOperationStatus.InvalidFileName, createAttempt.Status);
+    }
+}