scratchfoundation / scratch-desktop

Scratch 3.0 as a self-contained desktop application
BSD 3-Clause "New" or "Revised" License
344 stars 231 forks source link

We should disable Node integration in browser windows #100

Open cwillisf opened 4 years ago

cwillisf commented 4 years ago

Expected Behavior

Node integration should be disabled for all browser windows. Code which needs to use Node features can do so through a preload script. This isn't critical since Scratch Desktop generally doesn't display any remote content, but it wouldn't hurt to be extra careful.

Actual Behavior

Node integration is enabled on all browser windows.

See also

apple502j commented 3 years ago

@cwillisf Bumping this - and I think it's best to prioritize this more. As my exploit has shown, an XSS is enough to execute arbitrary code (currently) and Scratch app currently contains code that performs dangerous operations (e.g. manipulation of untrusted SVG.) While the issue is partially solved by adding a third party sanitizer, its bypass (such as mXSS) still allows arbitrary code execution. Disabling nodeIntegration is a simple way of reducing the risks of such potential vulnerabilities.

It's also worth mentioning that contextIsolation has to be enabled (otherwise disabling node integration is practically useless, because the sandbox is not strict enough.)

TurboWarp Desktop recently disabled node integration, could be worth checking: https://github.com/TurboWarp/desktop