From 7119253e6fcb7f66c652a6bebd3aa07b7168e74a Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 1 Mar 2023 15:17:08 +0100 Subject: [PATCH] Add allowlist of media hosts. --- .../Configuration/Models/ContentSettings.cs | 5 ++ .../Controllers/ImagesController.cs | 51 +++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/ContentSettings.cs b/src/Umbraco.Core/Configuration/Models/ContentSettings.cs index 4014930a5c..a7abdf8a48 100644 --- a/src/Umbraco.Core/Configuration/Models/ContentSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/ContentSettings.cs @@ -262,4 +262,9 @@ public class ContentSettings /// [DefaultValue(StaticDisallowedUploadFiles)] public string[] DisallowedUploadedFileExtensions { get; set; } = StaticDisallowedUploadFiles.Split(','); + + /// + /// Gets or sets the allowed external host for media. If empty only relative paths are allowed. + /// + public string[] AllowedMediaHosts { get; set; } = Array.Empty(); } diff --git a/src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs b/src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs index 8f7901b2b4..e718696ae3 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs @@ -1,10 +1,14 @@ using System.Web; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Media; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Web.Common.Attributes; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Web.BackOffice.Controllers; @@ -17,13 +21,31 @@ public class ImagesController : UmbracoAuthorizedApiController { private readonly IImageUrlGenerator _imageUrlGenerator; private readonly MediaFileManager _mediaFileManager; + private ContentSettings _contentSettings; + [Obsolete("Use non obsolete-constructor. Scheduled for removal in Umbraco 13.")] public ImagesController( MediaFileManager mediaFileManager, IImageUrlGenerator imageUrlGenerator) + : this(mediaFileManager, + imageUrlGenerator, + StaticServiceProvider.Instance.GetRequiredService>()) + { + + } + + [ActivatorUtilitiesConstructor] + public ImagesController( + MediaFileManager mediaFileManager, + IImageUrlGenerator imageUrlGenerator, + IOptionsMonitor contentSettingsMonitor) { _mediaFileManager = mediaFileManager; _imageUrlGenerator = imageUrlGenerator; + _contentSettings = contentSettingsMonitor.CurrentValue; + + contentSettingsMonitor.OnChange(x => _contentSettings = x); + } /// @@ -58,7 +80,7 @@ public class ImagesController : UmbracoAuthorizedApiController var ext = Path.GetExtension(encodedImagePath); // check if imagePath is local to prevent open redirect - if (!Uri.IsWellFormedUriString(encodedImagePath, UriKind.Relative)) + if (!IsAllowed(encodedImagePath)) { return Unauthorized(); } @@ -90,12 +112,33 @@ public class ImagesController : UmbracoAuthorizedApiController ImageCropMode = ImageCropMode.Max, CacheBusterValue = rnd }); - if (Url.IsLocalUrl(imageUrl)) + + if (imageUrl is not null) { - return new LocalRedirectResult(imageUrl, false); + return new RedirectResult(imageUrl, false); } - return Unauthorized(); + return NotFound(); + } + + private bool IsAllowed(string encodedImagePath) + { + if(Uri.IsWellFormedUriString(encodedImagePath, UriKind.Relative)) + { + return true; + } + + var builder = new UriBuilder(encodedImagePath); + + foreach (var allowedMediaHost in _contentSettings.AllowedMediaHosts) + { + if (string.Equals(builder.Host, allowedMediaHost, StringComparison.InvariantCultureIgnoreCase)) + { + return true; + } + } + + return false; } ///