Additional optional sanitization of scripting in TinyMCE (umbraco#10653)

Nuklon · Nov 2, 2021 · f68dba7 · f68dba7
1 parent bd23a5f
commit f68dba7
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 1 deletion.
diff --git a/src/Umbraco.Core/Configuration/GlobalSettings.cs b/src/Umbraco.Core/Configuration/GlobalSettings.cs
@@ -395,6 +395,27 @@ public bool UseHttps
             }
         }
 
+        /// <summary>
+        /// Returns true if TinyMCE scripting sanitization should be applied
+        /// </summary>
+        /// <remarks>
+        /// The default value is false
+        /// </remarks>
+        public bool SanitizeTinyMce
+        {
+            get
+            {
+                try
+                {
+                    return bool.Parse(ConfigurationManager.AppSettings[Constants.AppSettings.SanitizeTinyMce]);
+                }
+                catch
+                {
+                    return false;
+                }
+            }
+        }
+
         /// <summary>
         /// An int value representing the time in milliseconds to lock the database for a write operation
         /// </summary>

diff --git a/src/Umbraco.Core/Configuration/IGlobalSettings.cs b/src/Umbraco.Core/Configuration/IGlobalSettings.cs
@@ -77,5 +77,10 @@ public interface IGlobalSettings
         /// Gets the write lock timeout.
         /// </summary>
         int SqlWriteLockTimeOut { get; }
+
+        /// <summary>
+        /// Returns true if TinyMCE scripting sanitization should be applied
+        /// </summary>
+        bool SanitizeTinyMce { get; }
     }
 }
diff --git a/src/Umbraco.Core/Constants-AppSettings.cs b/src/Umbraco.Core/Constants-AppSettings.cs
@@ -109,7 +109,7 @@ public static class AppSettings
             /// A true or false indicating whether umbraco should force a secure (https) connection to the backoffice.
             /// </summary>
             public const string UseHttps = "Umbraco.Core.UseHttps";
-            
+
             /// <summary>
             /// A true/false value indicating whether the content dashboard should be visible for all user groups.
             /// </summary>
@@ -155,6 +155,11 @@ public static class Debug
             /// An int value representing the time in milliseconds to lock the database for a write operation
             /// </summary>
             public const string SqlWriteLockTimeOut = "Umbraco.Core.SqlWriteLockTimeOut";
+
+            /// <summary>
+            /// Returns true if TinyMCE scripting sanitization should be applied
+            /// </summary>
+            public const string SanitizeTinyMce = "Umbraco.Web.SanitizeTinyMce";
         }
     }
 }
diff --git a/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js b/src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js
@@ -1497,6 +1497,19 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s
                     });
 
                 }
+
+                if(Umbraco.Sys.ServerVariables.umbracoSettings.sanitizeTinyMce === true){
+                    /** prevent injecting arbitrary JavaScript execution in on-attributes. */
+                    const allNodes = Array.prototype.slice.call(args.editor.dom.doc.getElementsByTagName("*"));
+                    allNodes.forEach(node => {
+                        for (var i = 0; i < node.attributes.length; i++) {
+                            if(node.attributes[i].name.indexOf("on") === 0) {
+                                node.removeAttribute(node.attributes[i].name)
+                            }
+                        }
+                    });
+                }
+
             });
 
             args.editor.on('init', function (e) {
@@ -1508,6 +1521,60 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s
                 //enable browser based spell checking
                 args.editor.getBody().setAttribute('spellcheck', true);
 
+
+                /** Setup sanitization for preventing injecting arbitrary JavaScript execution in attributes:
+                 * https://github.com/advisories/GHSA-w7jx-j77m-wp65
+                 * https://github.com/advisories/GHSA-5vm8-hhgr-jcjp
+                 */
+                const uriAttributesToSanitize = ['src', 'href', 'data', 'background', 'action', 'formaction', 'poster', 'xlink:href'];
+                const parseUri = function() {
+                    // Encapsulated JS logic.
+                    const safeSvgDataUrlElements = [ 'img', 'video' ];
+                    const scriptUriRegExp = /((java|vb)script|mhtml):/i;
+                    const trimRegExp = /[\s\u0000-\u001F]+/g;
+                    const isInvalidUri = (uri, tagName) => {
+                        if (/^data:image\//i.test(uri)) {
+                            return safeSvgDataUrlElements.indexOf(tagName) !== -1 && /^data:image\/svg\+xml/i.test(uri);
+                        } else {
+                            return /^data:/i.test(uri);
+                        }
+                    };
+
+                    return function parseUri(uri, tagName) {
+                        uri = uri.replace(trimRegExp, '');
+                        try {
+                            // Might throw malformed URI sequence
+                            uri = decodeURIComponent(uri);
+                        } catch (ex) {
+                            // Fallback to non UTF-8 decoder
+                            uri = unescape(uri);
+                        }
+
+                        if (scriptUriRegExp.test(uri)) {
+                            return;
+                        }
+
+                        if (isInvalidUri(uri, tagName)) {
+                            return;
+                        }
+
+                        return uri;
+                    }
+                }();
+
+                if(Umbraco.Sys.ServerVariables.umbracoSettings.sanitizeTinyMce === true){
+                    args.editor.serializer.addAttributeFilter(uriAttributesToSanitize, function (nodes) {
+                        nodes.forEach(function(node) {
+                            node.attributes.forEach(function(attr) {
+                                const attrName = attr.name.toLowerCase();
+                                if(uriAttributesToSanitize.indexOf(attrName) !== -1) {
+                                    attr.value = parseUri(attr.value, node.name);
+                                }
+                            });
+                        });
+                    });
+                }
+
                 //start watching the value
                 startWatch();
             });

diff --git a/src/Umbraco.Web.UI/web.Template.config b/src/Umbraco.Web.UI/web.Template.config
@@ -51,6 +51,7 @@
         <add key="Umbraco.ModelsBuilder.Enable" value="true" />
         <add key="Umbraco.ModelsBuilder.ModelsMode" value="PureLive" />
         <add key="Umbraco.Web.PublishedCache.NuCache.Serializer" value="MsgPack"/>
+        <add key="Umbraco.Web.SanitizeTinyMce" value="true" />
     </appSettings>
 
     <!--

diff --git a/src/Umbraco.Web/Editors/BackOfficeServerVariables.cs b/src/Umbraco.Web/Editors/BackOfficeServerVariables.cs
@@ -361,6 +361,7 @@ internal Dictionary<string, object> GetServerVariables()
                         {"showAllowSegmentationForDocumentTypes", false},
                         {"minimumPasswordLength", userMembershipProvider.MinRequiredPasswordLength},
                         {"minimumPasswordNonAlphaNum", userMembershipProvider.MinRequiredNonAlphanumericCharacters},
+                        {"sanitizeTinyMce", Current.Configs.Global().SanitizeTinyMce}
                     }
                 },
                 {