Giter Site home page Giter Site logo

doyensec / electronegativity Goto Github PK

View Code? Open in Web Editor NEW
933.0 26.0 64.0 9.83 MB

Electronegativity is a tool to identify misconfigurations and security anti-patterns in Electron applications.

License: Apache License 2.0

JavaScript 88.08% HTML 4.73% TypeScript 7.18%
security electron electron-app nodejs

electronegativity's Introduction

Electronegativity

What's Electronegativity?

Electronegativity is a tool to identify misconfigurations and security anti-patterns in Electron-based applications.

It leverages AST and DOM parsing to look for security-relevant configurations, as described in the "Electron Security Checklist - A Guide for Developers and Auditors" whitepaper.

Software developers and security auditors can use this tool to detect and mitigate potential weaknesses and implementation bugs when developing applications using Electron. A good understanding of Electron (in)security is still required when using Electronegativity, as some of the potential issues detected by the tool require manual investigation.

If you're interested in Electron Security, have a look at our BlackHat 2017 research Electronegativity - A Study of Electron Security and keep an eye on the Doyensec's blog.

Electronegativity Demo

ElectroNG Improved Version

If you need something more powerful or updated, an improved SAST tool based on Electronegativity is available as the result of many years of applied R&D from Doyensec. At the end of 2020, we sat down to create a project roadmap and created a development team to work on what is now ElectroNG. You can read more some of the major improvements over the OSS version in a recent blog post.

Installation

Major releases are pushed to NPM and can be simply installed using:

$ npm install @doyensec/electronegativity -g

Usage

CLI

$ electronegativity -h
Option Description
-V output the version number
-i, --input input (directory, .js, .html, .asar)
-l, --checks only run the specified checks, passed in csv format
-x, --exclude-checks skip the specified checks list, passed in csv format
-s, --severity only return findings with the specified level of severity or above
-c, --confidence only return findings with the specified level of confidence or above
-o, --output <filename[.csv or .sarif]> save the results to a file in csv or sarif format
-r, --relative show relative path for files
-v, --verbose show the description for the findings, defaults to true
-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 to treat as using Electron 7
-p, --parser-plugins specify additional parser plugins to use separated by commas, e.g. -p optionalChaining
-h, --help output usage information

Using electronegativity to look for issues in a directory containing an Electron app:

$ electronegativity -i /path/to/electron/app

Using electronegativity to look for issues in an asar archive and saving the results in a csv file:

$ electronegativity -i /path/to/asar/archive -o result.csv

Using electronegativity when upgrading from one version of Electron to another to find breaking changes:

$ electronegativity -i /path/to/electron/app -v -u 7..8

Note: if you're running into the Fatal Error "JavaScript heap out of memory", you can run node using node --max-old-space-size=4096 electronegativity -i /path/to/asar/archive -o result.csv

Ignoring Lines or Files

Electronegativity lets you disable individual checks using eng-disable comments. For example, if you want a specific check to ignore a line of code, you can disable it as follows:

const res = eval(safeVariable); /* eng-disable DANGEROUS_FUNCTIONS_JS_CHECK */
<webview src="https://doyensec.com/" enableblinkfeatures="DangerousFeature"></webview> <!-- eng-disable BLINK_FEATURES_HTML_CHECK -->

Any eng-disable inline comment (// eng-disable, /* eng-disable */, <!-- eng-disable -->) will disable the specified check for just that line. It is also possible to provide multiple check names using both their snake case IDs (DANGEROUS_FUNCTIONS_JS_CHECK) or their construct names (dangerousFunctionsJSCheck):

shell.openExternal(eval(safeVar)); /* eng-disable OPEN_EXTERNAL_JS_CHECK DANGEROUS_FUNCTIONS_JS_CHECK */

If you put an eng-disable directive before any code at the top of a .js or .html file, that will disable the passed checks for the entire file.

Note on Global Checks and eng-disable annotations

Before v1.9.0 Global Checks couldn't be disabled using code annotations. If you are still using an old version, use -x CLI argument to manually disable a list of checks instead (e.g. -x LimitNavigationJsCheck,PermissionRequestHandlerJsCheck,CSPGlobalCheck). Note that using annotations may not be applicable for some higher-level checks such as CSP_GLOBAL_CHECK or AVAILABLE_SECURITY_FIXES_GLOBAL_CHECK. For those cases, you might want to use the -x flag to exclude specific checks from your scan.

CI/CD

Electronegativity Action may run as part of your GitHub CI/CD pipeline to get "Code scanning alerts":

Code scanning alerts

Programmatically

You can also use electronegativity programmatically, using similar options as for the CLI:

const run = require('@doyensec/electronegativity')
// or: import run from '@doyensec/electronegativity';

run({
  // input (directory, .js, .html, .asar)
  input: '/path/to/electron/app',
  // save the results to a file in csv or sarif format (optional)
  output: '/path/for/output/file',
  // true to save output as sarif, false to save as csv (optional)
  isSarif: false,
  // only run the specified checks (optional)
  customScan: ['dangerousfunctionsjscheck', 'remotemodulejscheck'],
  // only return findings with the specified level of severity or above (optional)
  severitySet: 'high',
  // only return findings with the specified level of confidence or above (optional)
  confidenceSet: 'certain',
  // 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',
  // assume the set Electron version, overriding the detected one
  electronVersion: '5.0.0',
  // use additional parser plugins
  parserPlugins: ['optionalChaining']
})
    .then(result => console.log(result))
    .catch(err => console.error(err));

The result contains the number of global and atomic checks, any errors encountered while parsing and an array of the issues found, like this:

{
  globalChecks: 6,
  atomicChecks: 36,
  errors: [
    {
      file: 'ts/main/main.ts',
      sample: 'shell.openExternal(url);',
      location: { line: 328, column: 4 },
      id: 'OPEN_EXTERNAL_JS_CHECK',
      description: 'Review the use of openExternal',
      properties: undefined,
      severity: { value: 2, name: 'MEDIUM', format: [Function: format] },
      confidence: { value: 0, name: 'TENTATIVE', format: [Function: format] },
      manualReview: true,
      shortenedURL: 'https://git.io/JeuMC'
    },
    {
      file: 'ts/main/main.ts',
      sample: 'const popup = new BrowserWindow(options);',
      location: { line: 340, column: 18 },
      id: 'CONTEXT_ISOLATION_JS_CHECK',
      description: 'Review the use of the contextIsolation option',
      properties: undefined,
      severity: { value: 3, name: 'HIGH', format: [Function: format] },
      confidence: { value: 1, name: 'FIRM', format: [Function: format] },
      manualReview: false,
      shortenedURL: 'https://git.io/Jeu1p'
    }
  ]
}

Contributing

If you're thinking about contributing to this project, please take a look at our CONTRIBUTING.md.

Credits

Electronegativity was made possible thanks to the work of many contributors.

This project has been sponsored by Doyensec LLC.

Doyensec Research

Engage us to break your Electron.js application!

electronegativity's People

Contributors

0xibram avatar baltpeter avatar bchurchill avatar bhbs avatar ikkisoft avatar jarlob avatar jason-invision avatar jnaulty avatar marshallofsound avatar mikko-apo avatar p4p3r avatar phosphore avatar vhashimotoo avatar voidsec avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

electronegativity's Issues

Output Results

It will be interesting to have an option/parameter that print out the scan result to a file.
Not something fancy, I think that a .csv should be fine.

File; Location;Issue;Description;URL "app/applicationMenu/darwin.js";"45:13";"OPEN_EXTERNAL_CHECK";"";"url"

Different results between source files and asar package

Hi,

I am trying to run your library against my Electron app and I have some issues:

  1. Does it support Typescript files?
  2. I run it twice, one with the actual source files and another with the asar package and I found some differences. When run with the source files it produced more results than the one with the asar package. In more detail, the results from the source files contained the real files with line numbers etc and the issues that were reported using the asar package.

Thanks

IteratorError Exception

Getting an error on a specific app.

░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 0% | 0/411
/Users/ikki/Downloads/electronegativity/dist/runner.js:106
        throw _iteratorError;
        ^
Error: Line 1: Unexpected token
    at ErrorHandler.constructError (/Users/ikki/Downloads/electronegativity/node_modules/esprima/dist/esprima.js:5004:22)
    at ErrorHandler.createError (/Users/ikki/Downloads/electronegativity/node_modules/esprima/dist/esprima.js:5020:27)
    at Parser.unexpectedTokenError (/Users/ikki/Downloads/electronegativity/node_modules/esprima/dist/esprima.js:1985:39)
    at Parser.tolerateUnexpectedToken (/Users/ikki/Downloads/electronegativity/node_modules/esprima/dist/esprima.js:1998:42)
    at Parser.parseStatementListItem (/Users/ikki/Downloads/electronegativity/node_modules/esprima/dist/esprima.js:3361:31)
    at Parser.parseScript (/Users/ikki/Downloads/electronegativity/node_modules/esprima/dist/esprima.js:4715:29)
    at parse (/Users/ikki/Downloads/electronegativity/node_modules/esprima/dist/esprima.js:122:61)
    at Parser.parse (/Users/ikki/Downloads/electronegativity/dist/parser/parser.js:35:37)
    at run (/Users/ikki/Downloads/electronegativity/dist/runner.js:64:34)
    at Object.<anonymous> (/Users/ikki/Downloads/electronegativity/dist/index.js:47:22)

Yarn 2 Support

Describe the bug

When running electronegativity as part of GitHub actions, I received the following error:

Run doyensec/[email protected]
  with:
    input: .
    version: latest
/opt/hostedtoolcache/node/12.22.1/x64/bin/electronegativity -> /opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/dist/index.js
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@~2.3.1 (node_modules/@doyensec/electronegativity/node_modules/chokidar/node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})

+ @doyensec/[email protected]
added 401 packages from 245 contributors in 21.089s
SyntaxError: Unknown token: { line: 3, col: 2, type: 'INVALID', value: undefined } 3:2 in lockfile
    at Parser.unexpected (/opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/node_modules/@yarnpkg/lockfile/index.js:5064:11)
    at Parser.parse (/opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/node_modules/@yarnpkg/lockfile/index.js:5193:14)
    at parse (/opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/node_modules/@yarnpkg/lockfile/index.js:5262:17)
    at Object.module.exports.exports.default (/opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/node_modules/@yarnpkg/lockfile/index.js:4835:96)
    at findElectronVersionsFromYarnLock (/opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/dist/util/electron_version.js:73:28)
    at findOldestElectronVersion (/opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/dist/util/electron_version.js:107:28)
    at async LoaderDirectory.load (/opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/dist/loader/loader_directory.js:76:29)
    at async run (/opt/hostedtoolcache/node/12.22.1/x64/lib/node_modules/@doyensec/electronegativity/dist/runner.js:57:3)

To Reproduce

Steps to reproduce the behavior:

  1. Run electronegativity on an Electron project using Yarn 2

Expected behavior
electronegativity should be able to parse a lockfile for Yarn 2 when a .yarnrc.yml is in the root directory.

**Stacktraces **
n/a

Platform (please complete the following information):

  • OS: Linux
  • Electronegativity version: latest (v1.9.1 as of this writing)

Additional context
n/a

BrowserWindow configs passed as variable is not supported

The current implementations of our JS checks do not support constructs in the following form:

let moreInfoWindow;

let windowConfig = {
   width: 800,
   height: 600,
   webPreferences: {
       preload: path.join(__dirname, 'renderer.js'),
       sandbox: true,
       nodeIntegration: false,
   }
};

moreInfoWindow = new BrowserWindow(windowConfig);
moreInfoWindow.loadURL("https://www.doyensec.com/whatever");

All JS checks based on webPreferences are looking for the BrowserWindow object astNode, and we don't have a way (do we?) to explore the entire AST again and determine the values of the webPreferences settings.

ENOENT: no such file or directory -4058 - Path exceeds 256 characters

When running the scan with the following command line, "electronegativity -i d:<pathToSource>" I received the error "[Error: ENOENT: no such file or directory, stat 'D:...
{
errorno: -4058
code: 'ENOENT'
syscall: 'stat',
path: 'D:\...
}

The problem is not that the file is missing, it is that the DOS path exceeds 256 characters and the folders and file names beyond the 256 limit are not recognized.

To Reproduce

  1. On a Windows 10 Machine create a folder structure that will exceed 256 characters
  2. Check out a project to scan in that folder path.
  3. From the command prompt run the command electronegativity -i <d:\LongPathToProject>
  4. This should reproduce the error.

Expected behavior
Most modern OSs do not have a restriction on the number of character for the filename and folder structure. My expectation was that the scan would have completed and not reported that a file was missing.

**Stacktraces **
See above error message.

Platform (please complete the following information):

  • OS: [Windows 10]
  • Electronegativity version: v1.7.0

Description and URL Field

screen shot 2018-08-21 at 11 19 19

At the moment the description field is empty and the URL is not clickable, leaving the scanner operator without any info about the findings

Support the setWindowOpenHandler API in Electron 12+

Is your feature request related to a problem? Please describe.

Electron 12 has a new API to capture and prevent window creation, webContents.setWindowOpenHandler. See: https://www.electronjs.org/docs/tutorial/security#how-10.

The existing new-window event has been deprecated.

Describe the solution you'd like

LIMIT_NAVIGATION_JS_CHECK should detect the presence of the new handler API and treat it the same way as the new-window event, assuming it is an appropriate alternative (or addition).

Describe alternatives you've considered

It's possible to continue using the deprecated event. It is not entirely clear to me how these APIs interact when used together, or whether they are completely interchangeable .

Cannot manually whitelist PERMISSION_REQUEST_HANDLER_GLOBAL_CHECK if using a partition

Describe the bug
When using a session partition, one cannot exclude the check for PERMISSION_REQUEST_HANDLER_GLOBAL_CHECK.

To Reproduce
Notice on line 50 (ses.fromPartition(partition)), one cannot exclude from the tool from throwing an audit error.

const {
    app,
    BrowserWindow,
    session
} = require("electron");
const path = require("path");
const isDev = process.env.NODE_ENV === "development";
const port = 40992; // Hardcoded; needs to match webpack.development.js and package.json
const selfHost = `http://localhost:${port}`;

// Keep a global reference of the window object, if you don't, the window will
// be closed automatically when the JavaScript object is garbage collected.
let win;

async function createWindow() {

    // Create the browser window.
    win = new BrowserWindow({
        width: 800,
        height: 600,
        title: "Application is currently initializing...",
        webPreferences: {
            devTools: isDev,
            nodeIntegration: false,
            nodeIntegrationInWorker: false,
            nodeIntegrationInSubFrames: false,
            contextIsolation: true,
            enableRemoteModule: false,
            additionalArguments: [`storePath:${app.getPath("userData")}`],
            preload: path.join(__dirname, "preload.js"),
            /* eng-disable PRELOAD_JS_CHECK */
            disableBlinkFeatures: "Auxclick"
        }
    });

    // Load app
    win.loadURL(selfHost);

    // Emitted when the window is closed.
    win.on("closed", () => {
        // Dereference the window object, usually you would store windows
        // in an array if your app supports multi windows, this is the time
        // when you should delete the corresponding element.
        win = null;
    });

    // https://electronjs.org/docs/tutorial/security#4-handle-session-permission-requests-from-remote-content
    const ses = session;
    const partition = "default";
    ses.fromPartition(partition) /* eng-disable PERMISSION_REQUEST_HANDLER_JS_CHECK */
        .setPermissionRequestHandler((webContents, permission, permCallback) => {
            let allowedPermissions = []; // Full list here: https://developer.chrome.com/extensions/declare_permissions#manifest

            if (allowedPermissions.includes(permission)) {
                permCallback(true); // Approve permission request
            } else {
                console.error(
                    `The application tried to request permission for '${permission}'. This permission was not whitelisted and has been blocked.`
                );

                permCallback(false); // Deny
            }
        });
}

// This method will be called when Electron has finished
// initialization and is ready to create browser windows.
// Some APIs can only be used after this event occurs.
app.on("ready", createWindow);

The error thrown in the tool is this

│ PERMISSION_REQUEST_HANDLER_GLOBAL_CHECK │ N/A                                                                         │ 
0:0      │ Missing PermissionRequestHandler to limit        │
│ MEDIUM | CERTAIN                        │                                                                             │ 
         │ specific permissions (e.g. openExternal) in      │
│                                         │                                                                             │ 
         │ response to events from particular origins.      │
│                                         │                                                                             │ 
         │ https://git.io/JeuM0                             │

Expected behavior
/* eng-disable PERMISSION_REQUEST_HANDLER_JS_CHECK */ should work if using a partition.

Stacktraces
None.

Platform (please complete the following information):

  • OS: Windows
  • Electronegativity version: 1.8.1

Additional context
For use in secure-electron-template, I'd like to use your tool to validate my template is secure 😄.

package.json `config` can contain non-strings

the following code seems to assume that everything under the "config" key in package.json will be a string or array:

and also here:

var res = npmConfig[config].includes(this.dangerousFlag);

this isn't the case, though – for example, with electron-forge, the default config looks like this:

  "config": {
    "forge": {
      "packagerConfig": {},
      "makers": [
        {
          "name": "@electron-forge/maker-squirrel",
          "config": {

meaning that electronegativity trips up when parsing it and prints Error parsing /path/to/package.json - npmConfig[config].includes is not a function.

Exclude from scan option throwing errors

EDIT: I now know that the correct way of excluded options is not SCREAMING_SNAKE_CASE but in PascalCase and the exclusion works.

Could you support both the SCREAMING_SNAKE_CASE name and the pascal case name? or explicitly indicate in the docs how it should be used?

Describe the bug
Exclude from scan option on API and -x option on CLI throwing the following error

You have an error in your custom checks list. Maybe you misspelt some check names?

I want to run the checker excluding "CSP_GLOBAL_CHECK" and "LIMIT_NAVIGATION_GLOBAL_CHECK", since I know I have them right but they error out anyway, maybe because I have them abstracted under a layer of wrapper testable code since electron itself is too difficult to mock around with

To Reproduce
Steps to reproduce the behavior:

  • Make a node script to run the checker:
const run = require("@doyensec/electronegativity")
const path = require("path")

run({
  // input (directory, .js, .html, .asar)
  input: path.resolve(__dirname, "..", "src"),
  excludeFromScan: ["CSP_GLOBAL_CHECK", "LIMIT_NAVIGATION_GLOBAL_CHECK"],
  parserPlugins: []
})
  .then((result) => {
    const { errors } = result
    if (errors && errors.length > 0) {
      console.error(result)
      process.exit(1)
    }
    process.exit(0)
  })
  .catch((err) => {
    console.error(err)
    process.exit(1)
  })

Outputs You have an error in your custom checks list. Maybe you misspelt some check names?

Expected behavior
For it to run the checker without checking the 2 mentioned check Ids

**Stacktraces **
None

Platform (please complete the following information):

  • OS: Ubuntu 16.04
  • Electronegativity version: 1.9.1

Other info
Besides that, running electronegativity by api without setting

...
  parserPlugins: []

Throws an error because you don't seem to be checking for the value to be a valid array before checking it's length

// Went from this
run({
 input: path.resolve(__dirname, "..", "src"),
 excludeFromScan: ["CSP_GLOBAL_CHECK", "LIMIT_NAVIGATION_GLOBAL_CHECK"]
})

// To this
run({
 // input (directory, .js, .html, .asar)
 input: path.resolve(__dirname, "..", "src"),
 excludeFromScan: ["CSP_GLOBAL_CHECK", "LIMIT_NAVIGATION_GLOBAL_CHECK"],
 parserPlugins: []
})

Update checks based on latest webPreferences defaults

We're now on Electron 7.x as stable. Since we built our checks - many webPreferences default configs have changed.

E.g. Since v5, nodeIntegration is disabled by default. Our current nodeIntegration checks will trigger a false positive if there's no explicit setting since we still assume that the default is on. I believe the same problem apply to many other checks.

Better node module support

Is your feature request related to a problem? Please describe.

👋 I'm one of the developers for Electron Forge, and I'd like to create a first-class Electron Forge plugin that would hook into the workflow right after an application is packaged (but before installers are generated). Currently, the main script for the module is the same as the bin, so when you require('@doyensec/electronegativity'), you'll just run the CLI without any arguments. This is sub-optimal for Electron Forge, as we'd have to shell out in order to configure it in any way.

Describe the solution you'd like

Move the bin script to a different file than src/index.ts, and export a Node API-friendly function instead.

If this sounds reasonable, I'd be happy to create a pull request for this.

Show warnings for missing security-relevant handlers

For example:

  1. If setPermissionRequestHandler is not used, the application does not limit session permissions at all.
  2. Creation of a new window or the navigation to a specific origin can be inspected and validated using callbacks for the new-window and willnavigate events. Your application can limit the navigation flows by implementing something like:
win.webContents.on('will-navigate', (event, newURL) => {
  if (win.webContents.getURL() !== 'https://doyensec.com' ) {
    event.preventDefault();
  }
})

Parsing error in package.json

Describe the bug
Parsing error in package.json - Unexpected token I in JSON at position 0

To Reproduce
Steps to reproduce the behavior:

  1. Clone Atom https://github.com/atom/atom
  2. Try to scan it

Stacktraces

Tolerable error parsing /Users/ikki/Downloads/atom/resources/win/atom.js - Line 5: Variable name may not be eval or arguments in strict mode
Error parsing /Users/ikki/Downloads/atom/spec/fixtures/typescript/invalid.ts - Unexpected token, expected ";" (1:14)
Error parsing /Users/ikki/Downloads/atom/spec/fixtures/packages/package-with-broken-package-json/package.json - Unexpected token I in JSON at position 0
(node:25462) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'chars' of undefined
    at Cell.mergeTableOptions (/Users/ikki/Downloads/electronegativity/node_modules/cli-table2/src/cell.js:37:33)
    at /Users/ikki/Downloads/electronegativity/node_modules/cli-table2/src/table.js:29:12
    at arrayEach (/Users/ikki/Downloads/electronegativity/node_modules/cli-table2/node_modules/lodash/lodash.js:516:11)
    at Function.forEach (/Users/ikki/Downloads/electronegativity/node_modules/cli-table2/node_modules/lodash/lodash.js:9344:14)
    at /Users/ikki/Downloads/electronegativity/node_modules/cli-table2/src/table.js:28:7
    at arrayEach (/Users/ikki/Downloads/electronegativity/node_modules/cli-table2/node_modules/lodash/lodash.js:516:11)
    at Function.forEach (/Users/ikki/Downloads/electronegativity/node_modules/cli-table2/node_modules/lodash/lodash.js:9344:14)
    at Table.toString (/Users/ikki/Downloads/electronegativity/node_modules/cli-table2/src/table.js:27:5)
    at run (/Users/ikki/Downloads/electronegativity/dist/runner.js:208:23)
(node:25462) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:25462) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Allow `eng-disable` comments to be placed above lines of code

Is your feature request related to a problem? Please describe.

eng-disable comments must be placed on the same line as the relevant code, e.g.:

contents.on("will-navigate", (ev) => { /* eng-disable LIMIT_NAVIGATION_GLOBAL_CHECK */
   ev.preventDefault();
});

This limitation poses a problem for Prettier and similar tools which autoformat code and comments based on line length and other criteria. For instance, Prettier will automatically re-format the above code like this:

contents.on("will-navigate", (ev) => {
   /* eng-disable LIMIT_NAVIGATION_GLOBAL_CHECK */
   ev.preventDefault();
});

Describe the solution you'd like

The ability to put the comment above the relevant line:

// eng-disable LIMIT_NAVIGATION_GLOBAL_CHECK
contents.on("will-navigate", (ev) => {
  ev.preventDefault();
}); 

This approach is compatible with Prettier and follows a convention used by other tools including TypeScript (// ts-ignore) and ESLint. It is resilient to future formatting changes. In my opinion, it is also more readable. :)

Describe alternatives you've considered

Sometimes code can be re-written to make Prettier happy:

contents.on("will-navigate", handlerCallback); /* eng-disable LIMIT_NAVIGATION_GLOBAL_CHECK */

But this is not always desirable or guaranteed to work due to line length restrictions.

Current npm version of Electronegativity has two issues preventing it from running properly

Describe the bug

Bug 1) The version of Electronegativity in npm currently has issues pulling the latest release list from Electron when the application is run.

Bug2) The fsevents dependency when installing a fresh copy of Electronegativity is now a 404, and when it automatically attemps to build that package, it results in several compile errors.

To Reproduce
Steps to reproduce the behavior:

(Bug 1)

  1. Run electronegativity on a target project
  2. Observe the following error:
    at Object.unlinkSync (fs.js:1035:3)
    at AvailableSecurityFixesGlobalCheck.updateReleasesList (/usr/local/lib/node_modules/@doyensec/electronegativity/dist/finder/checks/GlobalChecks/AvailableSecurityFixesGlobalCheck.js:205:26)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async AvailableSecurityFixesGlobalCheck.perform (/usr/local/lib/node_modules/@doyensec/electronegativity/dist/finder/checks/GlobalChecks/AvailableSecurityFixesGlobalCheck.js:51:7)
    at async GlobalChecks.getResults (/usr/local/lib/node_modules/@doyensec/electronegativity/dist/finder/globalchecks.js:96:20)
    at async run (/usr/local/lib/node_modules/@doyensec/electronegativity/dist/runner.js:199:12)
(node:37312) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:37312) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

(Bug 2)

  1. Install a fresh copy of electronegativity
  2. Observe the following 404 on dependency:
$> npm install @doyensec/electronegativity -g
/usr/local/bin/electronegativity -> /usr/local/lib/node_modules/@doyensec/electronegativity/dist/index.js

> [email protected] install /usr/local/lib/node_modules/@doyensec/electronegativity/node_modules/fsevents
> node install

node-pre-gyp WARN Using needle for node-pre-gyp https download
node-pre-gyp WARN Tried to download(404): https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.2.7/fse-v1.2.7-node-v72-darwin-x64.tar.gz
node-pre-gyp WARN Pre-built binaries not found for [email protected] and [email protected] (node-v72 ABI, unknown) (falling back to source compile with node-gyp)
[snip several compile errors]

Expected behavior
The application should pull the release list properly. Additionally, when installing from npm, the dependency should be available

**Stacktraces **
See bug descriptions

Platform (please complete the following information):

  • OS: MacOS
  • Electronegativity version: 1.4.0

Additional context
Thank you, this is a great tool! I confirmed the master branch works perfectly when I pull to my system locally and build/run it. I believe the problem is in the version that's being pulled from npm.

find location of nodes in html parsing

cheerio, which we're using for parsing html files, doesnt give access to the location of the vulnerable node. this blocks us from giving the location of the found issues in html files. any ideas on how to fix it? maybe switch to another html parser?

How to resolve issues with missing parsers? (eg, optionalChaining)

Is your feature request related to a problem? Please describe.
Is it possible to use configure electronNegativity to use the necessary parsers? I can't find any information on that.

The exact message I get is: This experimental syntax requires enabling the parser plugin: 'optionalChaining'

I'm not sure how I'd go about configuring Electronegativity to understand this syntax. Am I overlooking something obvious?

Fails to install 1.8.1 on Ubuntu

Describe the bug
The package fails to install on OSX for version 1.8.1

To Reproduce
On OSX:

  1. git clone https://github.com/reZach/secure-electron-template.git
  2. cd secure-electron-template
  3. Change package to version 1.8.1
  4. npm i
  5. Error occurs

Expected behavior
I expect the package to install without error on OSX.

**Stacktraces **

WIN-HVPUU8BOK0U:test bivald$ npm install  @doyensec/[email protected]
npm ERR! code ENOENT
npm ERR! syscall chmod
npm ERR! path /Users/xxxx/test/node_modules/@doyensec/electronegativity/dist/index.js
npm ERR! errno -2
npm ERR! enoent ENOENT: no such file or directory, chmod '/Users/.../node_modules/@doyensec/electronegativity/dist/index.js'
npm ERR! enoent This is related to npm not being able to find a file.
npm ERR! enoent

Platform (please complete the following information):

  • OS: Mac
  • Electronegativity version: 1.8.1

Additional context
I am the repository owner of the example in question, someone reported this error to me and I'm passing this to you to check if this is a possible known issue to you and your team.

Logical negation operator ! not considered in JS checks

Is your feature request related to a problem? Please describe.
As reported in https://twitter.com/CryptoGangsta/status/1254223839497613312?s=20

We don't currently take into account configurations like:

  // Create the browser window.
  mainWindow = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
      nodeIntegration: !0
    }
  })

We should check all boolean values for the ! sign and enhance the logic of several JS checks. Not sure if this is currently leading to false positives too.

TypeError: Cannot read property 'length' of undefined

Describe the bug
Unclear from the docs what the required input path is but I've tried I've tried an unpacked Electron dir, the /dist dir (holding a packed Electron app) as well as the .asar file itself - I get the same error:

Package returns TypeError: Cannot read property 'length' of undefined in my very simple test.

To Reproduce

const e_neg = require('@doyensec/electronegativity');
e_neg({
            // input (directory, .js, .html, .asar)
            input: path.join(/path/to/electron, 'dist', 'win-unpacked', 'resources', 'app.asar'),
        })
            .then(result => console.log(result))
            .catch(err => console.error(err));

Expected behavior
Take the input dir and run the Electron tests.

Stacktraces

TypeError: Cannot read property 'length' of undefined
    at run (D:\_a\desktop\_config\node_modules\@doyensec\electronegativity\dist\runner.js:78:26)

Platform (please complete the following information):
Win 10 x64
[email protected]

LoaderASAR tests fail

  Loader classes
    LoaderASAR
      √ fails if archive does not exist
debug: Files in ASAR archive: \browser,\browser\api,\browser\api\app.js,\browser\api\auto-updater,\browser\api\auto-updater\auto-updater-native.js,\browser\api\auto-updater\auto-updater-win.js,\browser\api\auto-updater\squirrel-update-win.js,\browser\api\auto-updater.js,\browser\api\browser-window.js,\browser\api\content-tracing.js,\browser\api\dialog.js,\browser\api\exports,\browser\api\exports\electron.js,\browser\api\global-shortcut.js,\browser\api\ipc-main.js,\browser\api\menu-item-roles.js,\browser\api\menu-item.js,\browser\api\menu.js,\browser\api\navigation-controller.js,\browser\api\power-monitor.js,\browser\api\power-save-blocker.js,\browser\api\protocol.js,\browser\api\screen.js,\browser\api\session.js,\browser\api\system-preferences.js,\browser\api\tray.js,\browser\api\web-contents.js,\browser\chrome-extension.js,\browser\desktop-capturer.js,\browser\guest-view-manager.js,\browser\guest-window-manager.js,\browser\init.js,\browser\objects-registry.js,\browser\rpc-server.js,\common,\common\api,\common\api\callbacks-registry.js,\common\api\clipboard.js,\common\api\crash-reporter.js,\common\api\deprecate.js,\common\api\deprecations.js,\common\api\exports,\common\api\exports\electron.js,\common\api\is-promise.js,\common\api\native-image.js,\common\api\shell.js,\common\init.js,\common\reset-search-paths.js,\renderer,\renderer\api,\renderer\api\desktop-capturer.js,\renderer\api\exports,\renderer\api\exports\electron.js,\renderer\api\ipc-renderer.js,\renderer\api\remote.js,\renderer\api\screen.js,\renderer\api\web-frame.js,\renderer\chrome-api.js,\renderer\content-scripts-injector.js,\renderer\extensions,\renderer\extensions\event.js,\renderer\extensions\i18n.js,\renderer\extensions\storage.js,\renderer\extensions\web-navigation.js,\renderer\init.js,\renderer\inspector.js,\renderer\override.js,\renderer\web-view,\renderer\web-view\guest-view-internal.js,\renderer\web-view\web-view-attributes.js,\renderer\web-view\web-view-constants.js,\renderer\web-view\web-view.js
debug: Extracting file: \browser\api\app.js
      1) returns a Map
debug: Files in ASAR archive: \browser,\browser\api,\browser\api\app.js,\browser\api\auto-updater,\browser\api\auto-updater\auto-updater-native.js,\browser\api\auto-updater\auto-updater-win.js,\browser\api\auto-updater\squirrel-update-win.js,\browser\api\auto-updater.js,\browser\api\browser-window.js,\browser\api\content-tracing.js,\browser\api\dialog.js,\browser\api\exports,\browser\api\exports\electron.js,\browser\api\global-shortcut.js,\browser\api\ipc-main.js,\browser\api\menu-item-roles.js,\browser\api\menu-item.js,\browser\api\menu.js,\browser\api\navigation-controller.js,\browser\api\power-monitor.js,\browser\api\power-save-blocker.js,\browser\api\protocol.js,\browser\api\screen.js,\browser\api\session.js,\browser\api\system-preferences.js,\browser\api\tray.js,\browser\api\web-contents.js,\browser\chrome-extension.js,\browser\desktop-capturer.js,\browser\guest-view-manager.js,\browser\guest-window-manager.js,\browser\init.js,\browser\objects-registry.js,\browser\rpc-server.js,\common,\common\api,\common\api\callbacks-registry.js,\common\api\clipboard.js,\common\api\crash-reporter.js,\common\api\deprecate.js,\common\api\deprecations.js,\common\api\exports,\common\api\exports\electron.js,\common\api\is-promise.js,\common\api\native-image.js,\common\api\shell.js,\common\init.js,\common\reset-search-paths.js,\renderer,\renderer\api,\renderer\api\desktop-capturer.js,\renderer\api\exports,\renderer\api\exports\electron.js,\renderer\api\ipc-renderer.js,\renderer\api\remote.js,\renderer\api\screen.js,\renderer\api\web-frame.js,\renderer\chrome-api.js,\renderer\content-scripts-injector.js,\renderer\extensions,\renderer\extensions\event.js,\renderer\extensions\i18n.js,\renderer\extensions\storage.js,\renderer\extensions\web-navigation.js,\renderer\init.js,\renderer\inspector.js,\renderer\override.js,\renderer\web-view,\renderer\web-view\guest-view-internal.js,\renderer\web-view\web-view-attributes.js,\renderer\web-view\web-view-constants.js,\renderer\web-view\web-view.js
debug: Extracting file: \browser\api\app.js
      2) extracts file from ASAR
    LoaderFile
      √ fails if archive does not exist
      √ returns a Map
      √ extracts file from ASAR

  73 passing (88ms)
  2 failing

  1) Loader classes
       LoaderASAR
         returns a Map:
     TypeError: Cannot read property 'files' of undefined
      at Filesystem.searchNodeFromDirectory (node_modules\asar\lib\filesystem.js:19:21)
      at Filesystem.getNode (node_modules\asar\lib\filesystem.js:129:23)
      at Filesystem.getFile (node_modules\asar\lib\filesystem.js:140:23)
      at Object.module.exports.extractFile (node_modules\asar\lib\asar.js:179:61)
      at LoaderAsar.load (C:/Projects/electronegativity/src/loader/loader_asar.js:26:31)
      at Context.<anonymous> (C:/Projects/electronegativity/test/test_loader.js:39:14)

  2) Loader classes
       LoaderASAR
         extracts file from ASAR:
     TypeError: Cannot read property 'files' of undefined
      at Filesystem.searchNodeFromDirectory (node_modules\asar\lib\filesystem.js:19:21)
      at Filesystem.getNode (node_modules\asar\lib\filesystem.js:129:23)
      at Filesystem.getFile (node_modules\asar\lib\filesystem.js:140:23)
      at Object.module.exports.extractFile (node_modules\asar\lib\asar.js:179:61)
      at LoaderAsar.load (C:/Projects/electronegativity/src/loader/loader_asar.js:26:31)
      at Context.<anonymous> (C:/Projects/electronegativity/test/test_loader.js:43:14)

Don't publish npm-shrinkwrap.json

Describe the bug
This project publishes its npm-shrinkwrap.json. That's discouraged:

It's strongly discouraged for library authors to publish this file, since that would prevent end users from having control over transitive dependency updates.

The practical impact is that electronegativity's dev dependencies are ending up in our package-lock.json (marked as either optional or extraneous, maybe depending on npm version). This is even though we are declaring electronegativity as a devDependency so its dev dependencies should be ignored. That may be caused by an npm issue. But if electronegativity didn't publish its npm-shrinkwrap.json, that bug wouldn't matter.

There are three dev dependencies that are particularly problematic because they or their dependencies have security advisories against them:

To Reproduce
Steps to reproduce the behavior:

  1. npm init in new directory
  2. In package.json add:
  "devDependencies": {
    "@doyensec/electronegativity": "^1.9.0"
  }
  1. npm install --include=dev
  2. Search in package-lock.json for "base", "chokidar", "snapdragon-node" to see optional or extraneous dependencies.

Expected behavior
Expect there to be no npm-shrinkwrap.json published with electronegativity, so its package.json is used to declare its dependencies. Only the runtime dependencies of electronegativity (and their trees of runtime dependencies) should end up in our package-lock.json.

Platform (please complete the following information):

  • OS: Windows
  • Electronegativity version: 1.9.0

Update Wiki Page about Context Isolation

Describe the bug
https://github.com/doyensec/electronegativity/wiki/CONTEXT_ISOLATION_JS_CHECK says that context isolation is to be expected to be enabled by default in v5 of electron. This feature is enabled by default in v12 only, according to https://www.electronjs.org/docs/tutorial/context-isolation#how-do-i-enable-it

To Reproduce
Steps to reproduce the behavior:

  1. Read https://github.com/doyensec/electronegativity/wiki/CONTEXT_ISOLATION_JS_CHECK
  2. Compare with https://www.electronjs.org/docs/tutorial/context-isolation#how-do-i-enable-it

Expected behavior
I expect the information on the wiki pages to be accurate and not misleading.

**Stacktraces **
not applicable

Platform (please complete the following information):
github wiki pages

Additional context
not applicable

Guideline for handling False Positives / False Negatives

Guideline for handling False Positives / False Negatives

Hi,
is there a guideline for handling False Positives / False Negatives?

We are currently looking into using electronegativity for automated security checks in a project.
However, we noticed that our code design and architecture can have a great effect on the results.

For example CONTEXT_ISOLATION_JS_CHECK:

Case a)

Declaration of webPreferences.contextIsolation is part of the BrowserWindow configuration object literal.

const window = new BrowserWindow({
  webPreferences: {
    contextIsolation: true,
  },
});

No issue is found, works as expected.

Case b)

Value of webPreferences.contextIsolation is not part of the BrowserWindow configuration object literal.

const contextIsolation = true
const window = new BrowserWindow({
  webPreferences: {
    contextIsolation: contextIsolation,
  },
});

CONTEXT_ISOLATION_JS_CHECK fails, which was not expected.

Case c)

BrowserWindow configuration is extracted to a variable and possibly to another file for reuse.

const configuration= {
  webPreferences: {
    contextIsolation: true,
  },
};
const window = new BrowserWindow(configuration);

CONTEXT_ISOLATION_JS_CHECK fails, which was not expected.

The reason for this became obvious when we looked at how the CONTEXT_ISOLATION_JS_CHECK is implemented.
It just leverages simple static analysis using only the AST which works fine for a).
However, b) and c) would require some form of control flow or data flow analysis.
Since control flow and data flow analysis is heavy lifting especially for JavaScript projects, we don't expect this to be implemented soon (or ever).
For now, we are considering just using your checks for manual review without running electronegativity.
But a guide for handling false postive / false negatives and how design decisions affect the outcome of the analysis would be nice.

Nevertheless, many thanks for this project and security recommendations for electron!

Implement a new check related to Electron versioning

what i have in mind is: if package.json is found in a directory or asar archive, use the check, otherwise, skip it. it would look for electron in the dependencies and then compare it to the latest version. the only thing we need to figure out is a way to fetch the latest electron version, any ideas?

Error when parsing app.js: "ImportDeclaration should appear when the mode is ES6 and in the module context."

Describe the bug
When analysing an electron app I receive a parsing error message from eslint. Unfortunately this is an electron app I am not allowed to share.

To Reproduce
Steps to reproduce the behavior:

  1. $ electronegativity -i .
▄▄▄ ▄▄▌ ▄▄▄ .▄▄·▄▄▄▄▄▄▄               
▀▄.▀██• ▀▄.▀▐█ ▌•██ ▀▄ █▪             
▐▀▀▪██▪ ▐▀▀▪██ ▄▄▐█.▐▀▀▄ ▄█▀▄         
▐█▄▄▐█▌▐▐█▄▄▐███▌▐█▌▐█•█▐█▌.▐▌        
 ▀▀▀.▀▀▀ ▀▀▀·▀▀▀ ▀▀▀.▀  ▀▀█▄▀▪        
 ▐ ▄▄▄▄ .▄▄ • ▄▄▄▄▄▄▄▪  ▌ ▐▪▄▄▄▄▄▄· ▄▌
•█▌▐▀▄.▀▐█ ▀ ▐█ ▀•██ ██▪█·██•██ ▐█▪██▌
▐█▐▐▐▀▀▪▄█ ▀█▄█▀▀█▐█.▐█▐█▐█▐█▐█.▐█▌▐█▪
██▐█▐█▄▄▐█▄▪▐▐█ ▪▐▐█▌▐█▌███▐█▐█▌·▐█▀·.
▀▀ █▪▀▀▀·▀▀▀▀ ▀  ▀▀▀▀▀▀. ▀ ▀▀▀▀▀  ▀ •
     v1.3.2  https://doyensec.com/

Scan Status:
40 check(s) successfully loaded: 5 global, 35 atomic
████████████████████████████████████████ 100% | 18/18
Error parsing <redacted>/resources/app/src/app.js - ImportDeclaration should appear when the mode is ES6 and in the module context.

Expected behavior
Since the config file /usr/lib/node_modules/@doyensec/electronegativity/.eslintrc.json specifies the necessary options for parsing as a module:

    "parserOptions": {
        "ecmaVersion": 2017,
        "sourceType": "module"
    },

Therefore I would expect app.js to be parsed without problems.

Platform (please complete the following information):

  • OS: Ubuntu 18.04
  • Electronegativity version: 1.3.2

Error when installing via npm on Ubuntu

Describe the bug
Installing via npm install @doyensec -g fails.

Linux <> 4.15.0-47-generic #50-Ubuntu SMP Wed Mar 13 10:44:52 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

To Reproduce
Steps to reproduce the behavior:

  1. sudo npm install @doyensec/electronegativity -g

Expected behavior
Install the app.

**Stacktraces **
npm ERR! path /usr/lib/node_modules/@doyensec/electronegativity/dist/index.js
npm ERR! code ENOENT
npm ERR! errno -2
npm ERR! syscall chmod
npm ERR! enoent ENOENT: no such file or directory, chmod '/usr/lib/node_modules/@doyensec/electronegativity/dist/index.js'
npm ERR! enoent This is related to npm not being able to find a file.
npm ERR! enoent

npm ERR! A complete log of this run can be found in:

Platform (please complete the following information):

  • OS: Ubuntu
  • Latest via NPM

Issue location, line of code

We should add a feature that will show the line of code (at least 100 chars from the point in location) of the issue, in this way the result will be immediately useful

Command line arguments search is not implemented

Checks like 'Additionally, search for the runtime flag —disable-web-security in
package.json, and within the application codebase.' are not implemented yet.

There is already package.json check for electron version. But it is not clear what to use for the search within codebase: full text grep or just string literals.

How does one ignore LIMIT_NAVIGATION_GLOBAL_CHECK?

Describe the bug
All of my new-window and will-navigate events are secure and do not need to be notified as errors in the report that's generated by electronegativity. How do I disable LIMIT_NAVIGATION_GLOBAL_CHECK from spitting out an error? I tried /* eng-disable LIMIT_NAVIGATION_GLOBAL_CHECK */ at the top of my file, but the error still persists.

To Reproduce
This sample main.js file can suffice to explain my attempts at disabling .

/* eng-disable LIMIT_NAVIGATION_GLOBAL_CHECK */
const {
    app,
    BrowserWindow,
  } = require("electron");
  const path = require("path");
  const isDev = process.env.NODE_ENV === "development";
  const port = 40992; // Hardcoded; needs to match webpack.development.js and package.json
  const selfHost = `http://localhost:${port}`;
  
  // Keep a global reference of the window object, if you don't, the window will
  // be closed automatically when the JavaScript object is garbage collected.
  let win;
  
  async function createWindow() {
  
    // Create the browser window.
    win = new BrowserWindow({
      width: 800,
      height: 600,
      title: "Application is currently initializing...",
      webPreferences: {
        devTools: isDev,
        nodeIntegration: false,
        nodeIntegrationInWorker: false,
        nodeIntegrationInSubFrames: false,
        contextIsolation: true,
        enableRemoteModule: false,
        additionalArguments: [`storePath:${app.getPath("userData")}`],
        preload: path.join(__dirname, "preload.js"), /* eng-disable PRELOAD_JS_CHECK */
        disableBlinkFeatures: "Auxclick"
      }
    });
    
    // Load app
    win.loadURL(selfHost);
  
    // Emitted when the window is closed.
    win.on("closed", () => {
      // Dereference the window object, usually you would store windows
      // in an array if your app supports multi windows, this is the time
      // when you should delete the corresponding element.
      win = null;
    });
  }
  
  // This method will be called when Electron has finished
  // initialization and is ready to create browser windows.
  // Some APIs can only be used after this event occurs.
  app.on("ready", createWindow);
  
  // Quit when all windows are closed.
  app.on("window-all-closed", () => {
    // On macOS it is common for applications and their menu bar
    // to stay active until the user quits explicitly with Cmd + Q
    if (process.platform !== "darwin") {
      app.quit();
    }
  });
  
  app.on("activate", () => {
    // On macOS it's common to re-create a window in the app when the
    // dock icon is clicked and there are no other windows open.
    if (win === null) {
      createWindow();
    }
  });
  
  // https://electronjs.org/docs/tutorial/security#12-disable-or-limit-navigation
  app.on("web-contents-created", (event, contents) => {
    contents.on("will-navigate", (contentsEvent, navigationUrl) => { /* eng-disable LIMIT_NAVIGATION_JS_CHECK  */
      const parsedUrl = new URL(navigationUrl);
      const validOrigins = [selfHost];
  
      // Log and prevent the app from navigating to a new page if that page's origin is not whitelisted
      if (!validOrigins.includes(parsedUrl.origin)) {
        console.error(
          `The application tried to redirect to the following address: '${parsedUrl}'. This origin is not whitelisted and the attempt to navigate was blocked.`
        );
  
        contentsEvent.preventDefault();
        return;
      }
    });
  
    // https://electronjs.org/docs/tutorial/security#13-disable-or-limit-creation-of-new-windows
    contents.on("new-window", (contentsEvent, navigationUrl) => { /* eng-disable LIMIT_NAVIGATION_JS_CHECK */
      const parsedUrl = new URL(navigationUrl);
      const validOrigins = [];
  
      // Log and prevent opening up a new window
      if (!validOrigins.includes(parsedUrl.origin)) {
        console.error(
          `The application tried to open a new window at the following address: '${navigationUrl}'. This attempt was blocked.`
        );
  
        contentsEvent.preventDefault();
        return;
      }
    });
  });

Expected behavior
I expect the LIMIT_NAVIGATION_GLOBAL_CHECK error not to be logged in the report.

Stacktraces

│ LIMIT_NAVIGATION_GLOBAL_CHECK           │ N/A                                                                         │ 
0:0      │ Missing navigation limits using .on new-window   │
│ HIGH | CERTAIN                          │                                                                             │ 
         │ and will-navigate events                         │
│                                         │                                                                             │ 
         │ https://git.io/JeuMs                             │

Platform (please complete the following information):

  • OS: Windows
  • Electronegativity version: 1.8.1

Additional context
I don't like putting the comment on the first line, I'd like to have the freedom to put it anywhere in my file.

Consider introducing "Severity" and "Confidence" attributes for each check. Ideas?

Several commercial security tools use the following two attributes for their scanner checks:

Severity (How critical is the issue discovered by the check)

  • High
  • Medium
  • Low
  • Informational

Confidence (How accurate is the check)

  • Certain
  • Firm
  • Tentative

I fell that 'confidence' can be useful to track the maturity / heuristics of each check, however 'severity' won't really provide much value (e.g. if we mark nodeIntegration:on as High, but the application doesn't load remote content...well, the check isn't accurate)

Allow annotations in source code to ignore a check

Is your feature request related to a problem? Please describe.
Due to manual review required checks always being directly unaddressable, we can't run electronegativity in CI without ignoring the return code.

Describe the solution you'd like
When running Electronegativity in CI it'd be great to have a means to ignore particular checks through adding an annotation in the source code, similar to eslint's ignore annotation. https://eslint.org/docs/user-guide/configuring.html#disabling-rules-with-inline-comments

Describe alternatives you've considered
Optionally an exclude flag to allow excluding a particular check from the runtime (although this would be global for the entire scan, which isn't ideal)

Crash

Hi, running the tool I got this crash.

257/335(node:17600) UnhandledPromiseRejectionWarning: SyntaxError: Unexpected token I in JSON at position 0
    at JSON.parse (<anonymous>)
    at ElectronVersionCheck.match (/usr/local/lib/node_modules/@doyensec/electronegativity/dist/finder/checks/ElectronVersionCheck.js:38:23)
    at Finder.find (/usr/local/lib/node_modules/@doyensec/electronegativity/dist/finder/finder.js:199:38)
    at run (/usr/local/lib/node_modules/@doyensec/electronegativity/dist/runner.js:83:27)
(node:17600) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejec)
(node:17600) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

It is crashing on the following repos:
Mailspring https://github.com/Foundry376/Mailspring
WWIIStatsViewer https://github.com/ImNotLiam/WWIIStatsViewer
atom https://github.com/atom/atom

TypeError parsing Electron Release list

Describe the bug
During a scan of my app I receive a TypeError while the Electron Release List is being parsed.

To Reproduce
Steps to reproduce the behavior:

  1. Run electronegativity -i my-app -o results.csv

Expected behavior
I expected the Release List to be parsed and my app to be scanned.

**Stacktraces **

Releases list is up to date.
(node:54021) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'value' of undefined
    at run (/Users/jasonrogers/.nvm/versions/node/v8.15.1/lib/node_modules/@doyensec/electronegativity/dist/runner.js:211:26)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:189:7)
(node:54021) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 7)
(node:54021) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Platform (please complete the following information):

  • OS: Mac
  • Electronegativity version: 1.2.1

Additional context
I noticed that downloading the Release List is taking a long time. Could the download be timing out, and, therefore, only a partial file being downloaded?

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

Repo: https://github.com/sotch-pr35mac/syng

  • electronegativity -i ./syng -o ./report/syng.csv
41% | 14/34
<--- Last few GCs --->

[7207:0x102803c00]    10076 ms: Scavenge 1394.6 (1422.7) -> 1394.3 (1424.2) MB, 3.0 / 0.0 ms  (average mu = 0.134, current mu = 0.040) allocation failure
[7207:0x102803c00]    11032 ms: Mark-sweep 1395.2 (1424.2) -> 1394.9 (1422.7) MB, 953.5 / 0.0 ms  (average mu = 0.077, current mu = 0.018) allocation failure scavenge might not succeed
[7207:0x102803c00]    11036 ms: Scavenge 1395.8 (1422.7) -> 1395.5 (1424.2) MB, 3.1 / 0.0 ms  (average mu = 0.077, current mu = 0.018) allocation failure


<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x30957ec841bd]
    1: StubFrame [pc: 0x30957ec87c1a]
Security context: 0x2b810b81e6c9 <JSObject>
    2: /* anonymous */ [0x2b81354ebcd9] [/usr/local/lib/node_modules/@doyensec/electronegativity/node_modules/esprima/dist/esprima.js:~5849] [pc=0x30957ef59e98](this=0x2b81ec521271 <Scanner map = 0x2b817faf11d1>)
    3: /* anonymous */ [0x2b81354ebe59] [/usr/local/lib/node_modules/@doyensec/electronegativity/node_modules...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 0x100033d65 node::Abort() [/usr/local/bin/node]
 2: 0x100035500 node::FatalTryCatch::~FatalTryCatch() [/usr/local/bin/node]
 3: 0x10019f10a v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/usr/local/bin/node]
 4: 0x10056d6b2 v8::internal::Heap::FatalProcessOutOfMemory(char const*) [/usr/local/bin/node]
 5: 0x10056c669 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/usr/local/bin/node]
 6: 0x10056a2f8 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/usr/local/bin/node]
 7: 0x10057683c v8::internal::Heap::AllocateRawWithRetry(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [/usr/local/bin/node]
 8: 0x1005451d4 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [/usr/local/bin/node]
 9: 0x1007cf974 v8::internal::Runtime_AllocateInNewSpace(int, v8::internal::Object**, v8::internal::Isolate*) [/usr/local/bin/node]
10: 0x30957ec841bd
Abort trap: 6

Improve ELECTRON_VERSION_CHECK

The current implementation of ELECTRON_VERSION_CHECK is using a static array for the "latest" versions (see safe_releases.json) - to compare the releases with the electron dependency in package.json (if present).

Ideally, we would want to improve this check in two ways:

  • Obtain the latest releases from the Electron Github/Website so that a new release won't require an update of this tool. Even better, we could warn the user if the version used is old AND contains security issues (this can be implemented by checking whether there're any [SECURITY] fixes in between releases - https://electronjs.org/releases )
  • Derive the current Electron version directly from the bundle (asar file), application binary, ...and all other possible ways (instead of just checking package.json). I have some ideas here - talk to @ikkisoft

UnhandledPromiseRejectionWarning while fetching new Electron releases

Describe the bug
During a scan of my app, I receive a UnhandledPromiseRejectionWarning while the new Electron releases are being fetched.

To Reproduce
Steps to reproduce the behavior:

  1. Run electronegativity -i my-app

Expected behavior
I expect the new Electron releases to be fetched and my app to be scanned.

Stacktraces

Scan Status:
40 check(s) successfully loaded: 5 global, 35 atomic
████████████████████████████████████████ 100% | 9/9
Fetching Electron's new releases, this may take a while...

(node:112) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory, open 'releases./"430ea9483d96f2286b350f4be5c9d5450ba9a5b3.json' at Object.fs.openSync (fs.js:646:18) at Object.fs.writeFileSync (fs.js:1299:33) at AvailableSecurityFixesGlobalCheck.updateReleasesList (/usr/local/lib/node_modules/@doyensec/electronegativity/dist/finder/checks/GlobalChecks/AvailableSecurityFixesGlobalCheck.js:245:22) at <anonymous> at process._tickCallback (internal/process/next_tick.js:188:7) (node:112) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1) (node:112) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Platform (please complete the following information):

  • OS: Ubuntu 18.04 LTS running under Windows 10 Linux subsystem
  • Electronegativity version: 1.3.0
  • Electron version: 5.0.6

eng-disable CSP_GLOBAL_CHECK not working

Describe the bug
Applying /* eng-disable CSP_GLOBAL_CHECK */ to the line reported CSP_GLOBAL_CHECK cannot disable the warning.

The line reported has applied CSP already.

To Reproduce
Steps to reproduce the behavior:

  1. Write an Electron application with CSP like what is mentioned here: https://www.electronjs.org/docs/tutorial/security#csp-http-header
  2. Scan with electronegativity
  3. Follow the location reported and apply /* eng-disable CSP_GLOBAL_CHECK */
  4. Scan again
  5. The warning is not dismissed

Expected behavior
The warning shall be dismissed

Platform (please complete the following information):

  • OS: Win
  • Electronegativity version: 1.8.1

Additional context
The warning is dismissed if I apply /* eng-disable CSP_JS_CHECK */. So I think the "Check ID" in the scan report is incorrect.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.