Security: Replace NoScripts blacklist with allowlist HTML sanitization

Directory.Packages.props

@@ -5,6 +5,7 @@
   <ItemGroup>
     <PackageVersion Include="Azure.Monitor.OpenTelemetry.AspNetCore" Version="1.6.0" />
     <PackageVersion Include="ExcelMapper" Version="6.0.641" />
+    <PackageVersion Include="HtmlSanitizer" Version="9.2.995" />
     <PackageVersion Include="MessagePack" Version="3.1.8" />
     <PackageVersion Include="Microsoft.AspNetCore.Http" Version="2.3.11" />
     <PackageVersion Include="Microsoft.AspNetCore.OpenApi" Version="10.0.10" />

src/Core/Grand.Infrastructure/Configuration/SecurityConfig.cs

@@ -93,4 +93,21 @@ public class SecurityConfig
     ///     The value is embedded in each stored hash, so raising it later does not break existing hashes.
     /// </summary>
     public int PasswordHashIterations { get; set; }
+
+    /// <summary>
+    ///     Hosts whose iframes survive HTML sanitization of rich-text content (product descriptions, blog posts, pages).
+    ///     An iframe pointing anywhere else is removed, because its src is otherwise attacker-controlled.
+    ///     Matching is case-insensitive on the host only; a leading "*." matches any subdomain.
+    ///     Leave empty to fall back to the built-in video-embed defaults; set to a single empty entry to block every iframe.
+    /// </summary>
+    public string[] SanitizerAllowedIframeHosts { get; set; }
+
+    /// <summary>
+    ///     Gets or sets a value indicating whether [SanitizeHtml] and [NoHtml] reject markup on save. Default true.
+    ///     This is an operational escape hatch, not a security setting: turn it off only temporarily, if the
+    ///     allowlist is found to reject legitimate content in production, while a fix is prepared - every field
+    ///     these attributes guard (Vendor/Store-manager-editable rich text rendered unencoded via Html.Raw/v-html)
+    ///     goes back to accepting raw, unsanitized HTML the moment this is false. Re-enable as soon as possible.
+    /// </summary>
+    public bool EnableHtmlSanitization { get; set; } = true;
 }

src/Core/Grand.Infrastructure/Grand.Infrastructure.csproj

@@ -3,6 +3,7 @@
   <ItemGroup>
     <PackageReference Include="FluentValidation.DependencyInjectionExtensions" />
     <PackageReference Include="GoogleAuthenticator" />
+    <PackageReference Include="HtmlSanitizer" />
     <PackageReference Include="Microsoft.Azure.AppConfiguration.AspNetCore" />
     <PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" />
     <PackageReference Include="Microsoft.Extensions.Caching.Memory" />

src/Core/Grand.Infrastructure/Security/HtmlSanitizationService.cs

@@ -0,0 +1,243 @@
+using Ganss.Xss;
+using Grand.Infrastructure.Configuration;
+
+namespace Grand.Infrastructure.Security;
+
+/// <summary>
+///     Allowlist-based implementation over HtmlSanitizer. Sanitizes nothing itself - it runs the library's
+///     allowlist and reports whether anything would have been removed, which is all a rejecting
+///     <see cref="System.ComponentModel.DataAnnotations.ValidationAttribute" /> needs.
+///     Registered as a singleton, but holds no mutable shared state: the allowlists below (<see cref="ISet{T}" />
+///     instances returned by a throwaway HtmlSanitizer during construction) are built once and never mutated
+///     afterwards, so concurrent calls can read them freely. Each detection call builds its own short-lived
+///     HtmlSanitizer around those shared, read-only allowlists, with event handlers that close over a local
+///     "was anything removed" flag - so two concurrent calls can never observe or overwrite each other's result,
+///     and no lock is needed.
+///     <see cref="SecurityConfig.EnableHtmlSanitization" /> is an operational kill switch, read once at
+///     construction: when false, both detection methods report "nothing disallowed" unconditionally, so
+///     [SanitizeHtml]/[NoHtml] accept any input. Off by exception only - see that property's remarks.
+/// </summary>
+public class HtmlSanitizationService : IHtmlSanitizationService
+{
+    /// <summary>
+    ///     Hosts allowed to be framed when the configuration does not name any. These are the embed hosts a store
+    ///     realistically pastes into a page body; anything else has to be opted into explicitly.
+    /// </summary>
+    private static readonly string[] DefaultAllowedIframeHosts =
+    [
+        "youtube.com", "*.youtube.com",
+        "youtube-nocookie.com", "*.youtube-nocookie.com",
+        "vimeo.com", "*.vimeo.com",
+        "google.com", "*.google.com"
+    ];
+
+    /// <summary>
+    ///     Tags that are in the library defaults but have no place in store content: they either collect input or
+    ///     submit it somewhere. A form rendered inside the admin panel is a credible phishing surface.
+    /// </summary>
+    private static readonly string[] InteractiveTags =
+    [
+        "form", "input", "button", "textarea", "select", "option", "optgroup", "label", "fieldset", "legend",
+        "datalist", "output", "progress", "meter"
+    ];
+
+    private readonly string[] _allowedIframeHosts;
+    private readonly bool _enabled;
+    private readonly HtmlSanitizerOptions _richTextOptions;
+
+    public HtmlSanitizationService(SecurityConfig securityConfig)
+    {
+        //defaults to enabled: a missing SecurityConfig or a config binder that never touched this property
+        //must fail closed (sanitize), not open (accept anything)
+        _enabled = securityConfig?.EnableHtmlSanitization ?? true;
+        _allowedIframeHosts = ResolveAllowedIframeHosts(securityConfig);
+        _richTextOptions = BuildRichTextOptions();
+    }
+
+    /// <summary>
+    ///     Built once from a throwaway sanitizer's defaults, then reused - unmutated - as the constructor
+    ///     argument for every sanitizer built per detection call below. HtmlSanitizerOptions only fills in what
+    ///     it is given (verified: leaving UriAttributes/AllowedCssProperties/AllowedAtRules unset produces empty
+    ///     sets, not the library defaults), so every allowlist a plain `new HtmlSanitizer()` would carry has to
+    ///     be copied across explicitly, not just the ones this class customizes.
+    /// </summary>
+    private static HtmlSanitizerOptions BuildRichTextOptions()
+    {
+        var defaults = new HtmlSanitizer();
+
+        //the library defaults already exclude script, object, embed, svg, style and every on* handler, because
+        //only allowlisted tags and attributes survive; these are the deviations from those defaults
+        foreach (var tag in InteractiveTags) defaults.AllowedTags.Remove(tag);
+
+        //framed media, restricted to allowlisted hosts by OnFilterUrl below
+        defaults.AllowedTags.Add("iframe");
+        defaults.AllowedAttributes.Add("allowfullscreen");
+        defaults.AllowedAttributes.Add("frameborder");
+        defaults.AllowedAttributes.Add("allow");
+        defaults.AllowedAttributes.Add("loading");
+
+        //editors emit class names for tables, images and alignment
+        defaults.AllowedAttributes.Add("class");
+
+        defaults.AllowedSchemes.Add("mailto");
+        defaults.AllowedSchemes.Add("tel");
+        //data: is accepted only for images, and only on img/src - enforced in OnFilterUrl
+        defaults.AllowedSchemes.Add("data");
+
+        return new HtmlSanitizerOptions {
+            AllowedTags = defaults.AllowedTags,
+            AllowedAttributes = defaults.AllowedAttributes,
+            AllowedSchemes = defaults.AllowedSchemes,
+            AllowedCssClasses = defaults.AllowedClasses,
+            AllowedCssProperties = defaults.AllowedCssProperties,
+            AllowedAtRules = defaults.AllowedAtRules,
+            UriAttributes = defaults.UriAttributes,
+            UriListAttributes = defaults.UriListAttributes,
+            AllowCssCustomProperties = defaults.AllowCssCustomProperties,
+            //data-* is not allowlisted per attribute, so it cannot be vetted; it is also read by the storefront
+            //scripts, which makes it a way to influence behaviour from stored content
+            AllowDataAttributes = false
+        };
+    }
+
+    public bool ContainsDisallowedRichText(string html)
+    {
+        if (string.IsNullOrWhiteSpace(html)) return false;
+        if (!_enabled) return false;
+
+        var disallowedContentSeen = false;
+
+        void MarkDisallowed(object sender, EventArgs e)
+        {
+            disallowedContentSeen = true;
+        }
+
+        void OnFilterUrl(object sender, FilterUrlEventArgs e)
+        {
+            var url = e.SanitizedUrl ?? e.OriginalUrl;
+            if (string.IsNullOrWhiteSpace(url)) return;
+
+            var tagName = e.Tag?.NodeName;
+
+            if (url.StartsWith("data:", StringComparison.OrdinalIgnoreCase))
+            {
+                //data:text/html is a script execution vector; an inline image is not
+                var isInlineImage = string.Equals(tagName, "IMG", StringComparison.OrdinalIgnoreCase) &&
+                                    url.StartsWith("data:image/", StringComparison.OrdinalIgnoreCase);
+                if (!isInlineImage)
+                {
+                    e.SanitizedUrl = null;
+                    disallowedContentSeen = true;
+                }
+
+                return;
+            }
+
+            if (string.Equals(tagName, "IFRAME", StringComparison.OrdinalIgnoreCase) &&
+                !IsAllowedIframeUrl(url, _allowedIframeHosts))
+            {
+                e.SanitizedUrl = null;
+                disallowedContentSeen = true;
+            }
+        }
+
+        var sanitizer = new HtmlSanitizer(_richTextOptions);
+        sanitizer.RemovingTag += MarkDisallowed;
+        sanitizer.RemovingAttribute += MarkDisallowed;
+        sanitizer.RemovingStyle += MarkDisallowed;
+        sanitizer.RemovingAtRule += MarkDisallowed;
+        sanitizer.RemovingCssClass += MarkDisallowed;
+        sanitizer.RemovingComment += MarkDisallowed;
+        sanitizer.FilterUrl += OnFilterUrl;
+
+        var document = sanitizer.SanitizeDom(html);
+
+        //a literal <html>/<head>/<body> tag in the input is merged into the parser's own document root, which
+        //sits outside the per-element sanitization loop - RemovingAttribute never fires for its own attributes
+        //even though everything inside it is correctly sanitized (verified: <body onload=alert(1)> survives
+        //with the handler intact). Rich-text content is always a fragment and never legitimately needs
+        //attributes on that wrapper, so any attribute there means the raw input smuggled one in.
+        if (HasOwnAttributes(document.DocumentElement) || HasOwnAttributes(document.Head) ||
+            HasOwnAttributes(document.Body))
+            disallowedContentSeen = true;
+
+        return disallowedContentSeen;
+    }
+
+    public bool ContainsMarkup(string text)
+    {
+        if (string.IsNullOrWhiteSpace(text)) return false;
+        if (!_enabled) return false;
+
+        var disallowedContentSeen = false;
+
+        void MarkDisallowed(object sender, EventArgs e)
+        {
+            disallowedContentSeen = true;
+        }
+
+        var sanitizer = new HtmlSanitizer(new HtmlSanitizerOptions {
+            AllowedTags = new HashSet<string>(),
+            AllowedAttributes = new HashSet<string>(),
+            AllowedSchemes = new HashSet<string>(),
+            AllowedCssProperties = new HashSet<string>(),
+            UriAttributes = new HashSet<string>()
+        });
+        sanitizer.RemovingTag += MarkDisallowed;
+        sanitizer.RemovingComment += MarkDisallowed;
+
+        var document = sanitizer.SanitizeDom(text);
+
+        //see the identical guard in ContainsDisallowedRichText - a literal <html>/<head>/<body> tag merges
+        //into the document root and its own attributes never reach RemovingAttribute
+        if (HasOwnAttributes(document.DocumentElement) || HasOwnAttributes(document.Head) ||
+            HasOwnAttributes(document.Body))
+            disallowedContentSeen = true;
+
+        return disallowedContentSeen;
+    }
+
+    private static bool HasOwnAttributes(AngleSharp.Dom.IElement element)
+    {
+        return element is not null && element.Attributes.Length > 0;
+    }
+
+    private static string[] ResolveAllowedIframeHosts(SecurityConfig securityConfig)
+    {
+        //absent configuration means "use the defaults"; a configured but empty list means "frame nothing"
+        if (securityConfig?.SanitizerAllowedIframeHosts is null) return DefaultAllowedIframeHosts;
+
+        return securityConfig.SanitizerAllowedIframeHosts
+            .Where(host => !string.IsNullOrWhiteSpace(host))
+            .Select(host => host.Trim())
+            .ToArray();
+    }
+
+    private static bool IsAllowedIframeUrl(string url, string[] allowedIframeHosts)
+    {
+        if (allowedIframeHosts.Length == 0) return false;
+
+        //protocol-relative ("//host/path") is parsed as a relative Uri by .NET - IsAbsoluteUri is false - but a
+        //browser resolves it against the current page's scheme, i.e. as an absolute url to an arbitrary host.
+        //Reject it before the relative-url fast path below would otherwise wave it through as same-origin.
+        if (url.StartsWith("//", StringComparison.Ordinal)) return false;
+
+        if (!Uri.TryCreate(url, UriKind.RelativeOrAbsolute, out var uri)) return false;
+
+        //a genuinely relative url (no leading "//") cannot leave this origin - it is how the file manager
+        //inserts self-hosted video
+        if (!uri.IsAbsoluteUri) return true;
+
+        if (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps) return false;
+
+        return allowedIframeHosts.Any(allowed => IsHostMatch(uri.Host, allowed));
+    }
+
+    private static bool IsHostMatch(string host, string allowed)
+    {
+        if (allowed.StartsWith("*.", StringComparison.Ordinal))
+            return host.EndsWith(allowed[1..], StringComparison.OrdinalIgnoreCase);
+
+        return string.Equals(host, allowed, StringComparison.OrdinalIgnoreCase);
+    }
+}

src/Core/Grand.Infrastructure/Security/IHtmlSanitizationService.cs

@@ -0,0 +1,28 @@
+namespace Grand.Infrastructure.Security;
+
+/// <summary>
+///     Detects markup that falls outside an allowlist. Content edited by a vendor, a store manager, or any
+///     non-superadmin account is untrusted: it is rendered back into the administration panel, so markup that
+///     survives here executes in an administrator's session.
+///     This is detection, not rewriting: callers reject the value (see <see cref="Grand.Infrastructure.Validators.SanitizeHtmlAttribute" />
+///     / <see cref="Grand.Infrastructure.Validators.NoHtmlAttribute" />) rather than silently cleaning it, so a
+///     rejected save always shows the editor what needs to change.
+/// </summary>
+public interface IHtmlSanitizationService
+{
+    /// <summary>
+    ///     True when the rich-text allowlist would remove something from <paramref name="html" /> - a tag,
+    ///     attribute, style rule, class, comment, or url (script, iframe to an unlisted host, javascript:, etc.)
+    ///     that can execute or navigate on its own. False for markup an editor legitimately produces, even if the
+    ///     allowlist would reformat it (e.g. an implied &lt;tbody&gt; made explicit).
+    /// </summary>
+    /// <param name="html">Untrusted markup; may be null</param>
+    bool ContainsDisallowedRichText(string html);
+
+    /// <summary>
+    ///     True when <paramref name="text" /> contains any HTML markup at all. For fields that are never rich
+    ///     text: meta titles, meta keywords, meta descriptions, admin comments, attribute names.
+    /// </summary>
+    /// <param name="text">Untrusted text; may be null</param>
+    bool ContainsMarkup(string text);
+}

src/Core/Grand.Infrastructure/StartupBase.cs

@@ -7,6 +7,7 @@
 using Grand.Infrastructure.Modules;
 using Grand.Infrastructure.Plugins;
 using Grand.Infrastructure.Roslyn;
+using Grand.Infrastructure.Security;
 using Grand.Infrastructure.TypeConverters;
 using Grand.Infrastructure.TypeSearch;
 using Grand.Infrastructure.Validators;
@@ -175,6 +176,9 @@ private static IMvcCoreBuilder RegisterApplication(IServiceCollection services,
 
         InitDatabase(services, configuration);
 
+        //resolved by SanitizeHtmlAttribute/NoHtmlAttribute via ValidationContext.GetService - no filter needed,
+        //ASP.NET Core's built-in model validation already invokes DataAnnotations attributes on every bind
+        services.AddSingleton<IHtmlSanitizationService, HtmlSanitizationService>();
         services.AddTransient<ValidationFilter>();
         var mvcCoreBuilder = services.AddMvcCore(options =>
         {

src/Core/Grand.Infrastructure/Validators/NoHtmlAttribute.cs

@@ -0,0 +1,35 @@
+using System.ComponentModel.DataAnnotations;
+using Grand.Infrastructure.Security;
+
+namespace Grand.Infrastructure.Validators;
+
+/// <summary>
+///     Rejects a bound string property that contains any HTML markup at all - for a property that is never rich
+///     text: a meta tag, a name, an internal comment. See <see cref="SanitizeHtmlAttribute" /> for why detection
+///     goes through <see cref="IHtmlSanitizationService" /> resolved via <see cref="ValidationContext.GetService" />.
+/// </summary>
+[AttributeUsage(AttributeTargets.Property)]
+public class NoHtmlAttribute() : ValidationAttribute("{0} must be plain text and cannot contain HTML markup.")
+{
+    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
+    {
+        if (value is not string text || string.IsNullOrWhiteSpace(text)) return ValidationResult.Success;
+
+        var sanitizationService = (IHtmlSanitizationService)validationContext
+            .GetService(typeof(IHtmlSanitizationService));
+
+        if (sanitizationService is null)
+            throw new InvalidOperationException(
+                $"{nameof(IHtmlSanitizationService)} could not be resolved - is it registered in DI?");
+
+        return sanitizationService.ContainsMarkup(text)
+            ? new ValidationResult(FormatErrorMessage(validationContext.DisplayName),
+                MemberNames(validationContext))
+            : ValidationResult.Success;
+    }
+
+    private static IEnumerable<string> MemberNames(ValidationContext validationContext)
+    {
+        return validationContext.MemberName is null ? [] : [validationContext.MemberName];
+    }
+}