diff --git a/CHANGELOG.md b/CHANGELOG.md index 918b6628..18f02fb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * `dataAttr` is now applied to the menu items themselves, using the correct attribute name (fixes #732, fixes #712) * Upgraded the embedded jQuery UI Position to 1.13.2 (CVE-2021-41184) (fixes #765) * Caller-supplied selector strings are no longer evaluated as HTML (fixes #731) +* `item.icon` is no longer interpolated into markup on the Font Awesome paths (fixes #810) * A left-click trigger no longer leaks a synthetic `contextmenu` event to unrelated ancestor listeners (fixes #754) * `$.contextMenu('update')` no longer throws when a `build` menu has not been shown yet (fixes #740) * `autoHide` now works for a nested trigger registered with a different trigger mode (fixes #727) diff --git a/src/jquery.contextMenu.js b/src/jquery.contextMenu.js index 5f77c0d9..d5299138 100644 --- a/src/jquery.contextMenu.js +++ b/src/jquery.contextMenu.js @@ -1934,7 +1934,11 @@ ) { // to enable font awesome $t.addClass(root.classNames.icon + ' ' + root.classNames.icon + '--fa5'); - item._icon = $(''); + // built with addClass instead of interpolating the icon + // into a markup string: an icon name coming from stored + // or otherwise non-literal data could otherwise close the + // class attribute and inject arbitrary markup. + item._icon = $('').addClass(item.icon); } else if (typeof(item.icon) === 'string' && item.icon.substring(0, 3) === 'fa-') { // legacy Font Awesome 4 style icon class (e.g. "fa-trash"), kept for // backwards compatibility. Just like the fas/far/fab/fad/fal syntax @@ -1944,7 +1948,7 @@ // which used to bleed into (and bold) the item's label text and clash // with this plugin's own icon styling. $t.addClass(root.classNames.icon + ' ' + root.classNames.icon + '--fa5'); - item._icon = $(''); + item._icon = $('').addClass('fa').addClass(item.icon); } else { item._icon = root.classNames.icon + ' ' + root.classNames.icon + '-' + item.icon; } diff --git a/test/unit/issue-810-icon-html-injection.test.js b/test/unit/issue-810-icon-html-injection.test.js new file mode 100644 index 00000000..3def69ee --- /dev/null +++ b/test/unit/issue-810-icon-html-injection.test.js @@ -0,0 +1,225 @@ +// Regression tests for https://github.com/swisnl/jQuery-contextMenu/issues/810 +// +// The Font Awesome icon branches used to build their element by string +// concatenation, so an `item.icon` value containing a quote or an angle bracket +// broke out of the class attribute and injected arbitrary markup. That matters +// for any menu whose icon names come from stored or otherwise non-literal data. +// +// The payload below is an with an invalid data URI, so the browser fires +// its `error` handler without needing a network round trip. The handler sets a +// flag, which is what the assertions check. +var ICON_XSS_TAIL = ' ">'; + +function xssRan810() { + return window.__contextMenuXss810 === true; +} + +function imageCount810() { + return document.querySelectorAll('img').length; +} + +function fixture810() { + var $fixture = $('#qunit-fixture'); + if ($fixture.length === 0) { + $('
').appendTo('body'); + $fixture = $('#qunit-fixture'); + } + return $fixture; +} + +// Registering a menu builds its