Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SECURITY NOTICE: electron-packager v5.2.1 - v6.0.2 don't check SSL certificate validity #333

Closed
feross opened this issue Apr 19, 2016 · 13 comments

Comments

@feross
Copy link
Contributor

feross commented Apr 19, 2016

There exists a bug in electron-packager from versions 5.2.1 - 6.0.2 where the --strict-ssl command line option defaults to false when not explicitly set to true.

This only affects users using the electron-packager CLI. The strict-ssl option defaults to true for the node.js API.

The commit that introduced the issue is here: 30bdd0b
The commit that fixed the issue is here: ebea1d8

The issue is fixed in v7.0.0. All users should upgrade immediately.

It's also recommended to delete the electron-download cache folder, by default named .electron, and located in your home folder. For example:

rm -rf ~/.electron

Props to @malept for discovering this.

@feross
Copy link
Contributor Author

feross commented Apr 19, 2016

I would like to npm deprecate the broken versions, but I'm not an npm owner anymore. I used to be, but looks like someone removed me.

Can a current npm owner please run the following command:

npm deprecate electron-packager@"5.2.1-6.0.2" "Critical security bug fixed in v7.0.0 - read more at https://github.com/electron-userland/electron-packager/issues/333"

@maxogden @stefanbuck @jden @sindresorhus @kfranqueiro @jlord @malept

@malept malept added this to the 7.0.0 milestone Apr 19, 2016
@malept malept self-assigned this Apr 19, 2016
@malept
Copy link
Member

malept commented Apr 19, 2016

I've got it.

@feross
Copy link
Contributor Author

feross commented Apr 19, 2016

@malept Thank you.

@malept
Copy link
Member

malept commented Apr 19, 2016

I also added some clarifying text to your advisory (underlined) because the download cache is configurable.

@feross
Copy link
Contributor Author

feross commented Apr 19, 2016

@malept Good call.

@malept
Copy link
Member

malept commented Apr 19, 2016

@feross I ran the command and am attempting to verify it:

mkdir /tmp/testcase
cd /tmp/testcase
npm init .
# [...a bunch of user input...]
npm install electron-packager@6.0.2

I should get a warning message with the issue URL, right? (I am not.)

@montogeek
Copy link

@malept You can use npm init -y :)

@malept malept removed their assignment Apr 19, 2016
@malept
Copy link
Member

malept commented Apr 19, 2016

I have to be AFK for a bit, @feross I've (re?)added you to the npm list so you can figure out what's going on here.

@montogeek that is helpful for future testcases, but my question still stands.

@montogeek
Copy link

I just tested it, not warning message

montogeek ~/test [09:02 PM]→ 😛
👉  npm install -D electron-packager@6.0.2
test@1.0.0 /Users/montogeek/test
└─┬ electron-packager@6.0.2
  ├─┬ asar@0.11.0
  │ ├── chromium-pickle-js@0.1.0
  │ ├─┬ commander@2.9.0
  │ │ └── graceful-readlink@1.0.1
  │ ├── cuint@0.2.1
  │ ├─┬ glob@6.0.4
  │ │ ├─┬ inflight@1.0.4
  │ │ │ └── wrappy@1.0.1
  │ │ ├── inherits@2.0.1
  │ │ └── once@1.3.3
  │ ├─┬ minimatch@3.0.0
  │ │ └─┬ brace-expansion@1.1.3
  │ │   ├── balanced-match@0.3.0
  │ │   └── concat-map@0.0.1
  │ └─┬ mksnapshot@0.3.0
  │   ├─┬ decompress-zip@0.3.0
  │   │ ├─┬ binary@0.3.0
  │   │ │ ├── buffers@0.1.1
  │   │ │ └─┬ chainsaw@0.1.0
  │   │ │   └── traverse@0.3.9
  │   │ ├── mkpath@0.1.0
  │   │ ├─┬ nopt@3.0.6
  │   │ │ └── abbrev@1.0.7
  │   │ ├── q@1.4.1
  │   │ ├─┬ readable-stream@1.1.14
  │   │ │ └── isarray@0.0.1
  │   │ └─┬ touch@0.0.3
  │   │   └── nopt@1.0.10
  │   └─┬ request@2.55.0
  │     ├── aws-sign2@0.5.0
  │     ├─┬ bl@0.9.5
  │     │ └── readable-stream@1.0.34
  │     ├── caseless@0.9.0
  │     ├─┬ combined-stream@0.0.7
  │     │ └── delayed-stream@0.0.5
  │     ├── forever-agent@0.6.1
  │     ├─┬ form-data@0.2.0
  │     │ └── async@0.9.2
  │     ├─┬ har-validator@1.8.0
  │     │ ├── bluebird@2.10.2
  │     │ ├─┬ chalk@1.1.3
  │     │ │ ├── ansi-styles@2.2.1
  │     │ │ ├── escape-string-regexp@1.0.5
  │     │ │ ├─┬ has-ansi@2.0.0
  │     │ │ │ └── ansi-regex@2.0.0
  │     │ │ ├── strip-ansi@3.0.1
  │     │ │ └── supports-color@2.0.0
  │     │ └─┬ is-my-json-valid@2.13.1
  │     │   ├── generate-function@2.0.0
  │     │   ├─┬ generate-object-property@1.2.0
  │     │   │ └── is-property@1.0.2
  │     │   ├── jsonpointer@2.0.0
  │     │   └── xtend@4.0.1
  │     ├─┬ hawk@2.3.1
  │     │ ├── boom@2.10.1
  │     │ ├── cryptiles@2.0.5
  │     │ ├── hoek@2.16.3
  │     │ └── sntp@1.0.9
  │     ├─┬ http-signature@0.10.1
  │     │ ├── asn1@0.1.11
  │     │ ├── assert-plus@0.1.5
  │     │ └── ctype@0.5.3
  │     ├── isstream@0.1.2
  │     ├── json-stringify-safe@5.0.1
  │     ├─┬ mime-types@2.0.14
  │     │ └── mime-db@1.12.0
  │     ├── node-uuid@1.4.7
  │     ├── oauth-sign@0.6.0
  │     ├── qs@2.4.2
  │     ├── stringstream@0.0.5
  │     ├── tough-cookie@2.2.2
  │     └── tunnel-agent@0.4.2
  ├─┬ electron-download@2.1.1
  │ ├─┬ debug@2.2.0
  │ │ └── ms@0.7.1
  │ ├── home-path@1.0.3
  │ ├─┬ nugget@1.6.2
  │ │ ├─┬ pretty-bytes@1.0.4
  │ │ │ ├── get-stdin@4.0.1
  │ │ │ └─┬ meow@3.7.0
  │ │ │   ├─┬ camelcase-keys@2.1.0
  │ │ │   │ └── camelcase@2.1.1
  │ │ │   ├── decamelize@1.2.0
  │ │ │   ├─┬ loud-rejection@1.3.0
  │ │ │   │ ├── array-find-index@1.0.1
  │ │ │   │ └── signal-exit@2.1.2
  │ │ │   ├── map-obj@1.0.1
  │ │ │   ├─┬ normalize-package-data@2.3.5
  │ │ │   │ ├── hosted-git-info@2.1.4
  │ │ │   │ ├─┬ is-builtin-module@1.0.0
  │ │ │   │ │ └── builtin-modules@1.1.1
  │ │ │   │ ├── semver@5.1.0
  │ │ │   │ └─┬ validate-npm-package-license@3.0.1
  │ │ │   │   ├─┬ spdx-correct@1.0.2
  │ │ │   │   │ └── spdx-license-ids@1.2.1
  │ │ │   │   └─┬ spdx-expression-parse@1.0.2
  │ │ │   │     └── spdx-exceptions@1.0.4
  │ │ │   ├─┬ read-pkg-up@1.0.1
  │ │ │   │ ├─┬ find-up@1.1.2
  │ │ │   │ │ ├── path-exists@2.1.0
  │ │ │   │ │ └─┬ pinkie-promise@2.0.1
  │ │ │   │ │   └── pinkie@2.0.4
  │ │ │   │ └─┬ read-pkg@1.1.0
  │ │ │   │   ├─┬ load-json-file@1.1.0
  │ │ │   │   │ ├─┬ parse-json@2.2.0
  │ │ │   │   │ │ └─┬ error-ex@1.3.0
  │ │ │   │   │ │   └── is-arrayish@0.2.1
  │ │ │   │   │ ├── pify@2.3.0
  │ │ │   │   │ └─┬ strip-bom@2.0.0
  │ │ │   │   │   └── is-utf8@0.2.1
  │ │ │   │   └── path-type@1.1.0
  │ │ │   ├─┬ redent@1.0.0
  │ │ │   │ ├─┬ indent-string@2.1.0
  │ │ │   │ │ └─┬ repeating@2.0.1
  │ │ │   │ │   └─┬ is-finite@1.0.1
  │ │ │   │ │     └── number-is-nan@1.0.0
  │ │ │   │ └── strip-indent@1.0.1
  │ │ │   └── trim-newlines@1.0.0
  │ │ ├─┬ progress-stream@1.2.0
  │ │ │ ├── speedometer@0.1.4
  │ │ │ └─┬ through2@0.2.3
  │ │ │   └─┬ xtend@2.1.2
  │ │ │     └── object-keys@0.4.0
  │ │ ├── single-line-log@0.4.1
  │ │ └── throttleit@0.0.2
  │ ├── path-exists@1.0.0
  │ └─┬ rc@1.1.6
  │   ├── deep-extend@0.4.1
  │   ├── ini@1.3.4
  │   └── strip-json-comments@1.0.4
  ├── electron-osx-sign@0.3.0
  ├─┬ extract-zip@1.5.0
  │ ├─┬ concat-stream@1.5.0
  │ │ ├─┬ readable-stream@2.0.6
  │ │ │ ├── core-util-is@1.0.2
  │ │ │ ├── isarray@1.0.0
  │ │ │ ├── process-nextick-args@1.0.6
  │ │ │ └── string_decoder@0.10.31
  │ │ └── typedarray@0.0.6
  │ ├── debug@0.7.4
  │ ├─┬ mkdirp@0.5.0
  │ │ └── minimist@0.0.8
  │ └─┬ yauzl@2.4.1
  │   └─┬ fd-slicer@1.0.1
  │     └── pend@1.2.0
  ├─┬ fs-extra@0.26.7
  │ ├── graceful-fs@4.1.3
  │ ├── jsonfile@2.3.0
  │ ├── klaw@1.2.0
  │ └── path-is-absolute@1.0.0
  ├─┬ get-package-info@0.0.2
  │ ├── bluebird@3.3.5
  │ └─┬ lodash.get@4.2.1
  │   └── lodash._stringtopath@4.7.1
  ├── minimist@1.2.0
  ├─┬ mkdirp@0.5.1
  │ └── minimist@0.0.8
  ├─┬ mv@2.1.1
  │ └── rimraf@2.4.5
  ├── ncp@2.0.0
  ├── object-assign@4.0.1
  ├─┬ plist@1.2.0
  │ ├── base64-js@0.0.8
  │ ├── util-deprecate@1.0.2
  │ ├─┬ xmlbuilder@4.0.0
  │ │ └── lodash@3.10.1
  │ └── xmldom@0.1.22
  ├── rcedit@0.5.0
  ├── resolve@1.1.7
  ├─┬ rimraf@2.5.2
  │ └── glob@7.0.3
  └── run-series@1.1.4

npm WARN test@1.0.0 No description
npm WARN test@1.0.0 No repository field.

@malept
Copy link
Member

malept commented Apr 19, 2016

Does npm deprecate give any output? I ran it twice, each time it took ~a dozen seconds to run and then exited.

@feross
Copy link
Contributor Author

feross commented Apr 19, 2016

Looks like the syntax was slightly off. Should be ">= 5.2.1 | <= 6.0.2".

I ran the command, so now everyone should be getting the warning.

@feross
Copy link
Contributor Author

feross commented Apr 19, 2016

I recommend leaving this issue open for at least a few weeks, so more users will have a chance to read this and upgrade.

linclark added a commit to linclark/tofino that referenced this issue Apr 19, 2016
fmitchell-r7 pushed a commit to rapid7/awsaml that referenced this issue May 4, 2016
Specifically, we want the security fix where SSL certificate validity
wasn't being checked.
electron/packager#333
antoinepairet added a commit to antoinepairet/electron-packager-interactive that referenced this issue May 9, 2016
@malept
Copy link
Member

malept commented May 11, 2016

It's been a few weeks, so closing. I think the deprecation notice is sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants