[BUG] Paper.js has a XSS vulnerability with `Project.importSVG()`

[InternetAstronaut]

paperjs/paper.js#2100

This vulnerability has been reported in the Paper.js repository by @GarboMuffin (e.g.: the guy that makes turbowarp), not only this but the general abandonment of development on Paper.js has me worried that we should consider migrating or re-making the 'view' part of Candlestick.

[Alani25]

I've looked into the vulnerability you mentioned and that's actually pretty useful information especially considering we've started chatting recently about SVG import. We may not be able to allow SVG imports with this risk at hand, however we may be able to add an extra layer of protection where we scan uploaded SVG files for malicious content before running it through paperJS (there seems to be a few libraries around this).
On another note, I do not believe we'd be able to exactly migrate to another engine as paperJS is heavily integrated into the engine, it'd be a major engine overhaul.
Thank you for bringing up this major point

[InternetAstronaut]

On another note, I do not believe we'd be able to exactly migrate to another engine as paperJS is heavily integrated into the engine, it'd be a major engine overhaul.

I have been thinking of this for a very long time, why not just build a wrapper on top of Pixi.js (there is no other choice for the goals that a renderer change would need) which implements Paper.js APIs?

[Alani25]

PixiJS and PaperJS are far too different; PaperJS is built around a vector math model while PixiJS is a sprite/ texture renderer.
We'd be rewriting most of PaperJS in the form of a wrapper.
It'd be far easier to just fork PaperJS and update it ourselves to make it more efficient than to rewrite it on top of pixiJS (in all cases we'd still come by roadblocks of some sort).
Though honestly forking PaperJS doesn't seem like too much of a bad idea if we just wanna push minor yet highly effective fixes… it hasn't been updated for some time…

Ofc reading the message chain you linked at the top with GarboMuffin, I doubt that fork (if we make it) would even be noticed by paperJS

[hobbsythe6th]

I think we can legally fork paper and use the fork instead of pushing it to the original, but not sure

[hobbsythe6th]

scratchfoundation/paper.js@a3a4a8c this is @GarboMuffin's code

[StickmanRed]

I agree with forking Turbowarp/paper.js. All the changes look compatible and beneficial to Wick, also I really like that PaperScript and tweening are removed in this fork, since they aren't used at all in Wick. We could even consider removing the engine/src/view/paper-ext files and directly adding them to our custom paper.js library

On another note, I do not believe we'd be able to exactly migrate to another engine as paperJS is heavily integrated into the engine, it'd be a major engine overhaul.

I was thinking to abstract Wick from paper.js as much as possible, but in small steps. For example not using paper objects elsewhere than engine/src/view, opting for custom objects like Wick.Color. (Some work on separating paper.js was even started in original Wick Editor development: https://github.com/Wicklets/wick-editor/commits/decouple-view-from-engine/)

[InternetAstronaut]

I was thinking to abstract Wick from paper.js as much as possible, but in small steps. For example not using paper objects elsewhere than engine/src/view, opting for custom objects like Wick.Color. (Some work on separating paper.js was even started in original Wick Editor development: https://github.com/Wicklets/wick-editor/commits/decouple-view-from-engine/)

Aside from the deprecated hitTest() method, Wick only depends fully on Paper for the Paths.