JS: Add ElectronShellOpenExternalSink class for Electron framework security - #4546

Here is an example to test the query.

1. create a file named InsecureOpenExternal.js

const { shell } = require('electron');
const args = process.argv.slice(2)
const url = args[0]

//BAD: `url` rely on untrusted input
shell.openExternal(url)

2. run the next command

npm i -g electron
electron InsecureOpenExternal.js https://google.co

Hi @toufik-airane 👋

Thanks for the contribution! I was a bit perplexed when you linked to PR #14, which also added some Electron support, but I'm guessing that was a coincidence.

I think it would be better suited for the ClientSideUrlRedirect query, because the attacker must be able to control the URL scheme.

As it stands we'll get FPs from code like:

shell.openExternal('https://google.com?q=' + tainted_value)

This is similar to code execution via the javascript: protocol. We understand code injection is more severe than a mere redirect, but due to the importance of controlling the prefix of the input string, we use the same query to flag both cases.

Dear @asgerf,

My bad, the link to PR #14 was a mistake! 😱
I've updated the class location to ClientSideUrlRedirect. 👍

However, I'm not sure that will succeed to detect the issue. Indeed, when I scan my example with the javascript/ql/src/Security/CWE-601/ClientSideUrlRedirect.ql it doesn't trigger.
I feel that I'm missing something to make it works.

Thanks! I'll help add some tests on Monday. Most of the JavaScript queries don't consider command-line arguments to be a source of untrusted input, so that's probably why your example wasn't flagged out of the box.

I've opened a PR against your fork with a test case.