5 ms·
Excellent effort! However, it also introduces a _major_ security hole: The Electron Desktop App simply loads an index.html which points at remote JavaScript. T
by felixrieseberg 10y ago
Excellent effort! However, it also introduces a _major_ security hole:
The Electron Desktop App simply loads an index.html which points at remote JavaScript. That's crazy dangerous - if that endpoint gets compromised, nothing keeps the attacker from running `rm -rf /` on every user's machine.
I'm a member of the Electron maintainers group and fully realize that this is partly on us for not educating better. Remote code should never run with full privileges - consider using a `webview` instead, which can be sandboxed.
- mobitar 10y agoGotcha, easy fix, will get that out asap. We did this as a way to push out changes quicker since we were still making major improvements often.
- pvg 10y agoI think you're not at a point where you should be advertising security as a feature (rather than eventual goal) of your product.
- Alterlife 10y agoTrue in some measure of any product in any stage of development.
- deleted 10y ago[deleted]
- golanggeek 10y agoWhat you found are just bugs. But their main point is indeed security - compared to say simplenote. Here everything is indeed encrypted and they have no way of reading our data
- pvg 10y agoAre they just bugs? encryptText(text, key) { var keyData = CryptoJS.enc.Hex.parse(key); var ivData = CryptoJS.enc.Hex.parse(""); var encrypted = CryptoJS.AES.encrypt(text, keyData, { iv: ivData, mode: CryptoJS.mode.CBC [...] The basic encryption operation seems to be AES CBC with a constant, reused IV.
- mobitar 10y agoItems are encrypted with random keys. No two items are ever encrypted with same key. Thus no need for IV, and makes implementations across platforms simpler.
- pvg 10y agoIs there actually any description of how you're encrypting, how you manage and distribute keys, why you believe this is secure, why you're not using an authenticated mode, etc? "Thus no need for IV" when your desktop client just pulls a minified blob from somewhere is not particularly reassuring.
- mobitar 10y agoYes, it's all here: http://standardfile.org/#encryption http://standardfile.org/#encryption
- dotancohen 10y ago> What you found are just bugs. Most security issues are. Unless you have an extensive background in application security, especially on the stack that you are using (Electron, etc), then don't advertise security as a feature. Let someone else who knows how to crack 'secure' apps decide if your app is secure or not. As Bruce Schneier famously quipped, anyone can invent a security system that he himself cannot break.
- smoyer 10y agoOr sign each release with a private key on a machine that's "very protected from the Internet" and only have the Electron app automatically pull the update if the signature can be verified with the public key. Note that the public key should also not be on the web server (try Keybase ... if you don't have it I'll send you an invite). EDIT: Oops ... see the comment below as apparently Electron supports code signing via certificates.
- napoleond 10y agoI'm just getting started with Electron, so forgive me if this is a stupid question, but how is that different from the updates endpoint getting compromised (for apps with auto update enabled)?
- felixrieseberg 10y agoThat's an excellent question! The auto updater requires your packages to be code-signed, meaning that someone would have to compromise the endpoint _and_ also be able to sign code with your root-trusted certificate.
- napoleond 10y agoOh neat! That makes sense, but I didn't realize it was set up for code signing. Good to know :)
- esonderegger 10y agoThe auto-updater really isn't ready for prime time, sadly. I'd suggest waiting until https://github.com/electron/electron/issues/8106 https://github.com/electron/electron/issues/8106 is resolved before using it in your project.
- lorenzhs 10y agoSubresource Integrity can also help prevent damage from compromised external hosts. It's a good defense against CDNs turning malicious.
- astrostl 10y agoIs education the only way around this? Sounds disastrous enough to warrant active policing.