From 417d43788b1ee5c45a1d2b890e6d5da97d7ba544 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 30 Jul 2025 08:34:54 +0200 Subject: [PATCH 1/2] Use a regex to filter our invalid culture codes rather than relying on the culture being installed on the operating system. --- .../Controllers/PreviewController.cs | 30 ++++++++++------ .../Controllers/PreviewControllerTests.cs | 34 +++++++++++++++++++ 2 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/PreviewControllerTests.cs diff --git a/src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs b/src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs index 3af0628313b7..112e11d6ef1d 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs @@ -178,26 +178,31 @@ public ActionResult Frame(int id, string culture) return RedirectPermanent($"../../{id}{query}"); } - private static bool ValidateProvidedCulture(string culture) + /// + /// Validates the provided culture code. + /// + /// + /// Marked as internal to expose for unit tests. + /// + internal static bool ValidateProvidedCulture(string culture) { if (string.IsNullOrEmpty(culture)) { return true; } - // We can be confident the backoffice will have provided a valid culture in linking to the - // preview, so we don't need to check that the culture matches an Umbraco language. - // We are only concerned here with protecting against XSS attacks from a fiddled preview - // URL, so we can just confirm we have a valid culture. - try - { - CultureInfo.GetCultureInfo(culture, true); - return true; - } - catch (CultureNotFoundException) + // Culture codes are expected to match this pattern. + if (CultureCodeRegex().IsMatch(culture) is false) { return false; } + + // We can be confident the backoffice will have provided a valid culture in linking to the + // preview, so we don't need to check that the culture matches an Umbraco language (or is even a + // valid culture code). + // We are only concerned here with protecting against XSS attacks from a fiddled preview + // URL, so we can proceed if the the regex is matched. + return true; } public ActionResult? EnterPreview(int id) @@ -261,4 +266,7 @@ public ActionResult End(string? redir = null) [GeneratedRegex("^\\/(?\\d*)(\\?culture=(?[\\w-]*))?$")] private static partial Regex DefaultPreviewRedirectRegex(); + + [GeneratedRegex(@"^[a-zA-Z0-9-]+$")] + private static partial Regex CultureCodeRegex(); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/PreviewControllerTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/PreviewControllerTests.cs new file mode 100644 index 000000000000..9e8c151de167 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/PreviewControllerTests.cs @@ -0,0 +1,34 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System.Globalization; +using NUnit.Framework; +using Umbraco.Cms.Web.BackOffice.Controllers; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers; + +[TestFixture] +public class PreviewControllerTests +{ + [TestCase("en-US", true)] // A framework culture. + [TestCase("en-JP", true)] // A valid culture string, but not one that's in the framework. + [TestCase("a!", false)] // Not a valid culture string. + [TestCase("", false)] + public void ValidateProvidedCulture_Validates_Culture(string culture, bool expectValid) + { + var result = PreviewController.ValidateProvidedCulture(culture); + Assert.AreEqual(expectValid, result); + } + + [Test] + public void ValidateProvidedCulture_Validates_Culture_For_All_Framework_Cultures() + { + var cultures = CultureInfo.GetCultures(CultureTypes.AllCultures); + foreach (var culture in cultures) + { + Assert.IsTrue(PreviewController.ValidateProvidedCulture(culture.Name), $"{culture.Name} is not considered a valid culture."); + Assert.IsTrue(PreviewController.ValidateProvidedCulture(culture.Name.ToUpperInvariant()), $"{culture.Name.ToUpperInvariant()} is not considered a valid culture."); + Assert.IsTrue(PreviewController.ValidateProvidedCulture(culture.Name.ToLowerInvariant()), $"{culture.Name.ToLowerInvariant()} is not considered a valid culture."); + } + } +} From 9a1000ee09fb5fa8e706121d89508803dc532236 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Thu, 31 Jul 2025 06:44:05 +0200 Subject: [PATCH 2/2] Update to more restrictive regex Co-authored-by: Nuklon --- src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs b/src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs index 112e11d6ef1d..3610b02adcce 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/PreviewController.cs @@ -267,6 +267,6 @@ public ActionResult End(string? redir = null) [GeneratedRegex("^\\/(?\\d*)(\\?culture=(?[\\w-]*))?$")] private static partial Regex DefaultPreviewRedirectRegex(); - [GeneratedRegex(@"^[a-zA-Z0-9-]+$")] + [GeneratedRegex(@"^[a-z]{2,3}[-0-9a-z]*$", RegexOptions.IgnoreCase)] private static partial Regex CultureCodeRegex(); }