diff --git a/src/Umbraco.Core/Composing/CompositionExtensions/Services.cs b/src/Umbraco.Core/Composing/CompositionExtensions/Services.cs index e912f7281c..3c9dd9d701 100644 --- a/src/Umbraco.Core/Composing/CompositionExtensions/Services.cs +++ b/src/Umbraco.Core/Composing/CompositionExtensions/Services.cs @@ -6,6 +6,7 @@ using Umbraco.Core.Events; using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Packaging; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; using Umbraco.Core.Telemetry; @@ -81,6 +82,7 @@ namespace Umbraco.Core.Composing.CompositionExtensions new DirectoryInfo(IOHelper.GetRootDirectorySafe()))); composition.RegisterUnique(); + composition.RegisterUnique(); return composition; } diff --git a/src/Umbraco.Core/Security/IHtmlSanitizer.cs b/src/Umbraco.Core/Security/IHtmlSanitizer.cs new file mode 100644 index 0000000000..fa1e0b3ee5 --- /dev/null +++ b/src/Umbraco.Core/Security/IHtmlSanitizer.cs @@ -0,0 +1,12 @@ +namespace Umbraco.Core.Security +{ + public interface IHtmlSanitizer + { + /// + /// Sanitizes HTML + /// + /// HTML to be sanitized + /// Sanitized HTML + string Sanitize(string html); + } +} diff --git a/src/Umbraco.Core/Security/NoopHtmlSanitizer.cs b/src/Umbraco.Core/Security/NoopHtmlSanitizer.cs new file mode 100644 index 0000000000..2ea34d52ea --- /dev/null +++ b/src/Umbraco.Core/Security/NoopHtmlSanitizer.cs @@ -0,0 +1,10 @@ +namespace Umbraco.Core.Security +{ + public class NoopHtmlSanitizer : IHtmlSanitizer + { + public string Sanitize(string html) + { + return html; + } + } +} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 6729930174..e27c6eeceb 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -194,6 +194,8 @@ + + diff --git a/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs b/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs index cafda161f4..9b8f5a05fd 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs @@ -16,6 +16,7 @@ using Umbraco.Core.Cache; using Umbraco.Core.Configuration; using Umbraco.Core.Logging; using Umbraco.Core.Models; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.Testing; @@ -54,7 +55,7 @@ namespace Umbraco.Tests.PublishedContent var dataTypeService = new TestObjects.TestDataTypeService( new DataType(new VoidEditor(logger)) { Id = 1 }, new DataType(new TrueFalsePropertyEditor(logger)) { Id = 1001 }, - new DataType(new RichTextPropertyEditor(logger, umbracoContextAccessor, imageSourceParser, linkParser, pastedImages, Mock.Of())) { Id = 1002 }, + new DataType(new RichTextPropertyEditor(logger, umbracoContextAccessor, imageSourceParser, linkParser, pastedImages, Mock.Of(), Mock.Of())) { Id = 1002 }, new DataType(new IntegerPropertyEditor(logger)) { Id = 1003 }, new DataType(new TextboxPropertyEditor(logger)) { Id = 1004 }, new DataType(new MediaPickerPropertyEditor(logger)) { Id = 1005 }); diff --git a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs index 6f919868f7..5ed3051e07 100644 --- a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs @@ -8,6 +8,7 @@ using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Templates; @@ -32,8 +33,9 @@ namespace Umbraco.Web.PropertyEditors private readonly RichTextEditorPastedImages _pastedImages; private readonly HtmlLocalLinkParser _localLinkParser; private readonly IImageUrlGenerator _imageUrlGenerator; + private readonly IHtmlSanitizer _htmlSanitizer; - [Obsolete("Use the constructor which takes an IImageUrlGenerator")] + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] public GridPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, @@ -43,12 +45,24 @@ namespace Umbraco.Web.PropertyEditors { } + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] public GridPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, RichTextEditorPastedImages pastedImages, HtmlLocalLinkParser localLinkParser, IImageUrlGenerator imageUrlGenerator) + : this(logger, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, imageUrlGenerator, Current.Factory.GetInstance()) + { + } + + public GridPropertyEditor(ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + RichTextEditorPastedImages pastedImages, + HtmlLocalLinkParser localLinkParser, + IImageUrlGenerator imageUrlGenerator, + IHtmlSanitizer htmlSanitizer) : base(logger) { _umbracoContextAccessor = umbracoContextAccessor; @@ -56,6 +70,7 @@ namespace Umbraco.Web.PropertyEditors _pastedImages = pastedImages; _localLinkParser = localLinkParser; _imageUrlGenerator = imageUrlGenerator; + _htmlSanitizer = htmlSanitizer; } public override IPropertyIndexValueFactory PropertyIndexValueFactory => new GridPropertyIndexValueFactory(); @@ -64,7 +79,7 @@ namespace Umbraco.Web.PropertyEditors /// Overridden to ensure that the value is validated /// /// - protected override IDataValueEditor CreateValueEditor() => new GridPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _pastedImages, _localLinkParser, _imageUrlGenerator); + protected override IDataValueEditor CreateValueEditor() => new GridPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _pastedImages, _localLinkParser, _imageUrlGenerator, _htmlSanitizer); protected override IConfigurationEditor CreateConfigurationEditor() => new GridConfigurationEditor(); @@ -77,7 +92,7 @@ namespace Umbraco.Web.PropertyEditors private readonly MediaPickerPropertyEditor.MediaPickerPropertyValueEditor _mediaPickerPropertyValueEditor; private readonly IImageUrlGenerator _imageUrlGenerator; - [Obsolete("Use the constructor which takes an IImageUrlGenerator")] + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] public GridPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, @@ -87,20 +102,32 @@ namespace Umbraco.Web.PropertyEditors { } + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] public GridPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, RichTextEditorPastedImages pastedImages, HtmlLocalLinkParser localLinkParser, IImageUrlGenerator imageUrlGenerator) + : this(attribute, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, imageUrlGenerator, Current.Factory.GetInstance()) + { + } + + public GridPropertyValueEditor(DataEditorAttribute attribute, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + RichTextEditorPastedImages pastedImages, + HtmlLocalLinkParser localLinkParser, + IImageUrlGenerator imageUrlGenerator, + IHtmlSanitizer htmlSanitizer) : base(attribute) { _umbracoContextAccessor = umbracoContextAccessor; _imageSourceParser = imageSourceParser; _pastedImages = pastedImages; - _richTextPropertyValueEditor = new RichTextPropertyEditor.RichTextPropertyValueEditor(attribute, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, _imageUrlGenerator); - _mediaPickerPropertyValueEditor = new MediaPickerPropertyEditor.MediaPickerPropertyValueEditor(attribute); _imageUrlGenerator = imageUrlGenerator; + _richTextPropertyValueEditor = new RichTextPropertyEditor.RichTextPropertyValueEditor(attribute, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, _imageUrlGenerator, htmlSanitizer); + _mediaPickerPropertyValueEditor = new MediaPickerPropertyEditor.MediaPickerPropertyValueEditor(attribute); } /// diff --git a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs index 2d698835b0..4a6c0c09b6 100644 --- a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs @@ -7,6 +7,7 @@ using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Examine; using Umbraco.Web.Macros; @@ -32,21 +33,46 @@ namespace Umbraco.Web.PropertyEditors private readonly HtmlLocalLinkParser _localLinkParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly IImageUrlGenerator _imageUrlGenerator; + private readonly IHtmlSanitizer _htmlSanitizer; /// /// The constructor will setup the property editor based on the attribute if one is found /// - [Obsolete("Use the constructor which takes an IImageUrlGenerator")] - public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages) + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] + public RichTextPropertyEditor( + ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + HtmlLocalLinkParser localLinkParser, + RichTextEditorPastedImages pastedImages) : this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, Current.ImageUrlGenerator) { } + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] + public RichTextPropertyEditor( + ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + HtmlLocalLinkParser localLinkParser, + RichTextEditorPastedImages pastedImages, + IImageUrlGenerator imageUrlGenerator) + : this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, imageUrlGenerator, Current.Factory.GetInstance()) + { + } + /// /// The constructor will setup the property editor based on the attribute if one is found /// - public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) + public RichTextPropertyEditor( + ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + HtmlLocalLinkParser localLinkParser, + RichTextEditorPastedImages pastedImages, + IImageUrlGenerator imageUrlGenerator, + IHtmlSanitizer htmlSanitizer) : base(logger) { _umbracoContextAccessor = umbracoContextAccessor; @@ -54,13 +80,14 @@ namespace Umbraco.Web.PropertyEditors _localLinkParser = localLinkParser; _pastedImages = pastedImages; _imageUrlGenerator = imageUrlGenerator; + _htmlSanitizer = htmlSanitizer; } /// /// Create a custom value editor /// /// - protected override IDataValueEditor CreateValueEditor() => new RichTextPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _localLinkParser, _pastedImages, _imageUrlGenerator); + protected override IDataValueEditor CreateValueEditor() => new RichTextPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _localLinkParser, _pastedImages, _imageUrlGenerator, _htmlSanitizer); protected override IConfigurationEditor CreateConfigurationEditor() => new RichTextConfigurationEditor(); @@ -76,8 +103,16 @@ namespace Umbraco.Web.PropertyEditors private readonly HtmlLocalLinkParser _localLinkParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly IImageUrlGenerator _imageUrlGenerator; + private readonly IHtmlSanitizer _htmlSanitizer; - public RichTextPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) + public RichTextPropertyValueEditor( + DataEditorAttribute attribute, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + HtmlLocalLinkParser localLinkParser, + RichTextEditorPastedImages pastedImages, + IImageUrlGenerator imageUrlGenerator, + IHtmlSanitizer htmlSanitizer) : base(attribute) { _umbracoContextAccessor = umbracoContextAccessor; @@ -85,6 +120,7 @@ namespace Umbraco.Web.PropertyEditors _localLinkParser = localLinkParser; _pastedImages = pastedImages; _imageUrlGenerator = imageUrlGenerator; + _htmlSanitizer = htmlSanitizer; } /// @@ -141,8 +177,9 @@ namespace Umbraco.Web.PropertyEditors var parseAndSavedTempImages = _pastedImages.FindAndPersistPastedTempImages(editorValue.Value.ToString(), mediaParentId, userId, _imageUrlGenerator); var editorValueWithMediaUrlsRemoved = _imageSourceParser.RemoveImageSources(parseAndSavedTempImages); var parsed = MacroTagParser.FormatRichTextContentForPersistence(editorValueWithMediaUrlsRemoved); + var sanitized = _htmlSanitizer.Sanitize(parsed); - return parsed.NullOrWhiteSpaceAsNull(); + return sanitized.NullOrWhiteSpaceAsNull(); } ///