doyensec / electronegativity

Electronegativity is a tool to identify misconfigurations and security anti-patterns in Electron applications.
Apache License 2.0
972 stars 66 forks source link

TypeError: Cannot read property 'length' of undefined #72

Closed petef19 closed 4 years ago

petef19 commented 4 years ago

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 electronegativity@1.6.0

phosphore commented 4 years ago

You should pass the needed options, even if empty/unused. See https://github.com/doyensec/electronegativity#programmatically:

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
  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'
})
    .then(result => console.log(result))
    .catch(err => console.error(err));

Also be aware that the current version v1.6.0 on NPM is behind a few commits from master https://github.com/doyensec/electronegativity/compare/v1.6.0...master!

petef19 commented 4 years ago

^^^ got it working.

But there should be clear instructions that something labeled as OPTIONAL is still required in the config.

Thanks.

phosphore commented 4 years ago

^^^ got it working.

But there should be clear instructions that something labeled as OPTIONAL is still required in the config.

Thanks.

You are right, we missed to set a default for customScan when used as a library (@baltpeter?). I'll keep this open to track the issue until a fix is pushed. Thanks!

baltpeter commented 4 years ago

You are right, we missed to set a default for customScan when used as a library (@baltpeter?).

Oh good point. I'll implement that later today.

baltpeter commented 4 years ago

Fixed in #73.


Although, in my defense, customScan was not labelled as optional. :D

  // only run the specified checks
  customScan: ['dangerousfunctionsjscheck', 'remotemodulejscheck'],
petef19 commented 4 years ago

played some more w/ it, got some follow up questions:

(1) on the IDE console, location, severity and confidence prints as [Object] but not the contents of the object

any suggestion on how to see that data ?

(2) we get no errors but same issues (repeatedly) listed, most of them are like Evaluate the implementation of the custom callback in setPermissionRequestHandler although we directly implemented the example from your website... are there more details on what triggered an issue other than to "evaluate", which we have done already ? ;-)

(3) for issues raised such as SANDBOX_JS_CHECK w/ Use sandbox for untrusted origins or NODE_INTEGRATION_JS_CHECK w/ Disable nodeIntegration for untrusted origins, how can we specify that we do NOT include untrusted and/or external content ?

meaning: this flag does not apply to our app

(4) is there an option to EXCLUDE certain scans, see (3) above ?

if we use the customScan array to include all scans we need then we'd have to list all the scans we want, in order to exclude 1-2 that we don't want...

Thanks.