feat: add sandboxing when appending to DOM in importSVG

src/containers/paper-canvas.jsx

@@ -7,6 +7,7 @@
 import Formats from '../lib/format';
 import log from '../log/log';
 
+import {getPaperSandbox} from '../helper/paper-sandbox';
 import {stripInvalidPaperData} from '../helper/strip-invalid-paper-data';
 import {performSnapshot} from '../helper/undo';
 import {undoSnapshot, clearUndoState} from '../reducers/undo';
@@ -39,6 +40,7 @@
             'onViewResize',
             'recalibrateSize'
         ]);
+        this._importGeneration = 0;
     }
     componentDidMount () {
         paper.setup(this.canvas);
@@ -196,10 +198,11 @@
         this.props.updateViewBounds(paper.view.matrix);
     }
     importSvg (svg, rotationCenterX, rotationCenterY) {
-        const paperCanvas = this;
+        this._importGeneration += 1;
+        const generation = this._importGeneration;
+
         // Pre-process SVG to prevent parsing errors (discussion from #213)
         // 1. Remove svg: namespace on elements.
-        // TODO: remove
         svg = svg.split(/<\s*svg:/).join('<');
         svg = svg.split(/<\/\s*svg:/).join('</');
         // 2. Add root svg namespace if it does not exist.
@@ -208,49 +211,56 @@
             svg = svg.replace(
                 '<svg ', '<svg xmlns="http://www.w3.org/2000/svg" ');
         }
-        // 3. Strip elements and attributes that fire on DOM-insertion. paper.js
-        // calls importSVG -> appendChild internally, so anything dangerous left
-        // in the SVG executes against the embedding origin. DOMPurify's SVG
-        // profile drops <script>, <foreignObject>, <a>, event-handler attrs,
-        // and similar. Run after the namespace fixups so DOMPurify sees a
-        // well-formed document.
+        // 3. Strip elements and attributes that fire on DOM-insertion.
+        // DOMPurify's SVG profile drops <script>, <foreignObject>, <a>,
+        // event-handler attrs, and similar. This narrows the input the
+        // sandbox sees, providing defense in depth.
         svg = sanitizeSvg.sanitizeSvgText(svg);
 
-        // 4. Parse once: read viewBox (translated back for some costumes
-        // to render correctly — paper translates it to (0, 0) on import)
-        // and strip data-paper-data values that fail JSON.parse (paper.js
-        // synchronously throws on these and aborts the whole import).
+        // 4. Parse once and strip data-paper-data values that fail
+        // JSON.parse (paper.js synchronously throws on these and aborts
+        // the whole import).
         const svgDom = new DOMParser().parseFromString(svg, 'text/xml');
         const modified = stripInvalidPaperData(svgDom);
-        const viewBox = svgDom.documentElement.attributes.viewBox ?
-            svgDom.documentElement.attributes.viewBox.value.match(/\S+/g) : null;
-        if (viewBox) {
-            for (let i = 0; i < viewBox.length; i++) {
-                viewBox[i] = parseFloat(viewBox[i]);
-            }
-        }
         if (modified) svg = new XMLSerializer().serializeToString(svgDom);
 
-        paper.project.importSVG(svg, {
-            expandShapes: true,
-            onLoad: function (item) {
-                if (!item) {
-                    log.error('SVG import failed:');
-                    log.info(svg);
-                    this.props.changeFormat(Formats.VECTOR_SKIP_CONVERT);
-                    performSnapshot(paperCanvas.props.undoSnapshot, Formats.VECTOR_SKIP_CONVERT);
-                    return;
-                }
-                item.remove();
+        // 5. Send the sanitized SVG to the sandboxed iframe where Paper.js
+        // runs importSVG (which does DOM-append) in an opaque-origin
+        // context. Receive the Paper.js JSON and viewBox back.
+        getPaperSandbox().then(sandbox => sandbox.send({svg})).then(result => {
+            // Discard the result if a newer import has already started.
+            if (generation !== this._importGeneration) return;
 
-                // Without the callback, rasters' load function has not been called yet, and they are
-                // positioned incorrectly
-                paperCanvas.queuedImport = paperCanvas.recalibrateSize(() => {
-                    paperCanvas.props.updateViewBounds(paper.view.matrix);
-                    paperCanvas.initializeSvg(item, rotationCenterX, rotationCenterY, viewBox);
-                });
+            const {paperJSON, viewBox} = result;
+
+            // Import the JSON into the parent's active layer. Paper.js's
+            // activeLayer.importJSON() creates the item, adds it to the
+            // layer, and returns it. This reconstructs the scene graph
+            // without triggering DOM insertion of untrusted SVG content.
+            const item = paper.project.activeLayer.importJSON(paperJSON);
+            if (!item) {
+                log.info(svg);
+                throw new Error('importJSON returned null');
             }
-        });
+
+            // Remove from the layer — initializeSvg re-adds with
+            // positioning, matching the original importSVG onLoad flow.
+            item.remove();
+
+            // Continue with the existing post-import flow.
+            this.queuedImport = this.recalibrateSize(() => {
+                this.props.updateViewBounds(paper.view.matrix);
+                this.initializeSvg(item, rotationCenterX, rotationCenterY, viewBox);
+            });
+        })
+            .catch(err => {
+                // Discard errors from superseded imports — the newer import
+                // is responsible for its own error handling.
+                if (generation !== this._importGeneration) return;
+                log.error('SVG import failed:', err);
+                this.props.changeFormat(Formats.VECTOR_SKIP_CONVERT);
+                performSnapshot(this.props.undoSnapshot, Formats.VECTOR_SKIP_CONVERT);
+            });
     }
     initializeSvg (item, rotationCenterX, rotationCenterY, viewBox) {
         if (this.queuedImport) this.queuedImport = null;

src/helper/paper-import-script.js

@@ -0,0 +1,95 @@
+/**
+ * Build the script string that runs inside a sandboxed iframe to perform
+ * Paper.js SVG import and JSON export.
+ *
+ * The returned script evaluates Paper.js inside the iframe, sets up a
+ * project with a canvas, and defines `window.onSandboxMessage` to handle
+ * SVG import requests. The entire `paper.project.importSVG` call — which
+ * appends parsed SVG nodes into the iframe's DOM — executes within the
+ * iframe's opaque origin. Any code execution triggered by DOM insertion
+ * (the attack vector described in the paperjs-xss analysis) is contained
+ * to the sandboxed context and cannot reach the parent origin.
+ *
+ * @param {string} paperJsSource The full source text of Paper.js
+ *     (e.g. the contents of `@scratch/paper/dist/paper-full.min.js`).
+ *     The source is evaluated inside the iframe via indirect eval.
+ * @returns {string} Script source to pass to `new Sandbox(script)`.
+ */
+const createPaperImportScript = paperJsSource => {
+    const paperSourceLiteral = JSON.stringify(paperJsSource);
+
+    // The script is eval'd inside the sandboxed iframe. It must use only
+    // ES5-compatible syntax (no arrow functions, no const/let in loops)
+    // for maximum browser compatibility, since the iframe runs whatever
+    // the browser ships natively — no transpilation.
+    return `(function () {
+    // Evaluate Paper.js; it declares a global 'paper' variable via its
+    // UMD wrapper: var paper = function(...){...}.call(this, ...)
+    (0, eval)(${paperSourceLiteral});
+
+    // Create a canvas for Paper.js to operate on. Paper needs a canvas
+    // to set up its project and coordinate system, even though we only
+    // use the SVG import/JSON export — not any visual rendering.
+    var canvas = document.createElement('canvas');
+    canvas.width = 480;
+    canvas.height = 360;
+    document.body.appendChild(canvas);
+    paper.setup(canvas);
+
+    window.onSandboxMessage = function (payload) {
+        var svg = payload.svg;
+
+        // Clear previous project state so successive imports don't
+        // accumulate items from prior calls.
+        paper.project.clear();
+
+        // Extract viewBox from the SVG DOM.
+        var viewBox = null;
+        var parser = new DOMParser();
+        var doc = parser.parseFromString(svg, 'image/svg+xml');
+        var svgEl = doc.documentElement;
+        var viewBoxAttr = svgEl.getAttribute('viewBox');
+        if (viewBoxAttr) {
+            var parts = viewBoxAttr.match(/\\S+/g);
+            if (parts) {
+                viewBox = [];
+                for (var i = 0; i < parts.length; i++) {
+                    viewBox.push(parseFloat(parts[i]));
+                }
+            }
+        }
+
+        // importSVG parses the SVG via DOMParser and then appends the
+        // parsed node into document.body during processing. This is the
+        // operation that must run inside the sandbox — any code execution
+        // triggered by DOM insertion is contained to the opaque origin.
+        //
+        // We use the onLoad callback to wait for embedded raster images
+        // (base64 data URIs in <image> elements) to finish loading before
+        // exporting JSON.
+        return new Promise(function (resolve, reject) {
+            paper.project.importSVG(svg, {
+                expandShapes: true,
+                onLoad: function (imported) {
+                    if (!imported) {
+                        reject(new Error('SVG import failed'));
+                        return;
+                    }
+
+                    // Export just the imported item's JSON (not the whole
+                    // project). The parent will use activeLayer.importJSON()
+                    // to re-create this single item.
+                    var paperJSON = imported.exportJSON({asString: true});
+
+                    resolve({paperJSON: paperJSON, viewBox: viewBox});
+                },
+                onError: function (message) {
+                    reject(new Error('SVG import error: ' + message));
+                }
+            });
+        });
+    };
+})();`;
+};
+
+export {createPaperImportScript};

src/helper/paper-sandbox.js

@@ -0,0 +1,33 @@
+import {Sandbox} from '@scratch/scratch-svg-renderer/sandbox';
+
+import {createPaperImportScript} from './paper-import-script';
+
+let paperSandboxPromise = null;
+
+/**
+ * Get or create the singleton Paper.js sandbox instance. The sandbox is
+ * lazily created on first call and reused for all subsequent imports.
+ * Paper.js source is loaded via a dynamic import (code-split chunk) to
+ * avoid doubling the main bundle size.
+ * @returns {Promise<Sandbox>} The Paper.js sandbox instance.
+ */
+const getPaperSandbox = () => {
+    if (!paperSandboxPromise) {
+        paperSandboxPromise = import(
+            /* webpackChunkName: "paper-source" */
+            '@scratch/paper/dist/paper-full.min.js?source'
+        ).then(module => {
+            const paperSource = module.default;
+            const script = createPaperImportScript(paperSource);
+            return new Sandbox(script);
+        }).catch(err => {
+            // Clear the cached promise so the next call retries rather
+            // than returning the same permanent rejection.
+            paperSandboxPromise = null;
+            throw err;
+        });
+    }
+    return paperSandboxPromise;
+};
+
+export {getPaperSandbox};

test/unit/paper-import-script.test.js

@@ -0,0 +1,18 @@
+/* eslint-env jest */
+import {createPaperImportScript} from '../../src/helper/paper-import-script';
+
+describe('createPaperImportScript', () => {
+    test('embeds the Paper.js source as a JSON-escaped literal', () => {
+        const fakePaperSource = 'var paper = {setup: function(){}, project: {}};';
+        const script = createPaperImportScript(fakePaperSource);
+        expect(script).toContain(JSON.stringify(fakePaperSource));
+    });
+
+    test('JSON-escapes special characters in Paper.js source', () => {
+        const sourceWithSpecialChars = 'var x = "hello\\nworld"; // comment with </script>';
+        const script = createPaperImportScript(sourceWithSpecialChars);
+        expect(script).toContain(JSON.stringify(sourceWithSpecialChars));
+        expect(script).toMatch(/^\(function \(\) \{/);
+        expect(script).toMatch(/\}\)\(\);$/);
+    });
+});

test/unit/sanitize-svg-import.test.js

@@ -1,16 +1,16 @@
 /* eslint-env jest */
 /**
  * Regression coverage for the SVG sanitization step that paper-canvas.jsx's
- * `importSvg` runs before `paper.project.importSVG`. Paper.js's import path
- * appends parsed SVG nodes into the document during processing, which fires
- * execution paths on `<foreignObject>`, event-handler attributes, and
- * similar features. `sanitizeSvg.sanitizeSvgText` uses DOMPurify's SVG
- * profile, which strips those shapes.
+ * `importSvg` runs before sending the SVG to the sandboxed iframe.
+ * Paper.js's import path appends parsed SVG nodes into the document during
+ * processing, which fires execution paths on `<foreignObject>`, event-handler
+ * attributes, and similar features. The sanitize step (DOMPurify's SVG
+ * profile) strips those shapes before the SVG reaches the iframe.
  *
  * Two layers of coverage: sanitizer-level assertions on hostile and
  * legitimate inputs, plus an integration assertion that `importSvg`
  * actually routes its input through the sanitizer before handing it to
- * paper.
+ * the sandbox.
  */
 
 import paper from '@scratch/paper';
@@ -20,6 +20,21 @@ import PaperCanvasConnected from '../../src/containers/paper-canvas';
 
 const PaperCanvas = PaperCanvasConnected.WrappedComponent;
 
+// Mock the paper-sandbox module to capture what gets sent to the sandbox.
+// getPaperSandbox() now returns a Promise<Sandbox>. The send mock returns
+// a never-resolving promise so the .then() chain doesn't execute (we only
+// need to assert on the input sent to the sandbox, not the full import
+// pipeline which requires layers, undo, etc.)
+jest.mock('../../src/helper/paper-sandbox', () => {
+    const sendMock = jest.fn(() => new Promise(() => {}));
+    return {
+        getPaperSandbox: () => Promise.resolve({send: sendMock}),
+        __sendMock: sendMock
+    };
+});
+
+const {__sendMock: sandboxSendMock} = require('../../src/helper/paper-sandbox');
+
 const wrap = body =>
     `<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 100 100">${body}</svg>`;
 
@@ -80,37 +95,32 @@ describe('sanitizeSvgText strips dangerous SVG shapes', () => {
     });
 });
 
-describe('PaperCanvas.importSvg routes input through sanitizeSvgText', () => {
-    let importSpy;
+describe('PaperCanvas.importSvg routes sanitized input to the sandbox', () => {
     let sanitizeSpy;
 
     beforeEach(() => {
         const canvas = document.createElement('canvas');
         paper.setup(canvas);
-        // Mock paper.project.importSVG to a no-op so we don't have to
-        // satisfy the rest of importSvg's onLoad chain — the assertion is
-        // about what gets handed to paper, not what paper does with it.
-        importSpy = jest.spyOn(paper.project, 'importSVG').mockImplementation(() => {});
         sanitizeSpy = jest.spyOn(sanitizeSvg, 'sanitizeSvgText');
+        sandboxSendMock.mockClear();
     });
 
     afterEach(() => {
-        importSpy.mockRestore();
         sanitizeSpy.mockRestore();
         while (paper.projects.length > 0) {
             paper.projects[paper.projects.length - 1].remove();
         }
     });
 
-    test('paper.project.importSVG receives input that has been through sanitizeSvgText', () => {
-        // importSvg accesses this.props.changeFormat / undoSnapshot only on
-        // the SVG-import-failed branch, and this.recalibrateSize only when
-        // the (mocked-out) onLoad fires. A minimal fakeThis with jest.fn()
-        // stand-ins is enough to call through.
-        const fakeThis = {
-            props: {changeFormat: jest.fn(), undoSnapshot: jest.fn()},
-            recalibrateSize: jest.fn()
-        };
+    const makeFakeThis = () => ({
+        _importGeneration: 0,
+        props: {changeFormat: jest.fn(), undoSnapshot: jest.fn(), updateViewBounds: jest.fn()},
+        recalibrateSize: jest.fn(),
+        queuedImport: null
+    });
+
+    test('sandbox receives input that has been through sanitizeSvgText', async () => {
+        const fakeThis = makeFakeThis();
         const hostile =
             '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10">' +
             '<foreignObject width="10" height="10">' +
@@ -121,12 +131,16 @@ describe('PaperCanvas.importSvg routes input through sanitizeSvgText', () => {
 
         PaperCanvas.prototype.importSvg.call(fakeThis, hostile, 0, 0);
 
+        // getPaperSandbox() returns a resolved Promise, so sandbox.send()
+        // is called in a microtask. Flush it before asserting.
+        await Promise.resolve();
+
         expect(sanitizeSpy).toHaveBeenCalledTimes(1);
-        expect(importSpy).toHaveBeenCalledTimes(1);
-        const svgPassedToPaper = importSpy.mock.calls[0][0];
-        expect(svgPassedToPaper).not.toMatch(/<foreignObject/i);
-        expect(svgPassedToPaper).not.toMatch(/onerror/i);
-        expect(svgPassedToPaper).not.toMatch(/onclick/i);
-        expect(svgPassedToPaper).not.toMatch(/<img/i);
+        expect(sandboxSendMock).toHaveBeenCalledTimes(1);
+        const svgSentToSandbox = sandboxSendMock.mock.calls[0][0].svg;
+        expect(svgSentToSandbox).not.toMatch(/<foreignObject/i);
+        expect(svgSentToSandbox).not.toMatch(/onerror/i);
+        expect(svgSentToSandbox).not.toMatch(/onclick/i);
+        expect(svgSentToSandbox).not.toMatch(/<img/i);
     });
 });