From 7ff2721ba9c5310f00eb422c4f0785164e81b4f6 Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Tue, 25 Sep 2018 02:35:15 +0800 Subject: [PATCH] - Enhancement: Auto-detect `allowedImageLibOrigins` based on locale rather than requiring user to supply --- CHANGES.md | 5 ++-- editor/extensions/ext-imagelib.js | 38 +++++++++++++++++------------ editor/svg-editor.js | 13 ++++------ editor/xdomain-svgedit-config-es.js | 3 +-- svgedit-config-es.js | 1 - 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 9109b24f..96b3316d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,8 +1,7 @@ # ? -- Security fix/Breaking change (Imagelib): Require `allowedImageLibOrigins` - config array be set with safe origins or otherwise reject `postMessage` - messages in case from untrusted sources +- Security fix/Breaking change (Imagelib): Only allow origins within + `imgLibs` to be accepted for `message` listener - Security fix/Breaking change (xdomain): Namespace xdomain file to avoid it being used to modify non-xdomain storage - Security fix (Imagelib): Avoid XSS diff --git a/editor/extensions/ext-imagelib.js b/editor/extensions/ext-imagelib.js index 636c9935..a3688cc3 100644 --- a/editor/extensions/ext-imagelib.js +++ b/editor/extensions/ext-imagelib.js @@ -11,10 +11,27 @@ export default { name: 'imagelib', async init ({decode64, importLocale, dropXMLInternalSubset}) { const imagelibStrings = await importLocale(); + + const modularVersion = !('svgEditor' in window) || + !window.svgEditor || + window.svgEditor.modules !== false; + imagelibStrings.imgLibs = imagelibStrings.imgLibs.map(({name, url, description}) => { + url = url + .replace(/\{path\}/g, extIconsPath) + .replace(/\{modularVersion\}/g, modularVersion + ? (imagelibStrings.moduleEnding || '-es') + : '' + ); + return {name, url, description}; + }); + const allowedImageLibOrigins = imagelibStrings.imgLibs.map(({url}) => { + return new URL(url).origin; + }); + const svgEditor = this; const $ = jQuery; - const {uiStrings, canvas: svgCanvas, curConfig: {allowedImageLibOrigins}} = svgEditor; + const {uiStrings, canvas: svgCanvas, curConfig: {extIconsPath}} = svgEditor; function closeBrowser () { $('#imgbrowse_holder').hide(); @@ -344,10 +361,7 @@ export default { cancel.prepend($.getSvgIcon('cancel', true)); back.prepend($.getSvgIcon('tool_imagelib', true)); - const modularVersion = !('svgEditor' in window) || - !window.svgEditor || - window.svgEditor.modules !== false; - $.each(imagelibStrings.imgLibs, function (i, {name, url, description}) { + imagelibStrings.imgLibs.forEach(function ({name, url, description}) { $('
  • ') .appendTo(libOpts) .text(name) @@ -355,15 +369,7 @@ export default { frame.attr( 'src', // Todo: Adopt some standard formatting library like `fluent.js` instead - url.replace( - '{path}', - svgEditor.curConfig.extIconsPath - ).replace( - '{modularVersion}', - modularVersion - ? (imagelibStrings.moduleEnding || '-es') - : '' - ) + url ).show(); header.text(name); libOpts.hide(); @@ -378,7 +384,7 @@ export default { const buttons = [{ id: 'tool_imagelib', type: 'app_menu', // _flyout - icon: svgEditor.curConfig.extIconsPath + 'imagelib.png', + icon: extIconsPath + 'imagelib.png', position: 4, events: { mouseup: showBrowser @@ -386,7 +392,7 @@ export default { }]; return { - svgicons: svgEditor.curConfig.extIconsPath + 'ext-imagelib.xml', + svgicons: extIconsPath + 'ext-imagelib.xml', buttons: imagelibStrings.buttons.map((button, i) => { return Object.assign(buttons[i], button); }), diff --git a/editor/svg-editor.js b/editor/svg-editor.js index 9c8ec6b9..34d7922a 100644 --- a/editor/svg-editor.js +++ b/editor/svg-editor.js @@ -181,7 +181,6 @@ const callbacks = [], * @property {string[]} [extensions=module:SVGEditor~defaultExtensions] Extensions to load on startup. Use an array in `setConfig` and comma separated file names in the URL. Extension names must begin with "ext-". Note that as of version 2.7, paths containing "/", "\", or ":", are disallowed for security reasons. Although previous versions of this list would entirely override the default list, as of version 2.7, the defaults will always be added to this explicit list unless the configuration `noDefaultExtensions` is included. * @property {module:SVGEditor.Stylesheet[]} [stylesheets=["@default"]] An array of required stylesheets to load in parallel; include the value `"@default"` within this array to ensure all default stylesheets are loaded. * @property {string[]} [allowedOrigins=[]] Used by `ext-xdomain-messaging.js` to indicate which origins are permitted for cross-domain messaging (e.g., between the embedded editor and main editor code). Besides explicit domains, one might add '*' to allow all domains (not recommended for privacy/data integrity of your user's content!), `window.location.origin` for allowing the same origin (should be safe if you trust all apps on your domain), 'null' to allow `file:///` URL usage - * @property {string[]} [allowedImageLibOrigins=[]] Used by `ext-imagelib.js` to indicate which origins are permitted for cross-domain image loading (image libraries). Besides explicit domains, one might add '*' to allow all domains (not recommended for privacy/data integrity of your user's content!), `window.location.origin` for allowing the same origin (should be safe if you trust all apps on your domain), 'null' to allow `file:///` URL usage * @property {null|PlainObject} [colorPickerCSS=null] Object of CSS properties mapped to values (for jQuery) to apply to the color picker. See {@link http://api.jquery.com/css/#css-properties}. A `null` value (the default) will cause the CSS to default to `left` with a position equal to that of the `fill_color` or `stroke_color` element minus 140, and a `bottom` equal to 40 * @property {string} [paramurl] This was available via URL only. Allowed an un-encoded URL within the query string (use "url" or "source" with a data: URI instead) * @property {Float} [canvas_expansion=3] The minimum area visible outside the canvas, as a multiple of the image dimensions. The larger the number, the more one can scroll outside the canvas. @@ -304,12 +303,10 @@ let svgCanvas, urldata, * avoiding it ensures some more security that even third * party apps on the same domain also cannot communicate * with this app by default. - * `allowedOrigins` is for use with `ext-xdomain-messaging.js` and - * `allowedImageLibOrigins` is for use with `ext-imagelib.js` + * For use with `ext-xdomain-messaging.js` * @todo We might instead make as a user-facing preference. */ - allowedOrigins: [], - allowedImageLibOrigins: [] + allowedOrigins: [] }; function loadSvgString (str, callback) { @@ -490,11 +487,11 @@ editor.setConfig = function (opts, cfgCfg) { } else { $.pref(key, val); } - } else if (['extensions', 'stylesheets', 'allowedOrigins', 'allowedImageLibOrigins'].includes(key)) { + } else if (['extensions', 'stylesheets', 'allowedOrigins'].includes(key)) { if (cfgCfg.overwrite === false && ( curConfig.preventAllURLConfig || - ['allowedOrigins', 'allowedImageLibOrigins', 'stylesheets'].includes(key) || + ['allowedOrigins', 'stylesheets'].includes(key) || (key === 'extensions' && curConfig.lockExtensions) ) ) { @@ -674,7 +671,7 @@ editor.init = function () { curConfig.extensions = curConfig.extensions.concat(defaultExtensions); } // ...and remove any dupes - ['extensions', 'stylesheets', 'allowedOrigins', 'allowedImageLibOrigins'].forEach(function (cfg) { + ['extensions', 'stylesheets', 'allowedOrigins'].forEach(function (cfg) { curConfig[cfg] = $.grep(curConfig[cfg], function (n, i) { // Supposedly faster than filter per http://amandeep1986.blogspot.hk/2015/02/jquery-grep-vs-js-filter.html return i === curConfig[cfg].indexOf(n); }); diff --git a/editor/xdomain-svgedit-config-es.js b/editor/xdomain-svgedit-config-es.js index 386332f1..daa62815 100644 --- a/editor/xdomain-svgedit-config-es.js +++ b/editor/xdomain-svgedit-config-es.js @@ -1,6 +1,5 @@ import svgEditor from './svg-editor.js'; svgEditor.setConfig({ canvasName: 'xdomain', // Namespace this - allowedOrigins: ['*'], - allowedImageLibOrigins: ['*'] + allowedOrigins: ['*'] }); diff --git a/svgedit-config-es.js b/svgedit-config-es.js index a01aa96b..1511c4e2 100644 --- a/svgedit-config-es.js +++ b/svgedit-config-es.js @@ -92,7 +92,6 @@ svgEditor.setConfig({ data privacy and integrity. */ // allowedOrigins: [location.origin || 'null'], // May be 'null' (as a string) when used as a `file:///` URL - // allowedImageLibOrigins: [location.origin || 'null'], // May be 'null' (as a string) when used as a `file:///` URL // DOCUMENT PROPERTIES // dimensions: [640, 480], // EDITOR OPTIONS