Skip to content

Commit 23ce69d

Browse files
nielslyngsoenul800sebastiaan
authored andcommitted
Additional optional sanitization of scripting in TinyMCE (#10653)
(cherry picked from commit f68dba7)
1 parent da48f84 commit 23ce69d

File tree

6 files changed

+101
-1
lines changed

6 files changed

+101
-1
lines changed

src/Umbraco.Core/Configuration/GlobalSettings.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,27 @@ public bool UseHttps
395395
}
396396
}
397397

398+
/// <summary>
399+
/// Returns true if TinyMCE scripting sanitization should be applied
400+
/// </summary>
401+
/// <remarks>
402+
/// The default value is false
403+
/// </remarks>
404+
public bool SanitizeTinyMce
405+
{
406+
get
407+
{
408+
try
409+
{
410+
return bool.Parse(ConfigurationManager.AppSettings[Constants.AppSettings.SanitizeTinyMce]);
411+
}
412+
catch
413+
{
414+
return false;
415+
}
416+
}
417+
}
418+
398419
/// <summary>
399420
/// An int value representing the time in milliseconds to lock the database for a write operation
400421
/// </summary>

src/Umbraco.Core/Configuration/IGlobalSettings.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,5 +77,10 @@ public interface IGlobalSettings
7777
/// Gets the write lock timeout.
7878
/// </summary>
7979
int SqlWriteLockTimeOut { get; }
80+
81+
/// <summary>
82+
/// Returns true if TinyMCE scripting sanitization should be applied
83+
/// </summary>
84+
bool SanitizeTinyMce { get; }
8085
}
8186
}

src/Umbraco.Core/Constants-AppSettings.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public static class AppSettings
109109
/// A true or false indicating whether umbraco should force a secure (https) connection to the backoffice.
110110
/// </summary>
111111
public const string UseHttps = "Umbraco.Core.UseHttps";
112-
112+
113113
/// <summary>
114114
/// A true/false value indicating whether the content dashboard should be visible for all user groups.
115115
/// </summary>
@@ -155,6 +155,11 @@ public static class Debug
155155
/// An int value representing the time in milliseconds to lock the database for a write operation
156156
/// </summary>
157157
public const string SqlWriteLockTimeOut = "Umbraco.Core.SqlWriteLockTimeOut";
158+
159+
/// <summary>
160+
/// Returns true if TinyMCE scripting sanitization should be applied
161+
/// </summary>
162+
public const string SanitizeTinyMce = "Umbraco.Web.SanitizeTinyMce";
158163
}
159164
}
160165
}

src/Umbraco.Web.UI.Client/src/common/services/tinymce.service.js

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,6 +1491,19 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s
14911491
});
14921492

14931493
}
1494+
1495+
if(Umbraco.Sys.ServerVariables.umbracoSettings.sanitizeTinyMce === true){
1496+
/** prevent injecting arbitrary JavaScript execution in on-attributes. */
1497+
const allNodes = Array.prototype.slice.call(args.editor.dom.doc.getElementsByTagName("*"));
1498+
allNodes.forEach(node => {
1499+
for (var i = 0; i < node.attributes.length; i++) {
1500+
if(node.attributes[i].name.indexOf("on") === 0) {
1501+
node.removeAttribute(node.attributes[i].name)
1502+
}
1503+
}
1504+
});
1505+
}
1506+
14941507
});
14951508

14961509
args.editor.on('init', function (e) {
@@ -1502,6 +1515,60 @@ function tinyMceService($rootScope, $q, imageHelper, $locale, $http, $timeout, s
15021515
//enable browser based spell checking
15031516
args.editor.getBody().setAttribute('spellcheck', true);
15041517

1518+
1519+
/** Setup sanitization for preventing injecting arbitrary JavaScript execution in attributes:
1520+
* https://github.com/advisories/GHSA-w7jx-j77m-wp65
1521+
* https://github.com/advisories/GHSA-5vm8-hhgr-jcjp
1522+
*/
1523+
const uriAttributesToSanitize = ['src', 'href', 'data', 'background', 'action', 'formaction', 'poster', 'xlink:href'];
1524+
const parseUri = function() {
1525+
// Encapsulated JS logic.
1526+
const safeSvgDataUrlElements = [ 'img', 'video' ];
1527+
const scriptUriRegExp = /((java|vb)script|mhtml):/i;
1528+
const trimRegExp = /[\s\u0000-\u001F]+/g;
1529+
const isInvalidUri = (uri, tagName) => {
1530+
if (/^data:image\//i.test(uri)) {
1531+
return safeSvgDataUrlElements.indexOf(tagName) !== -1 && /^data:image\/svg\+xml/i.test(uri);
1532+
} else {
1533+
return /^data:/i.test(uri);
1534+
}
1535+
};
1536+
1537+
return function parseUri(uri, tagName) {
1538+
uri = uri.replace(trimRegExp, '');
1539+
try {
1540+
// Might throw malformed URI sequence
1541+
uri = decodeURIComponent(uri);
1542+
} catch (ex) {
1543+
// Fallback to non UTF-8 decoder
1544+
uri = unescape(uri);
1545+
}
1546+
1547+
if (scriptUriRegExp.test(uri)) {
1548+
return;
1549+
}
1550+
1551+
if (isInvalidUri(uri, tagName)) {
1552+
return;
1553+
}
1554+
1555+
return uri;
1556+
}
1557+
}();
1558+
1559+
if(Umbraco.Sys.ServerVariables.umbracoSettings.sanitizeTinyMce === true){
1560+
args.editor.serializer.addAttributeFilter(uriAttributesToSanitize, function (nodes) {
1561+
nodes.forEach(function(node) {
1562+
node.attributes.forEach(function(attr) {
1563+
const attrName = attr.name.toLowerCase();
1564+
if(uriAttributesToSanitize.indexOf(attrName) !== -1) {
1565+
attr.value = parseUri(attr.value, node.name);
1566+
}
1567+
});
1568+
});
1569+
});
1570+
}
1571+
15051572
//start watching the value
15061573
startWatch();
15071574
});

src/Umbraco.Web.UI/web.Template.config

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
<add key="Umbraco.ModelsBuilder.Enable" value="true" />
5252
<add key="Umbraco.ModelsBuilder.ModelsMode" value="PureLive" />
5353
<add key="Umbraco.Web.PublishedCache.NuCache.Serializer" value="MsgPack"/>
54+
<add key="Umbraco.Web.SanitizeTinyMce" value="true" />
5455
</appSettings>
5556

5657
<!--

src/Umbraco.Web/Editors/BackOfficeServerVariables.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ internal Dictionary<string, object> GetServerVariables()
361361
{"showAllowSegmentationForDocumentTypes", false},
362362
{"minimumPasswordLength", userMembershipProvider.MinRequiredPasswordLength},
363363
{"minimumPasswordNonAlphaNum", userMembershipProvider.MinRequiredNonAlphanumericCharacters},
364+
{"sanitizeTinyMce", Current.Configs.Global().SanitizeTinyMce}
364365
}
365366
},
366367
{

0 commit comments

Comments
 (0)