From 60f41b80f66cc866c018c58e4567726cb6dac90d Mon Sep 17 00:00:00 2001 From: Bond_009 Date: Tue, 10 Jan 2023 17:02:23 +0100 Subject: [PATCH 1/2] Verify ContentType of uploaded images --- Jellyfin.Api/Controllers/ImageController.cs | 65 ++++++++++++++----- MediaBrowser.Model/Net/MimeTypes.cs | 5 ++ .../Controllers/ImageControllerTests.cs | 36 ++++++++++ 3 files changed, 91 insertions(+), 15 deletions(-) create mode 100644 tests/Jellyfin.Api.Tests/Controllers/ImageControllerTests.cs diff --git a/Jellyfin.Api/Controllers/ImageController.cs b/Jellyfin.Api/Controllers/ImageController.cs index aecdf00dc9..3c5f18af55 100644 --- a/Jellyfin.Api/Controllers/ImageController.cs +++ b/Jellyfin.Api/Controllers/ImageController.cs @@ -91,6 +91,7 @@ public class ImageController : BaseJellyfinApiController [Authorize] [AcceptsImageFile] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [SuppressMessage("Microsoft.Performance", "CA1801:ReviewUnusedParameters", MessageId = "imageType", Justification = "Imported from ServiceStack")] [SuppressMessage("Microsoft.Performance", "CA1801:ReviewUnusedParameters", MessageId = "index", Justification = "Imported from ServiceStack")] @@ -110,6 +111,11 @@ public class ImageController : BaseJellyfinApiController return StatusCode(StatusCodes.Status403Forbidden, "User is not allowed to update the image."); } + if (!TryGetImageExtensionFromContentType(Request.ContentType, out string? extension)) + { + return BadRequest("Incorrect ContentType."); + } + var memoryStream = await GetMemoryStream(Request.Body).ConfigureAwait(false); await using (memoryStream.ConfigureAwait(false)) { @@ -121,7 +127,7 @@ public class ImageController : BaseJellyfinApiController await _userManager.ClearProfileImageAsync(user).ConfigureAwait(false); } - user.ProfileImage = new Data.Entities.ImageInfo(Path.Combine(userDataPath, "profile" + MimeTypes.ToExtension(mimeType ?? string.Empty))); + user.ProfileImage = new Data.Entities.ImageInfo(Path.Combine(userDataPath, "profile" + extension)); await _providerManager .SaveImage(memoryStream, mimeType, user.ProfileImage.Path) @@ -145,6 +151,7 @@ public class ImageController : BaseJellyfinApiController [Authorize] [AcceptsImageFile] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [SuppressMessage("Microsoft.Performance", "CA1801:ReviewUnusedParameters", MessageId = "imageType", Justification = "Imported from ServiceStack")] [SuppressMessage("Microsoft.Performance", "CA1801:ReviewUnusedParameters", MessageId = "index", Justification = "Imported from ServiceStack")] @@ -164,6 +171,11 @@ public class ImageController : BaseJellyfinApiController return StatusCode(StatusCodes.Status403Forbidden, "User is not allowed to update the image."); } + if (!TryGetImageExtensionFromContentType(Request.ContentType, out string? extension)) + { + return BadRequest("Incorrect ContentType."); + } + var memoryStream = await GetMemoryStream(Request.Body).ConfigureAwait(false); await using (memoryStream.ConfigureAwait(false)) { @@ -175,7 +187,7 @@ public class ImageController : BaseJellyfinApiController await _userManager.ClearProfileImageAsync(user).ConfigureAwait(false); } - user.ProfileImage = new Data.Entities.ImageInfo(Path.Combine(userDataPath, "profile" + MimeTypes.ToExtension(mimeType ?? string.Empty))); + user.ProfileImage = new Data.Entities.ImageInfo(Path.Combine(userDataPath, "profile" + extension)); await _providerManager .SaveImage(memoryStream, mimeType, user.ProfileImage.Path) @@ -342,6 +354,7 @@ public class ImageController : BaseJellyfinApiController [Authorize(Policy = Policies.RequiresElevation)] [AcceptsImageFile] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] [SuppressMessage("Microsoft.Performance", "CA1801:ReviewUnusedParameters", MessageId = "index", Justification = "Imported from ServiceStack")] public async Task SetItemImage( @@ -354,6 +367,11 @@ public class ImageController : BaseJellyfinApiController return NotFound(); } + if (!TryGetImageExtensionFromContentType(Request.ContentType, out _)) + { + return BadRequest("Incorrect ContentType."); + } + var memoryStream = await GetMemoryStream(Request.Body).ConfigureAwait(false); await using (memoryStream.ConfigureAwait(false)) { @@ -379,6 +397,7 @@ public class ImageController : BaseJellyfinApiController [Authorize(Policy = Policies.RequiresElevation)] [AcceptsImageFile] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] [SuppressMessage("Microsoft.Performance", "CA1801:ReviewUnusedParameters", MessageId = "index", Justification = "Imported from ServiceStack")] public async Task SetItemImageByIndex( @@ -392,6 +411,11 @@ public class ImageController : BaseJellyfinApiController return NotFound(); } + if (!TryGetImageExtensionFromContentType(Request.ContentType, out _)) + { + return BadRequest("Incorrect ContentType."); + } + var memoryStream = await GetMemoryStream(Request.Body).ConfigureAwait(false); await using (memoryStream.ConfigureAwait(false)) { @@ -1763,22 +1787,14 @@ public class ImageController : BaseJellyfinApiController [AcceptsImageFile] public async Task UploadCustomSplashscreen() { + if (!TryGetImageExtensionFromContentType(Request.ContentType, out var extension)) + { + return BadRequest("Incorrect ContentType."); + } + var memoryStream = await GetMemoryStream(Request.Body).ConfigureAwait(false); await using (memoryStream.ConfigureAwait(false)) { - var mimeType = MediaTypeHeaderValue.Parse(Request.ContentType).MediaType; - - if (!mimeType.HasValue) - { - return BadRequest("Error reading mimetype from uploaded image"); - } - - var extension = MimeTypes.ToExtension(mimeType.Value); - if (string.IsNullOrEmpty(extension)) - { - return BadRequest("Error converting mimetype to an image extension"); - } - var filePath = Path.Combine(_appPaths.DataPath, "splashscreen-upload" + extension); var brandingOptions = _serverConfigurationManager.GetConfiguration("branding"); brandingOptions.SplashscreenLocation = filePath; @@ -2106,4 +2122,23 @@ public class ImageController : BaseJellyfinApiController return PhysicalFile(imagePath, imageContentType ?? MediaTypeNames.Text.Plain); } + + internal static bool TryGetImageExtensionFromContentType(string? contentType, [NotNullWhen(true)] out string? extension) + { + extension = null; + if (string.IsNullOrEmpty(contentType)) + { + return false; + } + + if (MediaTypeHeaderValue.TryParse(contentType, out var parsedValue) + && parsedValue.MediaType.HasValue + && MimeTypes.IsImage(parsedValue.MediaType.Value)) + { + extension = MimeTypes.ToExtension(parsedValue.MediaType.Value); + return extension is not null; + } + + return false; + } } diff --git a/MediaBrowser.Model/Net/MimeTypes.cs b/MediaBrowser.Model/Net/MimeTypes.cs index 8157dc0c24..5a1871070d 100644 --- a/MediaBrowser.Model/Net/MimeTypes.cs +++ b/MediaBrowser.Model/Net/MimeTypes.cs @@ -117,7 +117,9 @@ namespace MediaBrowser.Model.Net // Type image { "image/jpeg", ".jpg" }, + { "image/tiff", ".tiff" }, { "image/x-png", ".png" }, + { "image/x-icon", ".ico" }, // Type text { "text/plain", ".txt" }, @@ -178,5 +180,8 @@ namespace MediaBrowser.Model.Net var extension = Model.MimeTypes.GetMimeTypeExtensions(mimeType).FirstOrDefault(); return string.IsNullOrEmpty(extension) ? null : "." + extension; } + + public static bool IsImage(ReadOnlySpan mimeType) + => mimeType.StartsWith("image/", StringComparison.OrdinalIgnoreCase); } } diff --git a/tests/Jellyfin.Api.Tests/Controllers/ImageControllerTests.cs b/tests/Jellyfin.Api.Tests/Controllers/ImageControllerTests.cs new file mode 100644 index 0000000000..d6428fb2cd --- /dev/null +++ b/tests/Jellyfin.Api.Tests/Controllers/ImageControllerTests.cs @@ -0,0 +1,36 @@ +using System; +using Jellyfin.Api.Controllers; +using Xunit; + +namespace Jellyfin.Api.Tests.Controllers; + +public static class ImageControllerTests +{ + [Theory] + [InlineData("image/apng", ".apng")] + [InlineData("image/avif", ".avif")] + [InlineData("image/bmp", ".bmp")] + [InlineData("image/gif", ".gif")] + [InlineData("image/x-icon", ".ico")] + [InlineData("image/jpeg", ".jpg")] + [InlineData("image/png", ".png")] + [InlineData("image/png; charset=utf-8", ".png")] + [InlineData("image/svg+xml", ".svg")] + [InlineData("image/tiff", ".tiff")] + [InlineData("image/webp", ".webp")] + public static void TryGetImageExtensionFromContentType_Valid_True(string contentType, string extension) + { + Assert.True(ImageController.TryGetImageExtensionFromContentType(contentType, out var ex)); + Assert.Equal(extension, ex); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("text/html")] + public static void TryGetImageExtensionFromContentType_InValid_False(string contentType) + { + Assert.False(ImageController.TryGetImageExtensionFromContentType(contentType, out var ex)); + Assert.Null(ex); + } +} From a38cb3ade8f3dc50e1a5d968c6b6ac68306bc5bb Mon Sep 17 00:00:00 2001 From: Bond_009 Date: Tue, 10 Jan 2023 17:15:21 +0100 Subject: [PATCH 2/2] Fix tests --- tests/Jellyfin.Model.Tests/Net/MimeTypesTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Jellyfin.Model.Tests/Net/MimeTypesTests.cs b/tests/Jellyfin.Model.Tests/Net/MimeTypesTests.cs index cbab455f01..371c3811ab 100644 --- a/tests/Jellyfin.Model.Tests/Net/MimeTypesTests.cs +++ b/tests/Jellyfin.Model.Tests/Net/MimeTypesTests.cs @@ -127,9 +127,10 @@ namespace Jellyfin.Model.Tests.Net [InlineData("image/jpeg", ".jpg")] [InlineData("image/png", ".png")] [InlineData("image/svg+xml", ".svg")] - [InlineData("image/tiff", ".tif")] + [InlineData("image/tiff", ".tiff")] [InlineData("image/vnd.microsoft.icon", ".ico")] [InlineData("image/webp", ".webp")] + [InlineData("image/x-icon", ".ico")] [InlineData("image/x-png", ".png")] [InlineData("text/css", ".css")] [InlineData("text/csv", ".csv")]