From ba67bd8e63de963c2bc2632c553904f7d7a6c32d Mon Sep 17 00:00:00 2001 From: Benjamin Altpeter Date: Sat, 23 May 2020 12:33:29 +0200 Subject: [PATCH 1/6] Fixes #23: Pass electron version to checkers If the directory and ASAR loaders find a package.json file, they now check if there is a dependency on `electron` and if so, remember its version number. That version number is passed to the checkers' `match()` functions. I modified the `electron.asar` in the tests folder to include a minimal package.json file, containing: ``` { "devDependencies": { "electron": "6.1.11" } } ``` The `test_loader.js` test now also tests if this version is read correctly. --- src/finder/finder.js | 8 ++++---- src/loader/loader_asar.js | 12 +++++++++++- src/loader/loader_directory.js | 11 +++++++++++ src/loader/loader_interface.js | 2 ++ src/runner.js | 2 +- test/file_formats/electron.asar | Bin 190511 -> 190612 bytes test/test_loader.js | 7 ++++++- 7 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/finder/finder.js b/src/finder/finder.js index 4fd82d3..25b72f2 100644 --- a/src/finder/finder.js +++ b/src/finder/finder.js @@ -58,7 +58,7 @@ export class Finder { return sample; } - async find(file, data, type, content, use_only_checks = null) { + async find(file, data, type, content, use_only_checks = null, electron_version = null) { const checks = this._checks_by_type.get(type).filter((check) => { if (use_only_checks && !use_only_checks.includes(check.id)) { return false; @@ -75,7 +75,7 @@ export class Finder { enter: (node) => { rootData.Scope.updateFunctionScope(rootData.astParser.getNode(node), "enter"); for (const check of checks) { - const matches = check.match(rootData.astParser.getNode(node), rootData.astParser, rootData.Scope); + const matches = check.match(rootData.astParser.getNode(node), rootData.astParser, rootData.Scope, electron_version); if (matches) { for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); @@ -93,7 +93,7 @@ export class Finder { break; case sourceTypes.HTML: for (const check of checks) { - const matches = check.match(data, content); + const matches = check.match(data, content, electron_version); if(matches){ for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); @@ -105,7 +105,7 @@ export class Finder { break; case sourceTypes.JSON: for (const check of checks) { - const matches = await check.match(data); + const matches = await check.match(data, electron_version); if (matches) { for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); diff --git a/src/loader/loader_asar.js b/src/loader/loader_asar.js index b15c22b..b13accc 100644 --- a/src/loader/loader_asar.js +++ b/src/loader/loader_asar.js @@ -12,6 +12,8 @@ export class LoaderAsar extends Loader { // returns map filename -> content load(archive) { + this.archive = archive; + const archived_files = asar.listPackage(archive); logger.debug(`Files in ASAR archive: ${archived_files}`); @@ -23,6 +25,15 @@ export class LoaderAsar extends Loader { case 'json': if (f.toLowerCase().indexOf('package.json') < 0) continue; + else { + try { + const pjson_data = JSON.parse(this.load_buffer(f)); + const dependencies = Object.assign({}, pjson_data.devDependencies, pjson_data.dependencies); + if (dependencies.electron) this._electron_version = dependencies.electron; + } catch (e) { + logger.warn(`Couldn't read package.json data in: ${f}`); + } + } // eslint-disable-next-line no-fallthrough case 'js': case 'jsx': @@ -39,7 +50,6 @@ export class LoaderAsar extends Loader { } logger.debug(`Discovered ${this.list_files.size} files`); - this.archive = archive; } load_buffer(filename) { diff --git a/src/loader/loader_directory.js b/src/loader/loader_directory.js index e51b5ad..48e0216 100644 --- a/src/loader/loader_directory.js +++ b/src/loader/loader_directory.js @@ -1,3 +1,5 @@ +import logger from 'winston'; + import { read_file, list_files } from '../util'; import { Loader } from './loader_interface'; @@ -11,6 +13,15 @@ export class LoaderDirectory extends Loader { for (const file of files) { this._loaded.add(file); + if (file.endsWith('package.json')) { + try { + const pjson_data = JSON.parse(this.load_buffer(file)); + const dependencies = Object.assign({}, pjson_data.devDependencies, pjson_data.dependencies); + if (dependencies.electron) this._electron_version = dependencies.electron; + } catch (e) { + logger.warn(`Couldn't read package.json data in: ${file}`); + } + } } } diff --git a/src/loader/loader_interface.js b/src/loader/loader_interface.js index 3bdbf91..b07bc41 100644 --- a/src/loader/loader_interface.js +++ b/src/loader/loader_interface.js @@ -1,9 +1,11 @@ export class Loader { constructor() { this._loaded = new Set(); + this._electron_version = undefined; } get list_files() { return this._loaded; } + get electron_version() { return this._electron_version; } // eslint-disable-next-line no-unused-vars load_buffer(filename) { diff --git a/src/runner.js b/src/runner.js index ec2adcb..9801639 100644 --- a/src/runner.js +++ b/src/runner.js @@ -93,7 +93,7 @@ export default async function run(options, forCli = false) { } } - const result = await finder.find(file, data, type, content); + const result = await finder.find(file, data, type, content, null, loader.electron_version); issues.push(...result); } catch (error) { errors.push({ file: file, message: error.message, tolerable: false }); diff --git a/test/file_formats/electron.asar b/test/file_formats/electron.asar index d80b4f26da49b2c41044d5e4b782bf451214f8b0..8a4d55be5e04eaef577370bf156b05e7f876f190 100644 GIT binary patch delta 339 zcmX9)KTE?<986w>HZLhiQ4~aczDe5j&ujj@2BZj1`W0MC`>IuGOH(XZGKmOI%1?+m zSOh^gacUP=hYn)t;ur8kxS8m|EqBMk-OHzTGu2Xak|aIrlJpqy{xx-wG&|MWQFUMB z5B+wFdIRb=PX+aC*P!i2!xug3QN}FhIy5k7SG4M)drj8hE?H1YrU{iq4t|npcDmF2)miG@hXT>MsyZUbX-L delta 219 zcmbPok$e3GZeA7!28J3Q28IeCF5JkQ!7+IeM+mc_x!GiUcHPNpoI*^7CX=l=4VjHC z3@7JtYBL!dPQJ)2E^KP3qm-YPR-9U*WTj+iWMN=wIr%83HnX9r!Q{`J`phQgmXlSv zY?zD;CMR>*GZ`6AUc#lpWMnz{9G4cexrycE-&}@FMkbT>xV4y!jV4EN>i}&qpWMl< z15&%4+nd?I(sVK_j|Gzv&?!8o%qC_AlgoK@xlN3eY9WB3c^%L8bv%qr`=&o!&ScUq Ndzx{(>}jTQSpca0I*0%O diff --git a/test/test_loader.js b/test/test_loader.js index 25b6453..35eb3be 100644 --- a/test/test_loader.js +++ b/test/test_loader.js @@ -37,7 +37,12 @@ describe('Loader classes', () => { it('extracts file from ASAR', () => { loader.load(test_files.get('asar')); - loader.list_files.size.should.equal(60); + loader.list_files.size.should.equal(61); + }); + + it('finds Electron version number in package.json', () => { + loader.load(test_files.get('asar')); + loader.electron_version.should.equal('6.1.11'); }); }), From 85896b8eded6621981b5b2d705ed8858f964d358 Mon Sep 17 00:00:00 2001 From: Benjamin Altpeter Date: Sat, 23 May 2020 13:06:13 +0200 Subject: [PATCH 2/6] Assume first Electron version if we couldn't detect one This simplifies the checker code and maintains the condition to run all checks if no version can be detected. --- src/finder/finder.js | 4 ++++ src/runner.js | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/finder/finder.js b/src/finder/finder.js index 25b72f2..dea0fa0 100644 --- a/src/finder/finder.js +++ b/src/finder/finder.js @@ -59,6 +59,10 @@ export class Finder { } async find(file, data, type, content, use_only_checks = null, electron_version = null) { + // If the loader didn't detect the Electron version, assume the first one. Not knowing the version, we have to assume the worst (i.e. + // all options defaulting to insecure values). By always setting the version here, the code in the checkers is simplified as they now + // don't have to handle the case of unknown versions. + if (!electron_version) electron_version = '0.1.0'; const checks = this._checks_by_type.get(type).filter((check) => { if (use_only_checks && !use_only_checks.includes(check.id)) { return false; diff --git a/src/runner.js b/src/runner.js index 9801639..e9f90bc 100644 --- a/src/runner.js +++ b/src/runner.js @@ -1,6 +1,7 @@ import cliProgress from 'cli-progress'; import Table from 'cli-table3'; import chalk from 'chalk'; +import logger from 'winston'; import { LoaderFile, LoaderAsar, LoaderDirectory } from './loader'; import { Parser } from './parser'; @@ -28,6 +29,8 @@ export default async function run(options, forCli = false) { } await loader.load(options.input); + if (!loader.electron_version) + logger.warn("Couldn't detect Electron version, assuming v0.1.0. Defaults have probably changed for your actual version, please check manually."); if (options.severitySet) { if (!severity.hasOwnProperty(options.severitySet.toUpperCase())) { From 29a86279d29fbf2a9961037f28c2f736f971d361 Mon Sep 17 00:00:00 2001 From: Benjamin Altpeter Date: Mon, 29 Jun 2020 14:15:50 +0200 Subject: [PATCH 3/6] Pass webPreferences defaults to checkers --- defaults.json | 37 +++++++++++++++++++++++++++++++++++++ src/finder/finder.js | 18 ++++++++++++------ 2 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 defaults.json diff --git a/defaults.json b/defaults.json new file mode 100644 index 0000000..daa13d3 --- /dev/null +++ b/defaults.json @@ -0,0 +1,37 @@ +{ + "0.1.0": { + "allowRunningInsecureContent": false, + "contextIsolation": false, + "experimentalFeatures": false, + "images": true, + "javascript": true, + "nativeWindowOpen": false, + "nodeIntegration": true, + "nodeIntegrationInWorker": false, + "offscreen": false, + "plugins": false, + "sandbox": false, + "textAreasAreResizable": true, + "webSecurity": true, + "webgl": true, + "webviewTag": true + }, + "5.0.0": { + "allowRunningInsecureContent": false, + "contextIsolation": false, + "experimentalFeatures": false, + "images": true, + "javascript": true, + "nativeWindowOpen": false, + "nodeIntegration": false, + "nodeIntegrationInSubFrames": false, + "nodeIntegrationInWorker": false, + "offscreen": false, + "plugins": false, + "sandbox": false, + "textAreasAreResizable": true, + "webSecurity": true, + "webgl": true, + "webviewTag": false + } +} diff --git a/src/finder/finder.js b/src/finder/finder.js index dea0fa0..e73cc0d 100644 --- a/src/finder/finder.js +++ b/src/finder/finder.js @@ -2,15 +2,16 @@ import { CHECKS } from './checks/AtomicChecks'; import { sourceTypes } from '../parser/types'; import { ELECTRON_ATOMIC_UPGRADE_CHECKS } from './checks/AtomicChecks/ElectronAtomicUpgradeChecks'; import chalk from 'chalk'; +import { gte } from 'semver'; export class Finder { constructor(customScan, electronUpgrade) { - let candidateChecks = Array.from(CHECKS) + let candidateChecks = Array.from(CHECKS) if (electronUpgrade) { const [currentVersion, targetVersion] = electronUpgrade.split('..'); if (currentVersion && targetVersion) { Object.keys(ELECTRON_ATOMIC_UPGRADE_CHECKS).forEach(versionToCheck => { - if (versionToCheck > currentVersion && versionToCheck <= targetVersion) { + if (versionToCheck > currentVersion && versionToCheck <= targetVersion) { candidateChecks = candidateChecks.concat(ELECTRON_ATOMIC_UPGRADE_CHECKS[versionToCheck]); } }) @@ -26,7 +27,7 @@ export class Finder { console.log(chalk.red(`You have an error in your custom checks list. Maybe you misspelt some check names?`)); process.exit(1); } else { - for (var i = this._enabled_checks.length - 1; i >= 0; i--) + for (var i = this._enabled_checks.length - 1; i >= 0; i--) if (!customScan.includes(this._enabled_checks[i].name.toLowerCase())) this._enabled_checks.splice(i, 1); } @@ -63,6 +64,11 @@ export class Finder { // all options defaulting to insecure values). By always setting the version here, the code in the checkers is simplified as they now // don't have to handle the case of unknown versions. if (!electron_version) electron_version = '0.1.0'; + + const all_defaults = require('../../defaults.json'); + const version_of_last_default_change = Object.keys(all_defaults).sort().reverse().find(current_version => gte(electron_version, current_version)); + const defaults = all_defaults[version_of_last_default_change]; + const checks = this._checks_by_type.get(type).filter((check) => { if (use_only_checks && !use_only_checks.includes(check.id)) { return false; @@ -79,7 +85,7 @@ export class Finder { enter: (node) => { rootData.Scope.updateFunctionScope(rootData.astParser.getNode(node), "enter"); for (const check of checks) { - const matches = check.match(rootData.astParser.getNode(node), rootData.astParser, rootData.Scope, electron_version); + const matches = check.match(rootData.astParser.getNode(node), rootData.astParser, rootData.Scope, defaults, electron_version); if (matches) { for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); @@ -97,7 +103,7 @@ export class Finder { break; case sourceTypes.HTML: for (const check of checks) { - const matches = check.match(data, content, electron_version); + const matches = check.match(data, content, defaults, electron_version); if(matches){ for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); @@ -109,7 +115,7 @@ export class Finder { break; case sourceTypes.JSON: for (const check of checks) { - const matches = await check.match(data, electron_version); + const matches = await check.match(data, defaults, electron_version); if (matches) { for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); From 3d1beb701a43d24c0e7d2c5b46462cf0665db5fa Mon Sep 17 00:00:00 2001 From: Benjamin Altpeter Date: Mon, 29 Jun 2020 14:23:36 +0200 Subject: [PATCH 4/6] Update NodeIntegrationJSCheck to respect the default changes --- src/finder/checks/AtomicChecks/NodeIntegrationJSCheck.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/finder/checks/AtomicChecks/NodeIntegrationJSCheck.js b/src/finder/checks/AtomicChecks/NodeIntegrationJSCheck.js index 4578bb5..d8b3d8a 100644 --- a/src/finder/checks/AtomicChecks/NodeIntegrationJSCheck.js +++ b/src/finder/checks/AtomicChecks/NodeIntegrationJSCheck.js @@ -13,7 +13,7 @@ export default class NodeIntegrationJSCheck { //nodeIntegrationInWorker Boolean (optional) - Whether node integration is enabled in web workers. Default is false //nodeIntegrationInSubFrames Boolean (optional) - Whether node integration is enabled in in sub-frames such as iframes. Default is false - match(astNode, astHelper, scope){ + match(astNode, astHelper, scope, defaults){ if (astNode.type !== 'NewExpression') return null; if (astNode.callee.name !== 'BrowserWindow' && astNode.callee.name !== 'BrowserView') return null; @@ -37,7 +37,7 @@ export default class NodeIntegrationJSCheck { locations = locations.concat(loc); } - if (!nodeIntegrationFound) { + if (!nodeIntegrationFound && defaults.nodeIntegration) { locations.push({ line: astNode.loc.start.line, column: astNode.loc.start.column, id: this.id, description: this.description, shortenedURL: this.shortenedURL, severity: severity.HIGH, confidence: confidence.FIRM, manualReview: false }); } @@ -61,7 +61,7 @@ export default class NodeIntegrationJSCheck { if ((node.key.value === "sandbox" || node.key.name === "sandbox") && isIdentifier) continue; if ((nodeIntegrationStrings.includes(node.key.value) || nodeIntegrationStrings.includes(node.key.name)) && !isIdentifier) continue; } - + locations.push({ line: node.key.loc.start.line, column: node.key.loc.start.column, From 6c864b9f5898b28afe8dc306b99121496fc80648 Mon Sep 17 00:00:00 2001 From: Benjamin Altpeter Date: Mon, 29 Jun 2020 14:33:08 +0200 Subject: [PATCH 5/6] Allow overriding the detected Electron version --- README.md | 5 ++++- src/index.js | 18 ++++++++++-------- src/runner.js | 5 +++-- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 31947cd..7c90b8c 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ $ electronegativity -h | -r, --relative | show relative path for files | | -v, --verbose | show the description for the findings | | -u, --upgrade | run Electron upgrade checks, eg -u 7..8 to check upgrade from Electron 7 to 8 | +| -e, --electron-version | assume the set Electron version, overriding the detected one, eg -e 7.0.0 | | -h, --help | output usage information | @@ -86,7 +87,9 @@ run({ // show relative path for files (optional) isRelative: false, // run Electron upgrade checks, eg -u 7..8 to check upgrade from Electron 7 to 8 (optional) - electronUpgrade: '7..8' + electronUpgrade: '7..8', + // assume the set Electron version, overriding the detected one + electronVersion: '5.0.0' }) .then(result => console.log(result)) .catch(err => console.error(err)); diff --git a/src/index.js b/src/index.js index 8daffb8..d15ab32 100644 --- a/src/index.js +++ b/src/index.js @@ -8,11 +8,11 @@ import run from './runner.js'; const VER = require('../package.json').version; console.log(` -▄▄▄ ▄▄▌ ▄▄▄ .▄▄·▄▄▄▄▄▄▄ -▀▄.▀██• ▀▄.▀▐█ ▌•██ ▀▄ █▪ -▐▀▀▪██▪ ▐▀▀▪██ ▄▄▐█.▐▀▀▄ ▄█▀▄ -▐█▄▄▐█▌▐▐█▄▄▐███▌▐█▌▐█•█▐█▌.▐▌ - ▀▀▀.▀▀▀ ▀▀▀·▀▀▀ ▀▀▀.▀ ▀▀█▄▀▪ +▄▄▄ ▄▄▌ ▄▄▄ .▄▄·▄▄▄▄▄▄▄ +▀▄.▀██• ▀▄.▀▐█ ▌•██ ▀▄ █▪ +▐▀▀▪██▪ ▐▀▀▪██ ▄▄▐█.▐▀▀▄ ▄█▀▄ +▐█▄▄▐█▌▐▐█▄▄▐███▌▐█▌▐█•█▐█▌.▐▌ + ▀▀▀.▀▀▀ ▀▀▀·▀▀▀ ▀▀▀.▀ ▀▀█▄▀▪ ▐ ▄▄▄▄ .▄▄ • ▄▄▄▄▄▄▄▪ ▌ ▐▪▄▄▄▄▄▄· ▄▌ •█▌▐▀▄.▀▐█ ▀ ▐█ ▀•██ ██▪█·██•██ ▐█▪██▌ ▐█▐▐▐▀▀▪▄█ ▀█▄█▀▀█▐█.▐█▐█▐█▐█▐█.▐█▌▐█▪ @@ -33,6 +33,7 @@ program .option('-r, --relative', 'show relative path for files') .option('-v, --verbose', 'show the description for the findings') .option('-u, --upgrade ', 'run Electron upgrade checks, eg -u 7..8') + .option('-e, --electron-version ', 'assume the set Electron version, overriding the detected one, eg -e 7.0.0') .parse(process.argv); if(!program.input){ @@ -63,12 +64,13 @@ const forCli = true; run({ input, - output: program.output, + output: program.output, isSarif: program.fileFormat === 'sarif', customScan: program.checks, - severitySet: program.severity, + severitySet: program.severity, confidenceSet: program.confidence, isRelative: program.relative, isVerbose: program.verbose, - electronUpgrade: program.upgrade + electronUpgrade: program.upgrade, + electronVersionOverride: program.electronVersion }, forCli); diff --git a/src/runner.js b/src/runner.js index e9f90bc..3c0f310 100644 --- a/src/runner.js +++ b/src/runner.js @@ -29,7 +29,8 @@ export default async function run(options, forCli = false) { } await loader.load(options.input); - if (!loader.electron_version) + const electron_version = options.electronVersionOverride || loader.electron_version; + if (!electron_version) logger.warn("Couldn't detect Electron version, assuming v0.1.0. Defaults have probably changed for your actual version, please check manually."); if (options.severitySet) { @@ -96,7 +97,7 @@ export default async function run(options, forCli = false) { } } - const result = await finder.find(file, data, type, content, null, loader.electron_version); + const result = await finder.find(file, data, type, content, null, electron_version); issues.push(...result); } catch (error) { errors.push({ file: file, message: error.message, tolerable: false }); From 52780881b529e48017c77b8a3022780be37adcd8 Mon Sep 17 00:00:00 2001 From: Benjamin Altpeter Date: Tue, 30 Jun 2020 21:13:24 +0200 Subject: [PATCH 6/6] Changes as discussed in #66 * Add enableRemoteModule, safeDialogs, navigateOnDragDrop and spellcheck to defaults.json. * Add preliminary defaults for Electron 10. * Update remote module check for Electron 10. * Correctly sort versions from defaults.json. * Explain the --electrobit more. * Rename electron_version to electronVersion. --- README.md | 2 +- defaults.json | 31 +++++++++++++++++++ .../AtomicChecks/RemoteModuleJSCheck.js | 4 +-- src/finder/finder.js | 14 ++++----- src/index.js | 2 +- src/loader/loader_asar.js | 2 +- src/loader/loader_directory.js | 2 +- src/loader/loader_interface.js | 4 +-- src/runner.js | 6 ++-- test/test_loader.js | 2 +- 10 files changed, 50 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 7c90b8c..d6b33f0 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ $ electronegativity -h | -r, --relative | show relative path for files | | -v, --verbose | show the description for the findings | | -u, --upgrade | run Electron upgrade checks, eg -u 7..8 to check upgrade from Electron 7 to 8 | -| -e, --electron-version | assume the set Electron version, overriding the detected one, eg -e 7.0.0 | +| -e, --electron-version | assume the set Electron version, overriding the detected one, eg -e 7.0.0 to treat as using Electron 7 | | -h, --help | output usage information | diff --git a/defaults.json b/defaults.json index daa13d3..b2f3fd1 100644 --- a/defaults.json +++ b/defaults.json @@ -2,15 +2,19 @@ "0.1.0": { "allowRunningInsecureContent": false, "contextIsolation": false, + "enableRemoteModule": true, "experimentalFeatures": false, "images": true, "javascript": true, "nativeWindowOpen": false, + "navigateOnDragDrop": false, "nodeIntegration": true, "nodeIntegrationInWorker": false, "offscreen": false, "plugins": false, + "safeDialogs": false, "sandbox": false, + "spellcheck": true, "textAreasAreResizable": true, "webSecurity": true, "webgl": true, @@ -19,16 +23,43 @@ "5.0.0": { "allowRunningInsecureContent": false, "contextIsolation": false, + "enableRemoteModule": true, "experimentalFeatures": false, "images": true, "javascript": true, "nativeWindowOpen": false, + "navigateOnDragDrop": false, "nodeIntegration": false, "nodeIntegrationInSubFrames": false, "nodeIntegrationInWorker": false, "offscreen": false, "plugins": false, + "safeDialogs": false, "sandbox": false, + "spellcheck": true, + "textAreasAreResizable": true, + "webSecurity": true, + "webgl": true, + "webviewTag": false + }, + "10.0.0": { + "allowRunningInsecureContent": false, + "contextIsolation": false, + "enableRemoteModule": false, + "enableWebSQL": true, + "experimentalFeatures": false, + "images": true, + "javascript": true, + "nativeWindowOpen": false, + "navigateOnDragDrop": false, + "nodeIntegration": false, + "nodeIntegrationInSubFrames": false, + "nodeIntegrationInWorker": false, + "offscreen": false, + "plugins": false, + "safeDialogs": false, + "sandbox": false, + "spellcheck": true, "textAreasAreResizable": true, "webSecurity": true, "webgl": true, diff --git a/src/finder/checks/AtomicChecks/RemoteModuleJSCheck.js b/src/finder/checks/AtomicChecks/RemoteModuleJSCheck.js index 484367b..9d8d1ca 100644 --- a/src/finder/checks/AtomicChecks/RemoteModuleJSCheck.js +++ b/src/finder/checks/AtomicChecks/RemoteModuleJSCheck.js @@ -9,7 +9,7 @@ export default class RemoteModuleJSCheck { this.shortenedURL = "https://git.io/JvqrQ"; } - match(astNode, astHelper, scope){ + match(astNode, astHelper, scope, defaults){ if (astNode.type !== 'NewExpression') return null; if (astNode.callee.name !== 'BrowserWindow' && astNode.callee.name !== 'BrowserView') return null; @@ -36,7 +36,7 @@ export default class RemoteModuleJSCheck { if (wasFound) { return loc; - } else { // default is module 'remote' enabled (assuming nodeIntegration:true), which is a misconfiguration + } else if (defaults.enableRemoteModule) { // in earlier versions, 'remote' is enabled by default (assuming nodeIntegration:true), which is a misconfiguration return [{ line: astNode.loc.start.line, column: astNode.loc.start.column, id: this.id, description: this.description, shortenedURL: this.shortenedURL, severity: severity.MEDIUM, confidence: confidence.TENTATIVE, manualReview: true }]; } } diff --git a/src/finder/finder.js b/src/finder/finder.js index e73cc0d..95f5396 100644 --- a/src/finder/finder.js +++ b/src/finder/finder.js @@ -2,7 +2,7 @@ import { CHECKS } from './checks/AtomicChecks'; import { sourceTypes } from '../parser/types'; import { ELECTRON_ATOMIC_UPGRADE_CHECKS } from './checks/AtomicChecks/ElectronAtomicUpgradeChecks'; import chalk from 'chalk'; -import { gte } from 'semver'; +import { gte, compare } from 'semver'; export class Finder { constructor(customScan, electronUpgrade) { @@ -59,14 +59,14 @@ export class Finder { return sample; } - async find(file, data, type, content, use_only_checks = null, electron_version = null) { + async find(file, data, type, content, use_only_checks = null, electronVersion = null) { // If the loader didn't detect the Electron version, assume the first one. Not knowing the version, we have to assume the worst (i.e. // all options defaulting to insecure values). By always setting the version here, the code in the checkers is simplified as they now // don't have to handle the case of unknown versions. - if (!electron_version) electron_version = '0.1.0'; + if (!electronVersion) electronVersion = '0.1.0'; const all_defaults = require('../../defaults.json'); - const version_of_last_default_change = Object.keys(all_defaults).sort().reverse().find(current_version => gte(electron_version, current_version)); + const version_of_last_default_change = Object.keys(all_defaults).sort((a, b) => compare(a, b)).reverse().find(current_version => gte(electronVersion, current_version)); const defaults = all_defaults[version_of_last_default_change]; const checks = this._checks_by_type.get(type).filter((check) => { @@ -85,7 +85,7 @@ export class Finder { enter: (node) => { rootData.Scope.updateFunctionScope(rootData.astParser.getNode(node), "enter"); for (const check of checks) { - const matches = check.match(rootData.astParser.getNode(node), rootData.astParser, rootData.Scope, defaults, electron_version); + const matches = check.match(rootData.astParser.getNode(node), rootData.astParser, rootData.Scope, defaults, electronVersion); if (matches) { for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); @@ -103,7 +103,7 @@ export class Finder { break; case sourceTypes.HTML: for (const check of checks) { - const matches = check.match(data, content, defaults, electron_version); + const matches = check.match(data, content, defaults, electronVersion); if(matches){ for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); @@ -115,7 +115,7 @@ export class Finder { break; case sourceTypes.JSON: for (const check of checks) { - const matches = await check.match(data, defaults, electron_version); + const matches = await check.match(data, defaults, electronVersion); if (matches) { for(const m of matches) { const sample = this.get_sample(fileLines, m.line - 1); diff --git a/src/index.js b/src/index.js index d15ab32..1526ad0 100644 --- a/src/index.js +++ b/src/index.js @@ -33,7 +33,7 @@ program .option('-r, --relative', 'show relative path for files') .option('-v, --verbose', 'show the description for the findings') .option('-u, --upgrade ', 'run Electron upgrade checks, eg -u 7..8') - .option('-e, --electron-version ', 'assume the set Electron version, overriding the detected one, eg -e 7.0.0') + .option('-e, --electron-version ', 'assume the set Electron version, overriding the detected one, eg -e 7.0.0 to treat as using Electron 7') .parse(process.argv); if(!program.input){ diff --git a/src/loader/loader_asar.js b/src/loader/loader_asar.js index b13accc..1e166ec 100644 --- a/src/loader/loader_asar.js +++ b/src/loader/loader_asar.js @@ -29,7 +29,7 @@ export class LoaderAsar extends Loader { try { const pjson_data = JSON.parse(this.load_buffer(f)); const dependencies = Object.assign({}, pjson_data.devDependencies, pjson_data.dependencies); - if (dependencies.electron) this._electron_version = dependencies.electron; + if (dependencies.electron) this._electronVersion = dependencies.electron; } catch (e) { logger.warn(`Couldn't read package.json data in: ${f}`); } diff --git a/src/loader/loader_directory.js b/src/loader/loader_directory.js index 48e0216..2f32fb2 100644 --- a/src/loader/loader_directory.js +++ b/src/loader/loader_directory.js @@ -17,7 +17,7 @@ export class LoaderDirectory extends Loader { try { const pjson_data = JSON.parse(this.load_buffer(file)); const dependencies = Object.assign({}, pjson_data.devDependencies, pjson_data.dependencies); - if (dependencies.electron) this._electron_version = dependencies.electron; + if (dependencies.electron) this._electronVersion = dependencies.electron; } catch (e) { logger.warn(`Couldn't read package.json data in: ${file}`); } diff --git a/src/loader/loader_interface.js b/src/loader/loader_interface.js index b07bc41..89267c3 100644 --- a/src/loader/loader_interface.js +++ b/src/loader/loader_interface.js @@ -1,11 +1,11 @@ export class Loader { constructor() { this._loaded = new Set(); - this._electron_version = undefined; + this._electronVersion = undefined; } get list_files() { return this._loaded; } - get electron_version() { return this._electron_version; } + get electronVersion() { return this._electronVersion; } // eslint-disable-next-line no-unused-vars load_buffer(filename) { diff --git a/src/runner.js b/src/runner.js index 3c0f310..d8c046d 100644 --- a/src/runner.js +++ b/src/runner.js @@ -29,8 +29,8 @@ export default async function run(options, forCli = false) { } await loader.load(options.input); - const electron_version = options.electronVersionOverride || loader.electron_version; - if (!electron_version) + const electronVersion = options.electronVersionOverride || loader.electronVersion; + if (!electronVersion) logger.warn("Couldn't detect Electron version, assuming v0.1.0. Defaults have probably changed for your actual version, please check manually."); if (options.severitySet) { @@ -97,7 +97,7 @@ export default async function run(options, forCli = false) { } } - const result = await finder.find(file, data, type, content, null, electron_version); + const result = await finder.find(file, data, type, content, null, electronVersion); issues.push(...result); } catch (error) { errors.push({ file: file, message: error.message, tolerable: false }); diff --git a/test/test_loader.js b/test/test_loader.js index 35eb3be..fbb0369 100644 --- a/test/test_loader.js +++ b/test/test_loader.js @@ -42,7 +42,7 @@ describe('Loader classes', () => { it('finds Electron version number in package.json', () => { loader.load(test_files.get('asar')); - loader.electron_version.should.equal('6.1.11'); + loader.electronVersion.should.equal('6.1.11'); }); }),