diff --git a/CHANGES.md b/CHANGES.md index d3de35b1..7e4327a5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,12 @@ # ? +- 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 (xdomain): Namespace xdomain file to avoid + it being used to modify non-xdomain storage +- Security fix (Imagelib): Expose `dropXMLInternalSubset` to extensions + for preventing billion laughs attack (and use in Imagelib) - Security fix (minor): For embedded API, avoid chance for arbitrary property setting (though this was only for trusted origins anyways) - Security fix (minor): For embedded API example, copy params to iframe @@ -520,7 +527,7 @@ git log 4bb15e0..253b4bf * Deprecated "pngsave" option called by setCustomHandlers() in favor of "exportImage" (to accommodate export of other image types). Second argument will now supply, in addition to "issues" and "svg", the properties "type" (currently 'PNG', 'JPEG', 'BMP', 'WEBP'), "mimeType", and "quality" (for 'JPEG' and 'WEBP' types). * Default extensions will now always load (along with those supplied in the URL unless the latter is prohibited by configuration), so if you do not wish your old code to load all of the default extensions, you will need to add `&noDefaultExtensions=true` to the URL (or add equivalent configuration in config.js). ext-overview_window.js can now be excluded though it is still a default. * Preferences and configuration options must be within the list supplied within svg-editor.js (should include those of all documented extensions). - * Embedded messaging will no longer work by default for privacy/data integrity reasons. One must include the "ext-xdomain-messaging.js" extension and supply an array configuration item, "allowedOrigins" with potential values including: "\*" (to allow all domains--strongly discouraged!), "null" as a string to allow file:// access, window.location.origin (to allow same domain access), or specific trusted origins. The embedded editor works without the extension if the main editor is on the same domain, but if cross-domain control is needed, the "allowedOrigins" array must be supplied by a call to svgEditor.setConfig({allowedOrigins: [origin1, origin2, etc.]}) in the new config.js file. + * Embedded messaging will no longer work by default for privacy/data integrity reasons. One must include the "ext-xdomain-messaging.js" extension and supply an array configuration item, "allowedOrigins" with potential values including: "\*" (to allow all domains--strongly discouraged!), "null" as a string to allow `file:///` access, window.location.origin (to allow same domain access), or specific trusted origins. The embedded editor works without the extension if the main editor is on the same domain, but if cross-domain control is needed, the "allowedOrigins" array must be supplied by a call to svgEditor.setConfig({allowedOrigins: [origin1, origin2, etc.]}) in the new config.js file. # 2.6 (Cycloid) - January 15th, 2013 diff --git a/editor/extensions/ext-helloworld.js b/editor/extensions/ext-helloworld.js index 172df74e..631dda5b 100644 --- a/editor/extensions/ext-helloworld.js +++ b/editor/extensions/ext-helloworld.js @@ -32,7 +32,7 @@ export default { // Must match the icon ID in helloworld-icon.xml id: 'hello_world', - // Fallback, e.g., for `file://` access + // Fallback, e.g., for `file:///` access icon: svgEditor.curConfig.extIconsPath + 'helloworld.png', // This indicates that the button will be added to the "mode" diff --git a/editor/extensions/ext-imagelib.js b/editor/extensions/ext-imagelib.js index 744a69a1..25c5f8ab 100644 --- a/editor/extensions/ext-imagelib.js +++ b/editor/extensions/ext-imagelib.js @@ -9,12 +9,12 @@ */ export default { name: 'imagelib', - async init ({decode64, importLocale}) { + async init ({decode64, importLocale, dropXMLInteralSubset}) { const imagelibStrings = await importLocale(); const svgEditor = this; const $ = jQuery; - const {uiStrings, canvas: svgCanvas} = svgEditor; + const {uiStrings, canvas: svgCanvas, curConfig: {allowedImageLibOrigins}} = svgEditor; function closeBrowser () { $('#imgbrowse_holder').hide(); @@ -44,10 +44,8 @@ export default { let transferStopped = false; let preview, submit; - window.addEventListener('message', function (evt) { - // Receive `postMessage` data - let response = evt.data; - + // Receive `postMessage` data + window.addEventListener('message', function ({origin, data: response}) { if (!response || typeof response !== 'string') { // Do nothing return; @@ -60,6 +58,10 @@ export default { if (response.namespace !== 'imagelib') { return; } + if (!allowedImageLibOrigins.includes('*') && !allowedImageLibOrigins.includes(origin)) { + console.log(`Origin ${origin} not whitelisted for posting to ${window.origin}`); + return; + } } catch (e) { return; } @@ -90,7 +92,8 @@ export default { transferStopped = false; curMeta = response; - pending[curMeta.id] = curMeta; + // Should be safe to add dynamic property as passed metadata + pending[curMeta.id] = curMeta; // lgtm [js/remote-property-injection] const name = (curMeta.name || 'file'); @@ -170,7 +173,8 @@ export default { title = curMeta.name; } else { // Try to find a title - const xml = new DOMParser().parseFromString(response, 'text/xml').documentElement; + // `dropXMLInteralSubset` is to help prevent the billion laughs attack + const xml = new DOMParser().parseFromString(dropXMLInteralSubset(response), 'text/xml').documentElement; // lgtm [js/xml-bomb] title = $(xml).children('title').first().text() || '(SVG #' + response.length + ')'; } if (curMeta) { diff --git a/editor/extensions/ext-xdomain-messaging.js b/editor/extensions/ext-xdomain-messaging.js index 5b384ac2..3a337569 100644 --- a/editor/extensions/ext-xdomain-messaging.js +++ b/editor/extensions/ext-xdomain-messaging.js @@ -19,7 +19,7 @@ export default { return; } // The default is not to allow any origins, including even the same domain or - // if run on a file:// URL See svgedit-config-es.js for an example of how + // if run on a `file:///` URL. See `svgedit-config-es.js` for an example of how // to configure const {allowedOrigins} = svgEditor.curConfig; if (!allowedOrigins.includes('*') && !allowedOrigins.includes(e.origin)) { diff --git a/editor/extensions/imagelib/index.js b/editor/extensions/imagelib/index.js index 68d33547..8274be80 100644 --- a/editor/extensions/imagelib/index.js +++ b/editor/extensions/imagelib/index.js @@ -31,7 +31,7 @@ $('a').click(function () { try { data = canvas.toDataURL(); } catch (err) { - // This fails in Firefox with file:// URLs :( + // This fails in Firefox with `file:///` URLs :( alert('Data URL conversion failed: ' + err); data = ''; } diff --git a/editor/svg-editor.js b/editor/svg-editor.js index 765e72be..9c8ec6b9 100644 --- a/editor/svg-editor.js +++ b/editor/svg-editor.js @@ -180,7 +180,8 @@ const callbacks = [], * @property {boolean} [emptyStorageOnDecline=false] Used by `ext-storage.js`; empty any prior storage if the user declines to store * @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[]} [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. @@ -293,20 +294,22 @@ let svgCanvas, urldata, extensions: [], stylesheets: [], /** - * Can use window.location.origin to indicate the current + * Can use `location.origin` to indicate the current * origin. Can contain a '*' to allow all domains or 'null' (as - * a string) to support all file:// URLs. Cannot be set by + * a string) to support all `file:///` URLs. Cannot be set by * URL for security reasons (not safe, at least for * privacy or data integrity of SVG content). * Might have been fairly safe to allow - * `new URL(window.location.href).origin` by default but + * `new URL(location.href).origin` by default but * avoiding it ensures some more security that even third * party apps on the same domain also cannot communicate * with this app by default. - * For use with ext-xdomain-messaging.js + * `allowedOrigins` is for use with `ext-xdomain-messaging.js` and + * `allowedImageLibOrigins` is for use with `ext-imagelib.js` * @todo We might instead make as a user-facing preference. */ - allowedOrigins: [] + allowedOrigins: [], + allowedImageLibOrigins: [] }; function loadSvgString (str, callback) { @@ -487,11 +490,11 @@ editor.setConfig = function (opts, cfgCfg) { } else { $.pref(key, val); } - } else if (['extensions', 'stylesheets', 'allowedOrigins'].includes(key)) { + } else if (['extensions', 'stylesheets', 'allowedOrigins', 'allowedImageLibOrigins'].includes(key)) { if (cfgCfg.overwrite === false && ( curConfig.preventAllURLConfig || - ['allowedOrigins', 'stylesheets'].includes(key) || + ['allowedOrigins', 'allowedImageLibOrigins', 'stylesheets'].includes(key) || (key === 'extensions' && curConfig.lockExtensions) ) ) { @@ -671,7 +674,7 @@ editor.init = function () { curConfig.extensions = curConfig.extensions.concat(defaultExtensions); } // ...and remove any dupes - ['extensions', 'stylesheets', 'allowedOrigins'].forEach(function (cfg) { + ['extensions', 'stylesheets', 'allowedOrigins', 'allowedImageLibOrigins'].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/svgcanvas.js b/editor/svgcanvas.js index 04a74f04..517f14c1 100644 --- a/editor/svgcanvas.js +++ b/editor/svgcanvas.js @@ -31,7 +31,7 @@ import { preventClickDefault, snapToGrid, walkTree, walkTreePost, getBBoxOfElementAsPath, convertToPath, toXml, encode64, decode64, dataURLToObjectURL, createObjectURL, - getVisibleElements, + getVisibleElements, dropXMLInteralSubset, init as utilsInit, getBBox as utilsGetBBox, getStrokedBBoxDefaultVisible } from './utilities.js'; import * as history from './history.js'; @@ -7127,6 +7127,7 @@ this.clear(); * @property {module:history.HistoryCommand} BatchCommand * @property {module:history.HistoryCommand} ChangeElementCommand * @property {module:utilities.decode64} decode64 +* @property {module:utilities.dropXMLInteralSubset} dropXMLInteralSubset * @property {module:utilities.encode64} encode64 * @property {module:svgcanvas~ffClone} ffClone * @property {module:svgcanvas~findDuplicateGradient} findDuplicateGradient @@ -7166,6 +7167,7 @@ this.getPrivateMethods = function () { BatchCommand, ChangeElementCommand, decode64, + dropXMLInteralSubset, encode64, ffClone, findDefs, diff --git a/editor/utilities.js b/editor/utilities.js index a3d82fb9..03a09b67 100644 --- a/editor/utilities.js +++ b/editor/utilities.js @@ -102,6 +102,17 @@ export const init = function (editorContext) { svgroot_ = editorContext.getSVGRoot(); }; +/** + * Used to prevent the [Billion laughs attack]{@link https://en.wikipedia.org/wiki/Billion_laughs_attack} + * @function module:utilities.dropXMLInteralSubset + * @param {string} str String to be processed + * @returns {string} The string with entity declarations in the internal subset removed + * @todo This might be needed in other places `parseFromString` is used even without LGTM flagging + */ +export const dropXMLInteralSubset = (str) => { + return str.replace(/()/, '$1$2'); +}; + /** * Converts characters in a string to XML-friendly entities. * @function module:utilities.toXml diff --git a/editor/xdomain-svgedit-config-es.js b/editor/xdomain-svgedit-config-es.js index 8a311e3a..386332f1 100644 --- a/editor/xdomain-svgedit-config-es.js +++ b/editor/xdomain-svgedit-config-es.js @@ -1,4 +1,6 @@ import svgEditor from './svg-editor.js'; svgEditor.setConfig({ - allowedOrigins: ['*'] + canvasName: 'xdomain', // Namespace this + allowedOrigins: ['*'], + allowedImageLibOrigins: ['*'] }); diff --git a/svgedit-config-es.js b/svgedit-config-es.js index c90fc147..a01aa96b 100644 --- a/svgedit-config-es.js +++ b/svgedit-config-es.js @@ -87,11 +87,12 @@ svgEditor.setConfig({ // jGraduatePath: 'jgraduate/images/', /* Uncomment the following to allow at least same domain (embedded) access, - including file:// access. + including `file:///` access. Setting as `['*']` would allow any domain to access but would be unsafe to data privacy and integrity. */ - // allowedOrigins: [window.location.origin || 'null'], // May be 'null' (as a string) when used as a file:// URL + // 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