Consider Electron version and update checks according to new defaults (#23, #58)

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 <current version..target version> | run Electron upgrade checks, eg -u 7..8 to check upgrade from Electron 7 to 8 |
+| -e, --electron-version <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                          |
 
 
@@ -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));

defaults.json

@@ -0,0 +1,68 @@
+{
+    "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,
+        "webviewTag": true
+    },
+    "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,
+        "webviewTag": false
+    }
+}

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,

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 }];
     }
   }

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, compare } 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);
       }
@@ -58,7 +59,16 @@ 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, 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 (!electronVersion) electronVersion = '0.1.0';
+
+    const all_defaults = require('../../defaults.json');
+    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) => {
       if (use_only_checks && !use_only_checks.includes(check.id)) {
         return false;
@@ -75,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);
+              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);
@@ -93,7 +103,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, defaults, electronVersion);
           if(matches){
             for(const m of matches) {
               const sample = this.get_sample(fileLines, m.line - 1);
@@ -105,7 +115,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, defaults, electronVersion);
           if (matches) {
             for(const m of matches) {
               const sample = this.get_sample(fileLines, m.line - 1);

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 <current version..target version>', 'run Electron upgrade checks, eg -u 7..8')
+  .option('-e, --electron-version <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){
@@ -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);

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._electronVersion = 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) {

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._electronVersion = dependencies.electron;
+        } catch (e) {
+          logger.warn(`Couldn't read package.json data in: ${file}`);
+        }
+      }
     }
   }
 

src/loader/loader_interface.js

@@ -1,9 +1,11 @@
 export class Loader {
   constructor() {
     this._loaded = new Set();
+    this._electronVersion = undefined;
   }
 
   get list_files() { return this._loaded; }
+  get electronVersion() { return this._electronVersion; }
 
   // eslint-disable-next-line no-unused-vars
   load_buffer(filename) {

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,9 @@ export default async function run(options, forCli = false) {
   }
 
   await loader.load(options.input);
+  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) {
     if (!severity.hasOwnProperty(options.severitySet.toUpperCase())) {
@@ -93,7 +97,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, electronVersion);
         issues.push(...result);
       } catch (error) {
         errors.push({ file: file, message: error.message, tolerable: false });

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.electronVersion.should.equal('6.1.11');
     });
   }),