v10: Add HMAC image processing protection

nuget.config

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<configuration>
+  <packageSources>
+    <add key="SixLabors" value="https://www.myget.org/F/sixlabors/api/v3/index.json" />
+  </packageSources>
+</configuration>

src/Umbraco.Core/Configuration/Models/ImagingSettings.cs

@@ -4,15 +4,20 @@
 namespace Umbraco.Cms.Core.Configuration.Models;
 
 /// <summary>
-///     Typed configuration options for imaging settings.
+/// Typed configuration options for imaging settings.
 /// </summary>
 [UmbracoOptions(Constants.Configuration.ConfigImaging)]
 public class ImagingSettings
 {
     /// <summary>
-    ///     Gets or sets a value for imaging cache settings.
+    /// Gets or sets a value for the Hash-based Message Authentication Code (HMAC) secret key for request authentication.
     /// </summary>
-    public ImagingCacheSettings Cache { get; set; } = new();
+    public byte[]? HMACSecretKey { get; set; }
+
+    /// <summary>
+    /// Gets or sets a value for imaging cache settings.
+    /// </summary>
+    public ImagingCacheSettings Cache { get; set; } = new ImagingCacheSettings();
 
     /// <summary>
     ///     Gets or sets a value for imaging resize settings.

src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs

@@ -1,3 +1,4 @@
+using System.Globalization;
 using System.Web;
 using Microsoft.AspNetCore.Mvc;
 using Umbraco.Cms.Core;
@@ -10,13 +11,13 @@
 namespace Umbraco.Cms.Web.BackOffice.Controllers;
 
 /// <summary>
-///     A controller used to return images for media
+/// A controller used to return images for media.
 /// </summary>
 [PluginController(Constants.Web.Mvc.BackOfficeApiArea)]
 public class ImagesController : UmbracoAuthorizedApiController
 {
-    private readonly IImageUrlGenerator _imageUrlGenerator;
     private readonly MediaFileManager _mediaFileManager;
+    private readonly IImageUrlGenerator _imageUrlGenerator;
 
     public ImagesController(
         MediaFileManager mediaFileManager,
@@ -27,34 +28,33 @@ public ImagesController(
     }
 
     /// <summary>
-    ///     Gets the big thumbnail image for the original image path
+    /// Gets the big thumbnail image for the original image path.
     /// </summary>
     /// <param name="originalImagePath"></param>
     /// <returns></returns>
     /// <remarks>
-    ///     If there is no original image is found then this will return not found.
+    /// If there is no original image is found then this will return not found.
     /// </remarks>
-    public IActionResult GetBigThumbnail(string originalImagePath) =>
-        string.IsNullOrWhiteSpace(originalImagePath)
-            ? Ok()
-            : GetResized(originalImagePath, 500);
+    public IActionResult GetBigThumbnail(string originalImagePath)
+        => string.IsNullOrWhiteSpace(originalImagePath)
+        ? Ok()
+        : GetResized(originalImagePath, 500);
 
     /// <summary>
-    ///     Gets a resized image for the image at the given path
+    /// Gets a resized image for the image at the given path.
     /// </summary>
     /// <param name="imagePath"></param>
     /// <param name="width"></param>
     /// <returns></returns>
     /// <remarks>
-    ///     If there is no media, image property or image file is found then this will return not found.
+    /// If there is no media, image property or image file is found then this will return not found.
     /// </remarks>
     public IActionResult GetResized(string imagePath, int width)
     {
         // We have to use HttpUtility to encode the path here, for non-ASCII characters
         // We cannot use the WebUtility, as we only want to encode the path, and not the entire string
         var encodedImagePath = HttpUtility.UrlPathEncode(imagePath);
 
-
         var ext = Path.GetExtension(encodedImagePath);
 
         // check if imagePath is local to prevent open redirect
@@ -69,52 +69,50 @@ public IActionResult GetResized(string imagePath, int width)
             return NotFound();
         }
 
-        // redirect to ImageProcessor thumbnail with rnd generated from last modified time of original media file
+        // Redirect to thumbnail with cache buster value generated from last modified time of original media file
         DateTimeOffset? imageLastModified = null;
         try
         {
             imageLastModified = _mediaFileManager.FileSystem.GetLastModified(imagePath);
         }
-        catch (Exception)
+        catch
         {
             // if we get an exception here it's probably because the image path being requested is an image that doesn't exist
             // in the local media file system. This can happen if someone is storing an absolute path to an image online, which
             // is perfectly legal but in that case the media file system isn't going to resolve it.
             // so ignore and we won't set a last modified date.
         }
 
-        var rnd = imageLastModified.HasValue ? $"&rnd={imageLastModified:yyyyMMddHHmmss}" : null;
+        var cacheBusterValue = imageLastModified.HasValue ? imageLastModified.Value.ToFileTime().ToString("x", CultureInfo.InvariantCulture) : null;
         var imageUrl = _imageUrlGenerator.GetImageUrl(new ImageUrlGenerationOptions(encodedImagePath)
         {
             Width = width,
             ImageCropMode = ImageCropMode.Max,
-            CacheBusterValue = rnd
+            CacheBusterValue = cacheBusterValue
         });
+
         if (Url.IsLocalUrl(imageUrl))
         {
             return new LocalRedirectResult(imageUrl, false);
         }
-
-        return Unauthorized();
+        else
+        {
+            return Unauthorized();
+        }
     }
 
     /// <summary>
-    ///     Gets a processed image for the image at the given path
+    /// Gets a processed image for the image at the given path
     /// </summary>
     /// <param name="imagePath"></param>
     /// <param name="width"></param>
     /// <param name="height"></param>
     /// <param name="focalPointLeft"></param>
     /// <param name="focalPointTop"></param>
     /// <param name="mode"></param>
-    /// <param name="cacheBusterValue"></param>
-    /// <param name="cropX1"></param>
-    /// <param name="cropX2"></param>
-    /// <param name="cropY1"></param>
-    /// <param name="cropY2"></param>
     /// <returns></returns>
     /// <remarks>
-    ///     If there is no media, image property or image file is found then this will return not found.
+    /// If there is no media, image property or image file is found then this will return not found.
     /// </remarks>
     public string? GetProcessedImageUrl(
         string imagePath,
@@ -123,7 +121,7 @@ public IActionResult GetResized(string imagePath, int width)
         decimal? focalPointLeft = null,
         decimal? focalPointTop = null,
         ImageCropMode mode = ImageCropMode.Max,
-        string cacheBusterValue = "",
+        string cacheBusterValue = "", // TODO Change to: string? cacheBusterValue = null
         decimal? cropX1 = null,
         decimal? cropX2 = null,
         decimal? cropY1 = null,
@@ -139,13 +137,11 @@ public IActionResult GetResized(string imagePath, int width)
 
         if (focalPointLeft.HasValue && focalPointTop.HasValue)
         {
-            options.FocalPoint =
-                new ImageUrlGenerationOptions.FocalPointPosition(focalPointLeft.Value, focalPointTop.Value);
+            options.FocalPoint = new ImageUrlGenerationOptions.FocalPointPosition(focalPointLeft.Value, focalPointTop.Value);
         }
         else if (cropX1.HasValue && cropX2.HasValue && cropY1.HasValue && cropY2.HasValue)
         {
-            options.Crop =
-                new ImageUrlGenerationOptions.CropCoordinates(cropX1.Value, cropY1.Value, cropX2.Value, cropY2.Value);
+            options.Crop = new ImageUrlGenerationOptions.CropCoordinates(cropX1.Value, cropY1.Value, cropX2.Value, cropY2.Value);
         }
 
         return _imageUrlGenerator.GetImageUrl(options);

src/Umbraco.Web.Common/DependencyInjection/ConfigureImageSharpMiddlewareOptions.cs

@@ -1,17 +1,20 @@
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Extensions;
 using Microsoft.AspNetCore.Http.Headers;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Options;
 using Microsoft.Net.Http.Headers;
 using SixLabors.ImageSharp;
 using SixLabors.ImageSharp.Web.Commands;
 using SixLabors.ImageSharp.Web.Middleware;
 using SixLabors.ImageSharp.Web.Processors;
 using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.Media;
 
 namespace Umbraco.Cms.Web.Common.DependencyInjection;
 
 /// <summary>
-///     Configures the ImageSharp middleware options.
+/// Configures the ImageSharp middleware options.
 /// </summary>
 /// <seealso cref="IConfigureOptions{ImageSharpMiddlewareOptions}" />
 public sealed class ConfigureImageSharpMiddlewareOptions : IConfigureOptions<ImageSharpMiddlewareOptions>
@@ -20,7 +23,7 @@ public sealed class ConfigureImageSharpMiddlewareOptions : IConfigureOptions<Ima
     private readonly ImagingSettings _imagingSettings;
 
     /// <summary>
-    ///     Initializes a new instance of the <see cref="ConfigureImageSharpMiddlewareOptions" /> class.
+    /// Initializes a new instance of the <see cref="ConfigureImageSharpMiddlewareOptions" /> class.
     /// </summary>
     /// <param name="configuration">The ImageSharp configuration.</param>
     /// <param name="imagingSettings">The Umbraco imaging settings.</param>
@@ -35,29 +38,27 @@ public void Configure(ImageSharpMiddlewareOptions options)
     {
         options.Configuration = _configuration;
 
+        options.HMACSecretKey = _imagingSettings.HMACSecretKey;
         options.BrowserMaxAge = _imagingSettings.Cache.BrowserMaxAge;
         options.CacheMaxAge = _imagingSettings.Cache.CacheMaxAge;
         options.CacheHashLength = _imagingSettings.Cache.CacheHashLength;
 
         // Use configurable maximum width and height
         options.OnParseCommandsAsync = context =>
         {
-            if (context.Commands.Count == 0)
+            if (context.Commands.Count == 0 || _imagingSettings.HMACSecretKey?.Length > 0)
             {
+                // Nothing to parse or using HMAC authentication
                 return Task.CompletedTask;
             }
 
-            var width = context.Parser.ParseValue<int>(
-                context.Commands.GetValueOrDefault(ResizeWebProcessor.Width),
-                context.Culture);
+            int width = context.Parser.ParseValue<int>(context.Commands.GetValueOrDefault(ResizeWebProcessor.Width), context.Culture);
             if (width <= 0 || width > _imagingSettings.Resize.MaxWidth)
             {
                 context.Commands.Remove(ResizeWebProcessor.Width);
             }
 
-            var height = context.Parser.ParseValue<int>(
-                context.Commands.GetValueOrDefault(ResizeWebProcessor.Height),
-                context.Culture);
+            int height = context.Parser.ParseValue<int>(context.Commands.GetValueOrDefault(ResizeWebProcessor.Height), context.Culture);
             if (height <= 0 || height > _imagingSettings.Resize.MaxHeight)
             {
                 context.Commands.Remove(ResizeWebProcessor.Height);
@@ -73,11 +74,16 @@ public void Configure(ImageSharpMiddlewareOptions options)
             {
                 ResponseHeaders headers = context.Response.GetTypedHeaders();
 
-                CacheControlHeaderValue cacheControl =
-                    headers.CacheControl ?? new CacheControlHeaderValue { Public = true };
-                cacheControl.MustRevalidate = false; // ImageSharp enables this by default
+                CacheControlHeaderValue cacheControl = headers.CacheControl ?? new CacheControlHeaderValue()
+                {
+                    Public = true
+                };
+
+                // ImageSharp enables cache revalidation by default, so disable and add immutable directive
+                cacheControl.MustRevalidate = false;
                 cacheControl.Extensions.Add(new NameValueHeaderValue("immutable"));
 
+                // Set updated value
                 headers.CacheControl = cacheControl;
             }
 

src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilder.ImageSharp.cs

@@ -15,28 +15,23 @@ namespace Umbraco.Extensions;
 public static partial class UmbracoBuilderExtensions
 {
     /// <summary>
-    ///     Adds Image Sharp with Umbraco settings
+    /// Adds ImageSharp with Umbraco settings.
     /// </summary>
     public static IServiceCollection AddUmbracoImageSharp(this IUmbracoBuilder builder)
     {
         builder.Services.AddSingleton<IImageUrlGenerator, ImageSharpImageUrlGenerator>();
 
+        // Add ImageSharp, replace default image provider and add custom processors
         builder.Services.AddImageSharp()
-
-            // Replace default image provider
             .ClearProviders()
             .AddProvider<WebRootImageProvider>()
-
-            // Add custom processors
             .AddProcessor<CropWebProcessor>();
 
         // Configure middleware
-        builder.Services
-            .AddTransient<IConfigureOptions<ImageSharpMiddlewareOptions>, ConfigureImageSharpMiddlewareOptions>();
+        builder.Services.AddTransient<IConfigureOptions<ImageSharpMiddlewareOptions>, ConfigureImageSharpMiddlewareOptions>();
 
         // Configure cache options
-        builder.Services
-            .AddTransient<IConfigureOptions<PhysicalFileSystemCacheOptions>, ConfigurePhysicalFileSystemCacheOptions>();
+        builder.Services.AddTransient<IConfigureOptions<PhysicalFileSystemCacheOptions>, ConfigurePhysicalFileSystemCacheOptions>();
 
         return builder.Services;
     }

src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs

@@ -558,7 +558,7 @@ public static class ImageCropperTemplateCoreExtensions
         }
 
         var cacheBusterValue =
-            cacheBuster ? mediaItem.UpdateDate.ToFileTimeUtc().ToString(CultureInfo.InvariantCulture) : null;
+            cacheBuster ? mediaItem.UpdateDate.ToFileTimeUtc().ToString("x", CultureInfo.InvariantCulture) : null;
 
         return GetCropUrl(
             mediaItemUrl,